[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v2 15/15] vbox: Generate disk address element in dumpxml




On 10/24/2017 03:35 PM, Dawid Zamirski wrote:
> This patch adds <address> element to each <disk> device since device
> names alone won't adequately reflect the storage device layout in the
> VM. With this patch, the ouput produced by dumpxml will faithfully
> reproduce the storage layout of the VM if used with define.
> ---
>  src/vbox/vbox_common.c | 79 +++++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 68 insertions(+), 11 deletions(-)
> 

This one seems OK, but I didn't think long about it.  I'll look again
when the v3 is posted.

What I'll also ask is that when v3 hits, please be sure to create a
docs/news.xml article that briefly describes some of the fixes you've
made in this series that would go into the next release.

Tks -

John
> diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
> index d1d8804c7..95d35631f 100644
> --- a/src/vbox/vbox_common.c
> +++ b/src/vbox/vbox_common.c
> @@ -3211,8 +3211,9 @@ vboxDumpDisks(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine)
>      PRBool readOnly;
>      nsresult rc;
>      virDomainDiskDefPtr disk = NULL;
> +    virDomainControllerDefPtr ctrl = NULL;
>      char *mediumLocUtf8 = NULL;
> -    size_t sdCount = 0, i;
> +    size_t sdCount = 0, i, j;
>      int ret = -1;
>  
>      def->ndisks = 0;
> @@ -3353,26 +3354,83 @@ vboxDumpDisks(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine)
>              goto cleanup;
>          }
>  
> -        if (storageBus == StorageBus_IDE) {
> +        disk->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE;
> +        disk->info.addr.drive.bus = 0;
> +        disk->info.addr.drive.unit = devicePort;
> +
> +        switch ((enum StorageBus) storageBus) {
> +        case StorageBus_IDE:
>              disk->bus = VIR_DOMAIN_DISK_BUS_IDE;
> -        } else if (storageBus == StorageBus_SATA) {
> -            sdCount++;
> +            disk->info.addr.drive.bus = devicePort; /* primary, secondary */
> +            disk->info.addr.drive.unit = deviceSlot; /* master, slave */
> +
> +            break;
> +        case StorageBus_SATA:
>              disk->bus = VIR_DOMAIN_DISK_BUS_SATA;
> -        } else if (storageBus == StorageBus_SCSI ||
> -                   storageBus == StorageBus_SAS) {
>              sdCount++;
> +
> +            break;
> +        case StorageBus_SCSI:
> +        case StorageBus_SAS:
>              disk->bus = VIR_DOMAIN_DISK_BUS_SCSI;
> -        } else if (storageBus == StorageBus_Floppy) {
> +            sdCount++;
> +
> +            /* In vbox, if there's a disk attached to SAS controller, there will
> +             * be libvirt SCSI controller present with model "lsi1068", and we
> +             * need to find its index
> +             */
> +            for (j = 0; j < def->ncontrollers; j++) {
> +                ctrl = def->controllers[j];
> +
> +                if (ctrl->type != VIR_DOMAIN_CONTROLLER_TYPE_SCSI)
> +                    continue;
> +
> +                if (storageBus == StorageBus_SAS &&
> +                    ctrl->model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSISAS1068) {
> +                    disk->info.addr.drive.controller = ctrl->idx;
> +                    break;
> +                }
> +
> +                if (storageBus == StorageBus_SCSI &&
> +                    ctrl->model != VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSISAS1068) {
> +                    disk->info.addr.drive.controller = ctrl->idx;
> +                    break;
> +                }
> +            }
> +
> +            break;
> +        case StorageBus_Floppy:
>              disk->bus = VIR_DOMAIN_DISK_BUS_FDC;
> +
> +            break;
> +        case StorageBus_Null:
> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                           _("Unsupported null storage bus"));
> +            goto cleanup;
>          }
>  
> -        if (deviceType == DeviceType_HardDisk)
> +        switch ((enum DeviceType) deviceType) {
> +        case DeviceType_HardDisk:
>              disk->device = VIR_DOMAIN_DISK_DEVICE_DISK;
> -        else if (deviceType == DeviceType_Floppy)
> +
> +            break;
> +        case DeviceType_Floppy:
>              disk->device = VIR_DOMAIN_DISK_DEVICE_FLOPPY;
> -        else if (deviceType == DeviceType_DVD)
> +
> +            break;
> +        case DeviceType_DVD:
>              disk->device = VIR_DOMAIN_DISK_DEVICE_CDROM;
>  
> +            break;
> +        case DeviceType_Network:
> +        case DeviceType_USB:
> +        case DeviceType_SharedFolder:
> +        case DeviceType_Null:
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("Unsupported vbox device type: %d"), deviceType);
> +            goto cleanup;
> +        }
> +
>          if (readOnly == PR_TRUE)
>              disk->src->readonly = true;
>  
> @@ -4098,7 +4156,6 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags)
>          goto cleanup;
>      if (vboxDumpStorageControllers(def, machine) < 0)
>          goto cleanup;
> -
>      if (vboxDumpDisks(def, data, machine) < 0)
>          goto cleanup;
>  
> 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]