On Wed, Jul 10, 2013 at 02:34:30PM +0200, Martin Kletzander wrote: > We never check for disks with duplicate targets connected to the same > controller/bus/etc. That means we go ahead, create the same IDs for > them and pass them to qemu, which subsequently fails to start. > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=968899 > Signed-off-by: Martin Kletzander <mkletzan@xxxxxxxxxx> > --- > src/conf/domain_conf.c | 42 +++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 41 insertions(+), 1 deletion(-) > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index 3398d8b..01720e1 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -2629,6 +2629,45 @@ virDomainDeviceInfoIterate(virDomainDefPtr def, > > > static int > +virDomainDefRejectDuplicateDiskTargets(virDomainDefPtr def) > +{ > + char *disk_id = NULL; > + int ret = -1; > + size_t i = 0; > + virHashTablePtr targets = NULL; > + > + if (!(targets = virHashCreate(def->ndisks, NULL))) > + goto cleanup; > + > + for (i = 0; i < def->ndisks; i++) { > + virDomainDiskDefPtr disk = def->disks[i]; > + > + if (virAsprintf(&disk_id, "%d%s%d%d%d%d", > + disk->bus, > + NULLSTR(disk->dst), > + disk->info.addr.drive.controller, > + disk->info.addr.drive.bus, > + disk->info.addr.drive.target, > + disk->info.addr.drive.unit) < 0) Err, disk->info.addr is a union of many different types of address. You can't just arbitrarily reference info.addr.drive without first validating the type of the union. Also, it seems like this problem is more general than just disks. Any type of device can have an <address> element set and cause a clash, not merely disks attached to controllers. So I'd say we want something that iterates over all devices in the domaindef and validates every address element. Yes, we might catch PCI address clashes later in QEMU code, but there's no harm in detecting them up front, if we can do so in a way that is generally applicable to all address types Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list