[libvirt] [PATCH] Reject duplicate disk targets

[Daniel P. Berrange]

On Wed, Jul 10, 2013 at 03:51:45PM +0200, Martin Kletzander wrote:
> On 07/10/2013 02:39 PM, Daniel P. Berrange wrote:
> > 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 at redhat.com>
> >> ---
> >>  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.
> > 
> 
> I jumped straight into that because we know it is disk and we do it the
> same way when assigning aliases.  I'd be glad to see what option I
> missed, thanks for any pointers.

Assigning aliases is not the same, because the alias is stored
in the same part of the struct, regardless of address type.

Any disk with bus=virtio uses PCI addressing. Any disk with
bus=usb uses USB addresing.

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 :|