Re: [PATCH v1 09/14] virDomainDeviceInfoCheckABIStability: Check for alias too

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Oct 19, 2017 at 02:16:41PM +0200, Martin Kletzander wrote:
> On Thu, Oct 19, 2017 at 10:11:04AM +0200, Michal Privoznik wrote:
> > Since we'll be passing user's input onto qemu command line, we
> > have to make sure aliases don't change during migration and all
> > the other places where ABI is checked. Aliases are part of ABI
> > now.
> > 
> 
> I don't see how change of an alias would have an impact on the guest OS since it
> is not visible at all.  Is QEMU using the IDs to refer to objects in the
> migrated data stream?  AFAIK (actually not me, I just asked random stranger
> about it) it does not (except the memory modules that Peter told us about in one
> of previous series).  I see no reason why we would disable the option to rename
> a device when doing migration.  We are reconstructing the command-line anyway.
> Did migration fail for you when you tried this?

QEMU never includes the device ID in the migration stream, except for that
one screw-up wrt memory modules that we know about.

So technically the change below could be loosened to only apply to the
memory module alias. Since the whole point of aliases is to have a long
term stable identifier for devices though, I figure it is probably
beneficial to enforce no-renames for all devices even if QEMU is safe
without it.

> 
> > Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> > ---
> > src/conf/domain_conf.c | 7 +++++++
> > 1 file changed, 7 insertions(+)
> > 
> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > index 0cf67dff1..cb80939af 100644
> > --- a/src/conf/domain_conf.c
> > +++ b/src/conf/domain_conf.c
> > @@ -19870,6 +19870,13 @@ virDomainDeviceInfoCheckABIStability(virDomainDeviceInfoPtr src,
> >         return false;
> >     }
> > 
> > +    if (STRNEQ_NULLABLE(src->alias, dst->alias)) {
> > +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> > +                       _("Target device alias %s does not match source %s"),
> > +                       NULLSTR(src->alias), NULLSTR(dst->alias));
> > +        return false;
> > +    }
> > +
> >     switch ((virDomainDeviceAddressType) src->type) {
> >     case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI:
> >         if (src->addr.pci.domain != dst->addr.pci.domain ||

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux