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

Re: [libvirt] [PATCHv2 05/16] storage: use enum for disk driver type



On 10/13/2012 04:00 PM, Eric Blake wrote:
> Actually use the enum in the domain conf structure.
> 
> * src/conf/domain_conf.h (_virDomainDiskDef): Store enum rather
> than string for disk type.
> * src/conf/domain_conf.c (virDomainDiskDefFree)
> (virDomainDiskDefParseXML, virDomainDiskDefFormat)
> (virDomainDiskDefForeachPath): Adjust users.
> * src/xenxs/xen_sxpr.c (xenParseSxprDisks, xenFormatSxprDisk):
> Likewise.
> * src/xenxs/xen_xm.c (xenParseXM, xenFormatXMDisk): Likewise.
> * src/vbox/vbox_tmpl.c (vboxAttachDrives): Likewise.
> * src/libxl/libxl_conf.c (libxlMakeDisk): Likewise.
> ---

> 
> @@ -4158,23 +4152,34 @@ virDomainDiskDefParseXML(virCapsPtr caps,
>      def->wwn = wwn;
>      wwn = NULL;
> 
> -    if (!def->driverType &&
> -        caps->defaultDiskDriverType &&
> -        !(def->driverType = strdup(virStorageFileFormatTypeToString(
> -                                       caps->defaultDiskDriverType))))
> -        goto no_memory;
> +    if (driverType) {
> +        def->format = virStorageFileFormatTypeFromString(driverType);
> +        if (def->format <= 0) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                           _("unknown driver format value '%s'"),
> +                           driverType);
> +            goto error;
> +        }
> +    } else {
> +        def->format = caps->defaultDiskDriverType;
> +    }

> @@ -5209,11 +5210,10 @@ qemuBuildCommandLine(virConnectPtr conn,
> 
>              if (disk->type == VIR_DOMAIN_DISK_TYPE_DIR) {
>                  /* QEMU only supports magic FAT format for now */
> -                if (disk->driverType &&
> -                    STRNEQ(disk->driverType, "fat")) {
> +                if (disk->format && disk->format != VIR_STORAGE_FILE_FAT) {

Going by inspection, I need a tweak here - in the old code, if you had a
<disk type='dir'> but no <driver type='fat'/>, then disk->driverType was
NULL, which we treated as a synonym for auto.  But in the new code,
disk->format defaults to whatever the driver capabilities state, which
in patch 4/16 is either AUTO (-1) or RAW (1), so for the same behavior,
this needs to check 'disk->format > 0' to work correctly for AUTO, as
well as tweak domain_conf to not apply driver capability defaults to dir
or network protocols (formats only make sense on files and block
devices).  Most of the rest of this patch did it correctly.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


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