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

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(a)redhat.com&gt; > > --- > > 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 :|