Displaying mountables in the error output of guestfish etc
Cédric pointed out this problem we have: $ guestmount -a /var/lib/libvirt/images/sles12sp1-pv.img -m '/dev/sda1:/:subvol=.snapshots/2/snapshot:btrfs' /mnt libguestfs: error: mount_vfs: /dev/sda1 on / (options: 'subvol=.snapshots/2/snapshot'): mount: mount(2) failed: No such file or directory guestmount: '/dev/sda1' could not be mounted. guestmount: Check mount(8) man page to ensure options 'subvol=.snapshots/2/snapshot' guestmount: are supported by the filesystem that is being mounted. guestmount: Did you mean to mount one of these filesystems? guestmount: btrfsvol:/dev/sda1/@ (btrfs) guestmount: btrfsvol:/dev/sda1/@/.snapshots (btrfs) guestmount: btrfsvol:/dev/sda1/@/opt (btrfs) guestmount: btrfsvol:/dev/sda1/@/srv (btrfs) guestmount: btrfsvol:/dev/sda1/@/tmp (btrfs) guestmount: btrfsvol:/dev/sda1/@/home (btrfs) [etc] The problem here is the strings btrfsvol:... are the internal "mountable" format, and shouldn't be displayed to end users for a couple of reasons: (1) It's not intended that people would "know about" these strings. You're only compliant with the API if you get a string from an API like guestfs_list_filesystems and pass it to another API like guestfs_mount. (2) These strings aren't actually useful, because you cannot pass them back to the guestfish/guestmount `-m' option. Instead you have to use the non-obvious form: -m /dev/sda1:/:subvol=/@/tmp How do we solve this? Trickier than it seems. The first attempt to Cédric tried was to call guestfs_internal_parse_mountable which conveniently splits the internal string into a device and a volume. As the name suggests, that is an internal API. We shouldn't be calling this from our tools, and in any case it doesn't work unless we define -DGUESTFS_PRIVATE=1 all over the place. So that's a bad idea. I think probably the way to solve this is with new public APIs for returning the device and volume name from a mountable. Similar to guestfs_internal_parse_mountable in fact, but a public API that we commit to. char *device = guestfs_mountable_device (g, const char *mountable); char *volume = guestfs_mountable_volume (g, const char *mountable); If a mountable doesn't have a (sub-)volume, the second API should probably return a well-defined errno (ESRCH or ENOENT?). The code in fish/options.c then just has to: foreach fs in guestfs_list_filesystems: device = guestfs_mountable_device fs volume = guestfs_mountable_volume fs if guestfs_mountable_volume fs returned well defined errno: printf "-m %s" device else printf "-m %s:/:subvol=%s" device volume That's my idea anyway ... Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html
On Friday 26 February 2016 16:50:56 Richard W.M. Jones wrote:
Cédric pointed out this problem we have:
$ guestmount -a /var/lib/libvirt/images/sles12sp1-pv.img -m '/dev/sda1:/:subvol=.snapshots/2/snapshot:btrfs' /mnt libguestfs: error: mount_vfs: /dev/sda1 on / (options: 'subvol=.snapshots/2/snapshot'): mount: mount(2) failed: No such file or directory guestmount: '/dev/sda1' could not be mounted. guestmount: Check mount(8) man page to ensure options 'subvol=.snapshots/2/snapshot' guestmount: are supported by the filesystem that is being mounted. guestmount: Did you mean to mount one of these filesystems? guestmount: btrfsvol:/dev/sda1/@ (btrfs) guestmount: btrfsvol:/dev/sda1/@/.snapshots (btrfs) guestmount: btrfsvol:/dev/sda1/@/opt (btrfs) guestmount: btrfsvol:/dev/sda1/@/srv (btrfs) guestmount: btrfsvol:/dev/sda1/@/tmp (btrfs) guestmount: btrfsvol:/dev/sda1/@/home (btrfs) [etc]
The problem here is the strings btrfsvol:... are the internal "mountable" format, and shouldn't be displayed to end users for a couple of reasons:
(1) It's not intended that people would "know about" these strings. You're only compliant with the API if you get a string from an API like guestfs_list_filesystems and pass it to another API like guestfs_mount.
(2) These strings aren't actually useful, because you cannot pass them back to the guestfish/guestmount `-m' option. Instead you have to use the non-obvious form:
-m /dev/sda1:/:subvol=/@/tmp
How do we solve this? Trickier than it seems.
The first attempt to Cédric tried was to call guestfs_internal_parse_mountable which conveniently splits the internal string into a device and a volume. As the name suggests, that is an internal API. We shouldn't be calling this from our tools, and in any case it doesn't work unless we define -DGUESTFS_PRIVATE=1 all over the place. So that's a bad idea.
I think probably the way to solve this is with new public APIs for returning the device and volume name from a mountable. Similar to guestfs_internal_parse_mountable in fact, but a public API that we commit to.
char *device = guestfs_mountable_device (g, const char *mountable);
char *volume = guestfs_mountable_volume (g, const char *mountable);
If a mountable doesn't have a (sub-)volume, the second API should probably return a well-defined errno (ESRCH or ENOENT?).
Sounds good; I'd just do - guestfs_mountable_volume -> guestfs_mountable_subvolume - EINVAL as return value of the above in case of error
The code in fish/options.c then just has to:
foreach fs in guestfs_list_filesystems: device = guestfs_mountable_device fs volume = guestfs_mountable_volume fs if guestfs_mountable_volume fs returned well defined errno: printf "-m %s" device else printf "-m %s:/:subvol=%s" device volume
... switching the order of the checks: if subvolume: printf "-m %s:/:subvol=%s" device subvolume else printf "-m %s" device this way, if there will be more "subparts" in the filesystem identifiers, they can be added as cascading conditions in the if. -- Pino Toscano
participants (2)
-
Pino Toscano -
Richard W.M. Jones