On Fri, Oct 09, 2020 at 11:37:41AM +0100, Richard W.M. Jones wrote:
> On Fri, Oct 09, 2020 at 11:33:43AM +0100, Richard W.M. Jones wrote:
> >
> > This is the patch I tested which works (on top of the
> > patch posted):
> >
> > diff --git a/lib/canonical-name.c b/lib/canonical-name.c
> > index e0c7918b4..ae4def692 100644
> > --- a/lib/canonical-name.c
> > +++ b/lib/canonical-name.c
> > @@ -53,8 +53,16 @@ guestfs_impl_canonical_device_name (guestfs_h *g, const
> > char *device)
> > * BitLocker-encrypted volume, so simply return the original
> > * name in that case.
> > */
> > - if (ret == NULL && guestfs_last_errno (g) == EINVAL)
> > - ret = safe_strdup (g, device);
> > + if (ret == NULL) {
> > + if (guestfs_last_errno (g) == EINVAL)
> > + ret = safe_strdup (g, device);
> > + else
> > + /* Make sure the original error gets pushed through the
> > + * error handlers.
> > + */
> > + guestfs_int_error_errno (g, guestfs_last_errno (g),
> > + "%s", guestfs_last_error (g));
> > + }
> > }
> > else
> > ret = safe_strdup (g, device);
> >
> > ---
> >
> > Current upstream:
> >
> > $ guestfish scratch 1M : run : canonical-device-name "/dev/dm-999"
> > libguestfs: error: lvm_canonical_lv_name: stat: /dev/dm-999: No such file
> > or directory
> > /dev/dm-999 <-----
> >
> > Patch posted without the above patch added:
> >
> > $ ./run guestfish scratch 1M : run : canonical-device-name "/dev/dm-999"
> >
> > (no output, but the command fails with exit code 1)
> >
> > Patch posted + above patch:
> >
> > $ ./run guestfish scratch 1M : run : canonical-device-name "/dev/dm-999"
> > libguestfs: error: lvm_canonical_lv_name: stat: /dev/dm-999: No such file
> > or directory
>
> Actually I didn't notice this, but it improves on the current upstream
> behaviour.
>
> Current upstream calls the error handlers and returns a non-error
> original string (see <----- line above) and exit code 0. With the
> patch we return an error.
This breaks virt-inspector, at least when we run the test suite which
has a phony Ubuntu guest with a non-existent /dev/mapper/* in its
/etc/fstab.
The manpage for guestfs_canonical_device_name[1] is sort of ambiguous
here. It says that "/dev/mapper/*" is
Converted to /dev/VG/LV form using guestfs_lvm_canonical_lv_name.
and guestfs_lvm_canonical_lv_name will certainly return an error for a
non-existent name.
However it does also say:
Other strings are returned unmodified.
You can sort of read it both ways. Because it breaks a long-standing
user of this API I'm going to change this so it behaves more like the
current upstream code (original error goes to debug, return original
device string).
Rich.
[1] https://libguestfs.org/guestfs.3.html#guestfs_canonical_device_name
--
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
_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs
