[libvirt] [PATCH] scsi: Need to translate disk source pool in config attach path

[Erik Skultety]

On 06/11/2015 11:15 PM, John Ferlan wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1228007
> 
> When attaching a scsi volume lun via the attach-device --config or
> --persistent options, there was no translation of the source pool
> like there was for the live path, thus the attempt to modify the config
> would fail since not enough was known about the disk.
> 
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>  src/qemu/qemu_driver.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 34e5581..6bb8549 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -8016,7 +8016,8 @@ qemuDomainUpdateDeviceLive(virConnectPtr conn,
>  static int
>  qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps,
>                               virDomainDefPtr vmdef,
> -                             virDomainDeviceDefPtr dev)
> +                             virDomainDeviceDefPtr dev,
> +                             virConnectPtr conn)
>  {
>      virDomainDiskDefPtr disk;
>      virDomainNetDefPtr net;
> @@ -8033,6 +8034,8 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps,
>                             _("target %s already exists"), disk->dst);
>              return -1;
>          }
> +        if (virStorageTranslateDiskSourcePool(conn, disk) < 0)
> +            return -1;
>          if (qemuCheckDiskConfig(disk) < 0)
>              return -1;
>          if (virDomainDiskInsert(vmdef, disk))
> @@ -8501,7 +8504,8 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml,
>                                           VIR_DOMAIN_DEVICE_ACTION_ATTACH) < 0)
>              goto endjob;
>  
> -        if ((ret = qemuDomainAttachDeviceConfig(qemuCaps, vmdef, dev)) < 0)
> +        if ((ret = qemuDomainAttachDeviceConfig(qemuCaps, vmdef, dev,
> +                                                dom->conn)) < 0)
>              goto endjob;
>      }
>  
> 
I think we should also update the docs in the way that we also support
storage type 'volume' with disk device type 'lun'. And that is because
we do not really care about the storage type when performing a device
attach (we do care about this when doing snapshots), yet we still need
to call virStorageDiskSourcePool which returns instantly for every
storage type, except for volume. Documentation update should also imply
updating our RNG schema (domaincommon I think), otherwise the use case
described in the BZ won't validate. ACK if you also update both docs and
RNG.

Regards,
Erik