Opened 2 years ago

Closed 2 years ago

#18011 closed bug (fixed)

mountvolume doesn't show all volume names

Reported by: augiedoggie Owned by: nobody
Priority: normal Milestone: R1/beta4
Component: Applications/Command Line Tools Version: R1/Development
Keywords: Cc: korli
Blocked By: Blocking:
Platform: All

Description

I have a few scripts on my machine that make use of the mountvolume command but something seems to have changed lately. I am running hrev56541 and the command no longer shows the volume name for my extra Haiku partition which breaks my scripts.

Running mountvolume or mountvolume -l shows:

Volume                           File System                   Size Mounted At (Device)
--------------------------------------------------------------------------------------------
efiboot                          FAT32 File System           512.0M (nvme/0/0)
Haiku                            Be File System              119.0G /boot  (nvme/0/1)
                                 Be File System              119.0G /storage  (nvme/0/2)

The volume name for my 'storage' partition is missing but does appear correctly when viewed using DriveSetup

Change History (9)

comment:1 by bipolar, 2 years ago

BPartition::ContentName() now returns a BString (after hrev56463), and mountvolume still has lines like:

const char* name = partition->ContentName();

on line 166 and line 272.

Maybe that's the reason for this bug? (as it was for BootManager in #17969, and fixed in hrev56513)

Edit: a quick swapping of ContentName() for RawContentName() fixes the issue here, but I guess a more thought out fix would be better :-D

Last edited 2 years ago by bipolar (previous) (diff)

comment:2 by bipolar, 2 years ago

Searching ContentName() under haiku/src shows some places that might also need updates after hrev56463:

tests/apps/partitioner/Partitioner.cpp

tests/kits/storage/disk_device/DiskDeviceTest.cpp

And possibly servers/mount/Automounter.cpp? On line 216 reads if (partition->ContentName() == NULL, isn't that condition always false with ContentName() now returning a BString?

Version 0, edited 2 years ago by bipolar (next)

comment:3 by waddlesplash, 2 years ago

Cc: korli added
Milestone: UnscheduledR1/beta5

comment:4 by pulkomandy, 2 years ago

The first part of the patch for generating names for volumes was not merged: https://review.haiku-os.org/c/haiku/+/5381

Can someone contact jessicah and see if she agrees to remove her -2 vote which is blocking it? I think the part of the patch that was already merged resolves her concerns, but I didn't expect that someone would cherry-pick it and merge it separately from the first part, they were meant to be merged together.

comment:5 by korli, 2 years ago

bipolar says it's the change to BString as a return type which seems to introduce the issue. please check with: https://review.haiku-os.org/c/haiku/+/5757

comment:6 by bipolar, 2 years ago

Before I was getting this:

> mountvolume -l
Volume                           File System                   Size Mounted At (Device)
--------------------------------------------------------------------------------------------
k/scsi/0/0/0                     NT File System               20.0G (scsi/0/0/0/0)
k/scsi/0/0/0                     Ext2 File System             20.0G (scsi/0/0/0/1)
k/scsi/0/0/0                     NT File System              400.0G (scsi/0/0/0/2_0)
k/scsi/0/0/0                     Be File System               20.0G /boot  (scsi/0/0/0/2_1)
k/scsi/0/0/0/2_2                 Be File System                5.8G (scsi/0/0/0/2_2)

And after applying korli's change locally now I get:

> ./mountvolume -l
Volume                           File System                   Size Mounted At (Device)
--------------------------------------------------------------------------------------------
Win10 [500 GB]                   NT File System               20.0G (scsi/0/0/0/0)
Linux [500 GB]                   Ext2 File System             20.0G (scsi/0/0/0/1)
Datos [500 GB]                   NT File System              400.0G (scsi/0/0/0/2_0)
Haiku-[500GB]                    Be File System               20.0G /boot  (scsi/0/0/0/2_1)
Haiku-[500GB]-x86                Be File System                5.8G (scsi/0/0/0/2_2)

Thanks korli!

What about the other uses of BPartition::ContentName() I've mentioned on comment:2? The fixes on the test ones seem trivial (used on printf() calls, so adding .String() would do the trick there).

Any comments on that line 214 from Automounter.cpp? Can ContentName() ever return NULL now?

Last edited 2 years ago by bipolar (previous) (diff)

in reply to:  5 comment:7 by augiedoggie, 2 years ago

Replying to korli:

bipolar says it's the change to BString as a return type which seems to introduce the issue. please check with: https://review.haiku-os.org/c/haiku/+/5757

That patch fixes the problem for me also. Thanks guys!

comment:8 by korli, 2 years ago

applied in hrev56547

comment:9 by korli, 2 years ago

Milestone: R1/beta5R1/beta4
Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.