Re: [libvirt] [PATCHv7 3/4] libvirt/qemu - check address confliction before addition.

On Fri, Mar 25, 2011 at 05:17:03PM +0900, KAMEZAWA Hiroyuki wrote: > >From 638341bdf3eaac824e36d265e134608279750049 Mon Sep 17 00:00:00 2001 > From: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com&gt; > Date: Fri, 25 Mar 2011 17:10:58 +0900 > Subject: [PATCHv7 3/4] libvirt/qemu - check address confliction before addition. > > qemuDomainAttachDevicePersistent() calls qemuDomainAssignPCIAddresses() > and virDomainDefAddImplicitControllers() at the end of its call. > > But PCI/Drive address confliction checks are > PCI - confliction will be found but error report is not verbose. > Drive - never done. > > For example, you can add following (unusual) 2 devices without errors. > > <disk type='file' device='disk'> > <driver name='qemu' type='raw'/> > <source file='/var/lib/libvirt/images/test3.img'/> > <target dev="sdx" bus='scsi'/> > <address type='drive' controller='0' bus='0' unit='0'/> > </disk> > > <disk type='file' device='disk'> > <driver name='qemu' type='raw'/> > <source file='/var/lib/libvirt/images/test3.img'/> > <target dev="sdy" bus='scsi'/> > <address type='drive' controller='0' bus='0' unit='0'/> > </disk> > > It's better to check drive address confliction before addition. > > Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com&gt; > --- > src/conf/domain_conf.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++ > src/conf/domain_conf.h | 2 + > src/libvirt_private.syms | 1 + > src/qemu/qemu_driver.c | 9 +++++++ > 4 files changed, 71 insertions(+), 0 deletions(-) > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index 3e3f342..4a54f62 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -1287,6 +1287,65 @@ void virDomainDefClearDeviceAliases(virDomainDefPtr def) > virDomainDeviceInfoIterate(def, virDomainDeviceInfoClearAlias, NULL); > } > > +static int virDomainDeviceAddressMatch(virDomainDefPtr def ATTRIBUTE_UNUSED, > + virDomainDeviceInfoPtr info, > + void *opaque) > +{ > + virDomainDeviceInfoPtr checked = opaque; > + /* skip to check confliction of alias */ > + if (info->type != checked->type) > + return 0; > + if (info->alias && checked->alias && strcmp(info->alias, checked->alias)) !STREQ instead of strcmp

> + return -1; > + if (!memcmp(&info->addr, &checked->addr, sizeof(info->addr))) Is it safe to memcmp an union like this? In the cases members of an union are of different sizes, and we intent to memcmp an union member that has a smaller size than the other members, then data in space not used by the union member to be compared is also compared. This is not a desired result.