When an image has no backing file, using VIR_STORAGE_FILE_AUTO for its type is a bit confusing. Additionally, a future patch would like to reserve a default value for the case of no file type specified in the XML, but different from the current use of -1 to imply probing, since probing is not always safe. Also, a couple of file types were missing compared to supported code: libxl supports 'vhd', and qemu supports 'fat' for directories passed through as a file system. * src/util/storage_file.h (virStorageFileFormat): Add VIR_STORAGE_FILE_NONE, VIR_STORAGE_FILE_FAT, VIR_STORAGE_FILE_VHD. * src/util/storage_file.c (virStorageFileMatchesVersion): Match documentation when version probing not supported. (cowGetBackingStore, qcowXGetBackingStore, qcow1GetBackingStore) (qedGetBackingStore, virStorageFileGetMetadataFromBuf) (virStorageFileGetMetadataFromFD): Take NONE into account. * src/conf/domain_conf.c (virDomainDiskDefForeachPath): Likewise. * src/qemu/qemu_driver.c (qemuDomainGetBlockInfo): Likewise. * src/conf/storage_conf.c (virStorageVolumeFormatFromString): New function. (poolTypeInfo): Use it. --- src/conf/domain_conf.c | 2 +- src/conf/storage_conf.c | 15 ++++++++--- src/qemu/qemu_driver.c | 2 +- src/util/storage_file.c | 69 +++++++++++++++++++++++++++++++++++-------------- src/util/storage_file.h | 8 ++++-- 5 files changed, 69 insertions(+), 27 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 33e1e7f..77dc688 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -14577,7 +14577,7 @@ int virDomainDiskDefForeachPath(virDomainDiskDefPtr disk, if (STREQ(formatStr, "aio")) formatStr = "raw"; /* Xen compat */ - if ((format = virStorageFileFormatTypeFromString(formatStr)) < 0) { + if ((format = virStorageFileFormatTypeFromString(formatStr)) <= 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unknown disk format '%s' for %s"), disk->driverType, disk->src); diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 4daf059..5d9e61a 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -135,6 +135,15 @@ struct _virStoragePoolTypeInfo { virStorageVolOptions volOptions; }; +static int +virStorageVolumeFormatFromString(const char *format) +{ + int ret = virStorageFileFormatTypeFromString(format); + if (ret == VIR_STORAGE_FILE_NONE) + return -1; + return ret; +} + static virStoragePoolTypeInfo poolTypeInfo[] = { { .poolType = VIR_STORAGE_POOL_LOGICAL, .poolOptions = { @@ -148,7 +157,7 @@ static virStoragePoolTypeInfo poolTypeInfo[] = { { .poolType = VIR_STORAGE_POOL_DIR, .volOptions = { .defaultFormat = VIR_STORAGE_FILE_RAW, - .formatFromString = virStorageFileFormatTypeFromString, + .formatFromString = virStorageVolumeFormatFromString, .formatToString = virStorageFileFormatTypeToString, }, }, @@ -161,7 +170,7 @@ static virStoragePoolTypeInfo poolTypeInfo[] = { }, .volOptions = { .defaultFormat = VIR_STORAGE_FILE_RAW, - .formatFromString = virStorageFileFormatTypeFromString, + .formatFromString = virStorageVolumeFormatFromString, .formatToString = virStorageFileFormatTypeToString, }, }, @@ -175,7 +184,7 @@ static virStoragePoolTypeInfo poolTypeInfo[] = { }, .volOptions = { .defaultFormat = VIR_STORAGE_FILE_RAW, - .formatFromString = virStorageFileFormatTypeFromString, + .formatFromString = virStorageVolumeFormatFromString, .formatToString = virStorageFileFormatTypeToString, }, }, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 97ad23e..86cb297 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9189,7 +9189,7 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom, /* Probe for magic formats */ if (disk->driverType) { - if ((format = virStorageFileFormatTypeFromString(disk->driverType)) < 0) { + if ((format = virStorageFileFormatTypeFromString(disk->driverType)) <= 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unknown disk format %s for %s"), disk->driverType, disk->src); diff --git a/src/util/storage_file.c b/src/util/storage_file.c index eea760c..3d1b7cf 100644 --- a/src/util/storage_file.c +++ b/src/util/storage_file.c @@ -1,7 +1,7 @@ /* * storage_file.c: file utility functions for FS storage backend * - * Copyright (C) 2007-2011 Red Hat, Inc. + * Copyright (C) 2007-2012 Red Hat, Inc. * Copyright (C) 2007-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -45,9 +45,11 @@ VIR_ENUM_IMPL(virStorageFileFormat, VIR_STORAGE_FILE_LAST, + "none", "raw", "dir", "bochs", "cloop", "cow", "dmg", "iso", - "qcow", "qcow2", "qed", "vmdk", "vpc") + "qcow", "qcow2", "qed", "vmdk", "vpc", + "fat", "vhd") enum lv_endian { LV_LITTLE_ENDIAN = 1, /* 1234 */ @@ -123,8 +125,12 @@ qedGetBackingStore(char **, int *, const unsigned char *, size_t); static struct FileTypeInfo const fileTypeInfo[] = { - [VIR_STORAGE_FILE_RAW] = { NULL, NULL, LV_LITTLE_ENDIAN, -1, 0, 0, 0, 0, 0, NULL }, - [VIR_STORAGE_FILE_DIR] = { NULL, NULL, LV_LITTLE_ENDIAN, -1, 0, 0, 0, 0, 0, NULL }, + [VIR_STORAGE_FILE_NONE] = { NULL, NULL, LV_LITTLE_ENDIAN, + -1, 0, 0, 0, 0, 0, NULL }, + [VIR_STORAGE_FILE_RAW] = { NULL, NULL, LV_LITTLE_ENDIAN, + -1, 0, 0, 0, 0, 0, NULL }, + [VIR_STORAGE_FILE_DIR] = { NULL, NULL, LV_LITTLE_ENDIAN, + -1, 0, 0, 0, 0, 0, NULL }, [VIR_STORAGE_FILE_BOCHS] = { /*"Bochs Virtual HD Image", */ /* Untested */ NULL, NULL, @@ -180,6 +186,11 @@ static struct FileTypeInfo const fileTypeInfo[] = { LV_BIG_ENDIAN, 12, 0x10000, 8 + 4 + 4 + 8 + 4 + 4 + 2 + 2 + 4, 8, 1, -1, NULL }, + /* Not direct file formats, but used for various drivers */ + [VIR_STORAGE_FILE_FAT] = { NULL, NULL, LV_LITTLE_ENDIAN, + -1, 0, 0, 0, 0, 0, NULL }, + [VIR_STORAGE_FILE_VHD] = { NULL, NULL, LV_LITTLE_ENDIAN, + -1, 0, 0, 0, 0, 0, NULL }, }; verify(ARRAY_CARDINALITY(fileTypeInfo) == VIR_STORAGE_FILE_LAST); @@ -195,8 +206,10 @@ cowGetBackingStore(char **res, if (buf_size < 4+4+ COW_FILENAME_MAXLEN) return BACKING_STORE_INVALID; - if (buf[4+4] == '\0') /* cow_header_v2.backing_file[0] */ + if (buf[4+4] == '\0') { /* cow_header_v2.backing_file[0] */ + *format = VIR_STORAGE_FILE_NONE; return BACKING_STORE_OK; + } *res = strndup ((const char*)buf + 4+4, COW_FILENAME_MAXLEN); if (*res == NULL) { @@ -298,8 +311,11 @@ qcowXGetBackingStore(char **res, | (buf[QCOWX_HDR_BACKING_FILE_SIZE+1] << 16) | (buf[QCOWX_HDR_BACKING_FILE_SIZE+2] << 8) | buf[QCOWX_HDR_BACKING_FILE_SIZE+3]); /* QCowHeader.backing_file_size */ - if (size == 0) + if (size == 0) { + if (format) + *format = VIR_STORAGE_FILE_NONE; return BACKING_STORE_OK; + } if (offset + size > buf_size || offset + size < offset) return BACKING_STORE_INVALID; if (size + 1 == 0) @@ -335,8 +351,12 @@ qcowXGetBackingStore(char **res, * between the end of the header (QCOW2_HDR_TOTAL_SIZE) * and the start of the backingStoreName (offset) */ - if (isQCow2 && format) - qcow2GetBackingStoreFormat(format, buf, buf_size, QCOW2_HDR_TOTAL_SIZE, offset); + if (isQCow2 && format) { + qcow2GetBackingStoreFormat(format, buf, buf_size, QCOW2_HDR_TOTAL_SIZE, + offset); + if (*format <= VIR_STORAGE_FILE_NONE) + return BACKING_STORE_INVALID; + } return BACKING_STORE_OK; } @@ -348,10 +368,15 @@ qcow1GetBackingStore(char **res, const unsigned char *buf, size_t buf_size) { + int ret; + /* QCow1 doesn't have the extensions capability * used to store backing format */ *format = VIR_STORAGE_FILE_AUTO; - return qcowXGetBackingStore(res, NULL, buf, buf_size, false); + ret = qcowXGetBackingStore(res, NULL, buf, buf_size, false); + if (ret == 0 && *buf == '\0') + *format = VIR_STORAGE_FILE_NONE; + return ret; } static int @@ -401,6 +426,7 @@ vmdk4GetBackingStore(char **res, desc[len] = '\0'; start = strstr(desc, prefix); if (start == NULL) { + *format = VIR_STORAGE_FILE_NONE; ret = BACKING_STORE_OK; goto cleanup; } @@ -411,6 +437,7 @@ vmdk4GetBackingStore(char **res, goto cleanup; } if (end == start) { + *format = VIR_STORAGE_FILE_NONE; ret = BACKING_STORE_OK; goto cleanup; } @@ -464,8 +491,10 @@ qedGetBackingStore(char **res, if (buf_size < QED_HDR_FEATURES_OFFSET+8) return BACKING_STORE_INVALID; flags = qedGetHeaderULL(buf + QED_HDR_FEATURES_OFFSET); - if (!(flags & QED_F_BACKING_FILE)) + if (!(flags & QED_F_BACKING_FILE)) { + *format = VIR_STORAGE_FILE_NONE; return BACKING_STORE_OK; + } /* Parse the backing file */ if (buf_size < QED_HDR_BACKING_FILE_OFFSET+8) @@ -485,12 +514,10 @@ qedGetBackingStore(char **res, memcpy(*res, buf + offset, size); (*res)[size] = '\0'; - if (format) { - if (flags & QED_F_BACKING_FORMAT_NO_PROBE) - *format = virStorageFileFormatTypeFromString("raw"); - else - *format = VIR_STORAGE_FILE_AUTO_SAFE; - } + if (flags & QED_F_BACKING_FORMAT_NO_PROBE) + *format = virStorageFileFormatTypeFromString("raw"); + else + *format = VIR_STORAGE_FILE_AUTO_SAFE; return BACKING_STORE_OK; } @@ -564,7 +591,7 @@ virStorageFileMatchesVersion(int format, /* Validate version number info */ if (fileTypeInfo[format].versionOffset == -1) - return true; + return false; if ((fileTypeInfo[format].versionOffset + 4) > buflen) return false; @@ -607,7 +634,9 @@ virStorageFileGetMetadataFromBuf(int format, /* XXX we should consider moving virStorageBackendUpdateVolInfo * code into this method, for non-magic files */ - if (!fileTypeInfo[format].magic) { + if (format <= VIR_STORAGE_FILE_NONE || + format >= VIR_STORAGE_FILE_LAST || + !fileTypeInfo[format].magic) { return 0; } @@ -682,7 +711,7 @@ virStorageFileGetMetadataFromBuf(int format, meta->backingStoreFormat = backingFormat; } else { meta->backingStore = NULL; - meta->backingStoreFormat = VIR_STORAGE_FILE_AUTO; + meta->backingStoreFormat = VIR_STORAGE_FILE_NONE; } } @@ -867,7 +896,7 @@ virStorageFileGetMetadataFromFD(const char *path, if (format == VIR_STORAGE_FILE_AUTO) format = virStorageFileProbeFormatFromBuf(path, head, len); - if (format < 0 || + if (format <= VIR_STORAGE_FILE_NONE || format >= VIR_STORAGE_FILE_LAST) { virReportSystemError(EINVAL, _("unknown storage file format %d"), format); diff --git a/src/util/storage_file.h b/src/util/storage_file.h index 2e76b1b..683ec73 100644 --- a/src/util/storage_file.h +++ b/src/util/storage_file.h @@ -1,7 +1,7 @@ /* * storage_file.c: file utility functions for FS storage backend * - * Copyright (C) 2007-2009 Red Hat, Inc. + * Copyright (C) 2007-2009, 2012 Red Hat, Inc. * Copyright (C) 2007-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -29,7 +29,8 @@ enum virStorageFileFormat { VIR_STORAGE_FILE_AUTO_SAFE = -2, VIR_STORAGE_FILE_AUTO = -1, - VIR_STORAGE_FILE_RAW = 0, + VIR_STORAGE_FILE_NONE = 0, + VIR_STORAGE_FILE_RAW, VIR_STORAGE_FILE_DIR, VIR_STORAGE_FILE_BOCHS, VIR_STORAGE_FILE_CLOOP, @@ -41,6 +42,9 @@ enum virStorageFileFormat { VIR_STORAGE_FILE_QED, VIR_STORAGE_FILE_VMDK, VIR_STORAGE_FILE_VPC, + VIR_STORAGE_FILE_FAT, + VIR_STORAGE_FILE_VHD, + VIR_STORAGE_FILE_LAST, }; -- 1.7.11.4

At one point, the code passed through arbitrary strings for file formats, which supposedly lets qemu handle a new file type even before libvirt has been taught to handle it. However, to properly label files, libvirt has to learn the file type anyway, so we might as well make our life easier by only accepting file types that we are prepared to handle. This patch lets the RNG validation ensure that only known strings are let through. * docs/schemas/domaincommon.rng (driverFormat): Limit to list of supported strings. * docs/schemas/domainsnapshot.rng (driver): Likewise. --- docs/schemas/domaincommon.rng | 27 ++++++++++++++++++++++++--- docs/schemas/domainsnapshot.rng | 2 +- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index f47fdad..1a3a9e8 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1188,11 +1188,32 @@ <ref name="genericName"/> </attribute> <optional> - <attribute name="type"> - <ref name="genericName"/> + <attribute name='type'> + <choice> + <ref name='diskFormat'/> + <value>aio</value> <!-- back-compat for 'raw' --> + </choice> </attribute> </optional> </define> + <define name='diskFormat'> + <choice> + <value>raw</value> + <value>dir</value> + <value>bochs</value> + <value>cloop</value> + <value>cow</value> + <value>dmg</value> + <value>iso</value> + <value>qcow</value> + <value>qcow2</value> + <value>qed</value> + <value>vmdk</value> + <value>vpc</value> + <value>fat</value> + <value>vhd</value> + </choice> + </define> <define name="driverCache"> <attribute name="cache"> <choice> @@ -3319,7 +3340,7 @@ </attribute> <optional> <attribute name='format'> - <ref name="genericName"/> + <ref name='diskFormat'/> </attribute> </optional> <optional> diff --git a/docs/schemas/domainsnapshot.rng b/docs/schemas/domainsnapshot.rng index 0ef0631..ecaafe9 100644 --- a/docs/schemas/domainsnapshot.rng +++ b/docs/schemas/domainsnapshot.rng @@ -105,7 +105,7 @@ <element name='driver'> <optional> <attribute name='type'> - <ref name='genericName'/> + <ref name='diskFormat'/> </attribute> </optional> <empty/> -- 1.7.11.4

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. --- src/conf/domain_conf.c | 60 ++++++++++++++++---------------- src/conf/domain_conf.h | 4 +-- src/libxl/libxl_conf.c | 42 +++++++++++++---------- src/libxl/libxl_driver.c | 6 +--- src/qemu/qemu_command.c | 18 +++++----- src/qemu/qemu_domain.c | 7 ++-- src/qemu/qemu_driver.c | 89 ++++++++++++++++-------------------------------- src/qemu/qemu_hotplug.c | 9 ++--- src/vbox/vbox_tmpl.c | 6 ++-- src/xenxs/xen_sxpr.c | 26 +++++++++----- src/xenxs/xen_xm.c | 29 ++++++++++------ 11 files changed, 145 insertions(+), 151 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a209d26..64013bb 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -964,9 +964,7 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def) VIR_FREE(def->src); VIR_FREE(def->dst); VIR_FREE(def->driverName); - VIR_FREE(def->driverType); VIR_FREE(def->mirror); - VIR_FREE(def->mirrorFormat); VIR_FREE(def->auth.username); VIR_FREE(def->wwn); if (def->auth.secretType == VIR_DOMAIN_DISK_SECRET_TYPE_USAGE) @@ -4107,12 +4105,8 @@ virDomainDiskDefParseXML(virCapsPtr caps, authUsername = NULL; def->driverName = driverName; driverName = NULL; - def->driverType = driverType; - driverType = NULL; def->mirror = mirror; mirror = NULL; - def->mirrorFormat = mirrorFormat; - mirrorFormat = NULL; def->mirroring = mirroring; def->encryption = encryption; encryption = NULL; @@ -4121,23 +4115,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; + } if (!def->driverName && caps->defaultDiskDriverName && !(def->driverName = strdup(caps->defaultDiskDriverName))) goto no_memory; - - if (def->mirror && !def->mirrorFormat && - caps->defaultDiskDriverType && - !(def->mirrorFormat = strdup(virStorageFileFormatTypeToString( - caps->defaultDiskDriverType)))) - goto no_memory; + if (mirrorFormat) { + def->mirrorFormat = virStorageFileFormatTypeFromString(mirrorFormat); + if (def->mirrorFormat <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown mirror format value '%s'"), + driverType); + goto error; + } + } else if (def->mirror) { + def->mirrorFormat = caps->defaultDiskDriverType; + } if (def->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && virDomainDiskDefAssignAddress(caps, def) < 0) @@ -11665,13 +11670,14 @@ virDomainDiskDefFormat(virBufferPtr buf, virDomainSnapshotLocationTypeToString(def->snapshot)); virBufferAddLit(buf, ">\n"); - if (def->driverName || def->driverType || def->cachemode || + if (def->driverName || def->format > 0 || def->cachemode || def->ioeventfd || def->event_idx || def->copy_on_read) { virBufferAddLit(buf, " <driver"); if (def->driverName) virBufferAsprintf(buf, " name='%s'", def->driverName); - if (def->driverType) - virBufferAsprintf(buf, " type='%s'", def->driverType); + if (def->format > 0) + virBufferAsprintf(buf, " type='%s'", + virStorageFileFormatTypeToString(def->format)); if (def->cachemode) virBufferAsprintf(buf, " cache='%s'", cachemode); if (def->error_policy) @@ -11783,7 +11789,8 @@ virDomainDiskDefFormat(virBufferPtr buf, if (def->mirror && !(flags & VIR_DOMAIN_XML_INACTIVE)) { virBufferEscapeString(buf, " <mirror file='%s'", def->mirror); if (def->mirrorFormat) - virBufferAsprintf(buf, " format='%s'", def->mirrorFormat); + virBufferAsprintf(buf, " format='%s'", + virStorageFileFormatTypeToString(def->mirrorFormat)); if (def->mirroring) virBufferAddLit(buf, " ready='yes'"); virBufferAddLit(buf, "/>\n"); @@ -14576,15 +14583,8 @@ int virDomainDiskDefForeachPath(virDomainDiskDefPtr disk, return ret; } - if (disk->driverType) { - const char *formatStr = disk->driverType; - - if ((format = virStorageFileFormatTypeFromString(formatStr)) <= 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unknown disk format '%s' for %s"), - disk->driverType, disk->src); - goto cleanup; - } + if (disk->format > 0) { + format = disk->format; } else { if (allowProbing) { format = VIR_STORAGE_FILE_AUTO; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 14dead3..f0b43c3 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -563,10 +563,10 @@ struct _virDomainDiskDef { } secret; } auth; char *driverName; - char *driverType; + int format; /* enum virStorageFileFormat */ char *mirror; - char *mirrorFormat; + int mirrorFormat; /* enum virStorageFileFormat */ bool mirroring; struct { diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index b988fa7..fa931c3 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -41,6 +41,7 @@ #include "capabilities.h" #include "libxl_driver.h" #include "libxl_conf.h" +#include "storage_file.h" #define VIR_FROM_THIS VIR_FROM_LIBXL @@ -505,25 +506,30 @@ libxlMakeDisk(virDomainDefPtr def, virDomainDiskDefPtr l_disk, if (l_disk->driverName) { if (STREQ(l_disk->driverName, "tap") || STREQ(l_disk->driverName, "tap2")) { - if (l_disk->driverType) { - if (STREQ(l_disk->driverType, "qcow")) { - x_disk->format = DISK_FORMAT_QCOW; - x_disk->backend = DISK_BACKEND_QDISK; - } else if (STREQ(l_disk->driverType, "qcow2")) { - x_disk->format = DISK_FORMAT_QCOW2; - x_disk->backend = DISK_BACKEND_QDISK; - } else if (STREQ(l_disk->driverType, "vhd")) { - x_disk->format = DISK_FORMAT_VHD; - x_disk->backend = DISK_BACKEND_TAP; - } else if (STREQ(l_disk->driverType, "aio") || - STREQ(l_disk->driverType, "raw")) { - x_disk->format = DISK_FORMAT_RAW; - x_disk->backend = DISK_BACKEND_TAP; - } - } else { + switch (l_disk->format) { + case VIR_STORAGE_FILE_QCOW: + x_disk->format = DISK_FORMAT_QCOW; + x_disk->backend = DISK_BACKEND_QDISK; + break; + case VIR_STORAGE_FILE_QCOW2: + x_disk->format = DISK_FORMAT_QCOW2; + x_disk->backend = DISK_BACKEND_QDISK; + break; + case VIR_STORAGE_FILE_VHD: + x_disk->format = DISK_FORMAT_VHD; + x_disk->backend = DISK_BACKEND_TAP; + break; + case VIR_STORAGE_FILE_NONE: /* No subtype specified, default to raw/tap */ - x_disk->format = DISK_FORMAT_RAW; - x_disk->backend = DISK_BACKEND_TAP; + case VIR_STORAGE_FILE_RAW: + x_disk->format = DISK_FORMAT_RAW; + x_disk->backend = DISK_BACKEND_TAP; + break; + default: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("libxenlight does not support disk driver %s"), + virStorageFileFormatTypeToString(l_disk->format)); + return -1; } } else if (STREQ(l_disk->driverName, "file")) { x_disk->format = DISK_FORMAT_RAW; diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 6fe284f..f4e9aa6 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -3202,11 +3202,7 @@ libxlDomainUpdateDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev) orig->driverName = disk->driverName; disk->driverName = NULL; } - if (disk->driverType) { - VIR_FREE(orig->driverType); - orig->driverType = disk->driverType; - disk->driverType = NULL; - } + orig->format = disk->format; disk->src = NULL; break; default: diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 20730a9..c0a8d2a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -43,6 +43,7 @@ #include "virnetdevtap.h" #include "base64.h" #include "device_conf.h" +#include "storage_file.h" #include <sys/utsname.h> #include <sys/stat.h> @@ -2128,11 +2129,10 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, disk->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN)) { 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) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unsupported disk driver type for '%s'"), - disk->driverType); + virStorageFileFormatTypeToString(disk->format)); goto error; } if (!disk->readonly) { @@ -2229,10 +2229,11 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, _("transient disks not supported yet")); goto error; } - if (disk->driverType && *disk->driverType != '\0' && + if (disk->format > 0 && disk->type != VIR_DOMAIN_DISK_TYPE_DIR && qemuCapsGet(caps, QEMU_CAPS_DRIVE_FORMAT)) - virBufferAsprintf(&opt, ",format=%s", disk->driverType); + virBufferAsprintf(&opt, ",format=%s", + virStorageFileFormatTypeToString(disk->format)); /* generate geometry command string */ if (disk->geometry.cylinders > 0 && @@ -5197,11 +5198,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) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unsupported disk driver type for '%s'"), - disk->driverType); + virStorageFileFormatTypeToString(disk->format)); goto error; } if (!disk->readonly) { @@ -7008,7 +7008,7 @@ qemuParseCommandLineDisk(virCapsPtr caps, virReportOOMError(); goto cleanup; } - def->driverType = values[i]; + def->format = virStorageFileFormatTypeFromString(values[i]); values[i] = NULL; } else if (STREQ(keywords[i], "cache")) { if (STREQ(values[i], "off") || diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 17ae3b9..98d85e8 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -36,6 +36,7 @@ #include "virfile.h" #include "domain_event.h" #include "virtime.h" +#include "storage_file.h" #include <sys/time.h> #include <fcntl.h> @@ -1414,7 +1415,7 @@ void qemuDomainObjCheckDiskTaint(struct qemud_driver *driver, virDomainDiskDefPtr disk, int logFD) { - if (!disk->driverType && + if ((!disk->format || disk->format == VIR_STORAGE_FILE_AUTO) && driver->allowDiskFormatProbing) qemuDomainObjTaint(driver, obj, VIR_DOMAIN_TAINT_DISK_PROBING, logFD); @@ -1666,8 +1667,8 @@ qemuDomainSnapshotForEachQcow2Raw(struct qemud_driver *driver, for (i = 0; i < ndisks; i++) { /* FIXME: we also need to handle LVM here */ if (def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_DISK) { - if (!def->disks[i]->driverType || - STRNEQ(def->disks[i]->driverType, "qcow2")) { + if (def->disks[i]->format > 0 && + def->disks[i]->format != VIR_STORAGE_FILE_QCOW2) { if (try_all) { /* Continue on even in the face of error, since other * disks in this VM may have the same snapshot name. diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 94cda54..15eda52 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6221,11 +6221,8 @@ qemuDomainUpdateDeviceConfig(qemuCapsPtr caps, orig->driverName = disk->driverName; disk->driverName = NULL; } - if (disk->driverType) { - VIR_FREE(orig->driverType); - orig->driverType = disk->driverType; - disk->driverType = NULL; - } + if (disk->format) + orig->format = disk->format; disk->src = NULL; break; @@ -9188,13 +9185,8 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom, } /* Probe for magic formats */ - if (disk->driverType) { - if ((format = virStorageFileFormatTypeFromString(disk->driverType)) <= 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unknown disk format %s for %s"), - disk->driverType, disk->src); - goto cleanup; - } + if (disk->format) { + format = disk->format; } else { if (driver->allowDiskFormatProbing) { if ((format = virStorageFileProbeFormat(disk->src)) < 0) @@ -10366,7 +10358,7 @@ qemuDomainSnapshotIsAllowed(virDomainObjPtr vm) if ((disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) || (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK && - STRNEQ_NULLABLE(disk->driverType, "qcow2"))) { + disk->format > 0 && disk->format != VIR_STORAGE_FILE_QCOW2)) { virReportError(VIR_ERR_OPERATION_INVALID, _("Disk '%s' does not support snapshotting"), disk->src); @@ -10560,13 +10552,14 @@ qemuDomainSnapshotDiskPrepare(virDomainObjPtr vm, virDomainSnapshotDefPtr def, disk->name); goto cleanup; } - if (!vm->def->disks[i]->driverType || - STRNEQ(vm->def->disks[i]->driverType, "qcow2")) { + if (vm->def->disks[i]->format > 0 && + vm->def->disks[i]->format != VIR_STORAGE_FILE_QCOW2) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("internal snapshot for disk %s unsupported " "for storage type %s"), disk->name, - NULLSTR(vm->def->disks[i]->driverType)); + virStorageFileFormatTypeToString( + vm->def->disks[i]->format)); goto cleanup; } found = true; @@ -10653,13 +10646,12 @@ qemuDomainSnapshotCreateSingleDiskActive(struct qemud_driver *driver, qemuDomainObjPrivatePtr priv = vm->privateData; char *device = NULL; char *source = NULL; - char *driverType = NULL; + int format = VIR_STORAGE_FILE_NONE; char *persistSource = NULL; - char *persistDriverType = NULL; int ret = -1; int fd = -1; char *origsrc = NULL; - char *origdriver = NULL; + int origdriver; bool need_unlink = false; if (snap->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) { @@ -10668,14 +10660,19 @@ qemuDomainSnapshotCreateSingleDiskActive(struct qemud_driver *driver, return -1; } + if (snap->driverType) { + format = virStorageFileFormatTypeFromString(snap->driverType); + if (format <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown driver type %s"), snap->driverType); + goto cleanup; + } + } + if (virAsprintf(&device, "drive-%s", disk->info.alias) < 0 || !(source = strdup(snap->file)) || - (STRNEQ_NULLABLE(disk->driverType, snap->driverType) && - !(driverType = strdup(snap->driverType))) || (persistDisk && - (!(persistSource = strdup(source)) || - (STRNEQ_NULLABLE(persistDisk->driverType, snap->driverType) && - !(persistDriverType = strdup(snap->driverType)))))) { + !(persistSource = strdup(source)))) { virReportOOMError(); goto cleanup; } @@ -10692,8 +10689,8 @@ qemuDomainSnapshotCreateSingleDiskActive(struct qemud_driver *driver, origsrc = disk->src; disk->src = source; - origdriver = disk->driverType; - disk->driverType = (char *) "raw"; /* Don't want to probe backing files */ + origdriver = disk->format; + disk->format = VIR_STORAGE_FILE_RAW; /* Don't want to probe backing files */ if (virDomainLockDiskAttach(driver->lockManager, vm, disk) < 0) goto cleanup; @@ -10713,8 +10710,7 @@ qemuDomainSnapshotCreateSingleDiskActive(struct qemud_driver *driver, disk->src = origsrc; origsrc = NULL; - disk->driverType = origdriver; - origdriver = NULL; + disk->format = origdriver; /* create the actual snapshot */ ret = qemuMonitorDiskSnapshot(priv->mon, actions, device, source, @@ -10728,34 +10724,24 @@ qemuDomainSnapshotCreateSingleDiskActive(struct qemud_driver *driver, VIR_FREE(disk->src); disk->src = source; source = NULL; - if (driverType) { - VIR_FREE(disk->driverType); - disk->driverType = driverType; - driverType = NULL; - } + disk->format = format; if (persistDisk) { VIR_FREE(persistDisk->src); persistDisk->src = persistSource; persistSource = NULL; - if (persistDriverType) { - VIR_FREE(persistDisk->driverType); - persistDisk->driverType = persistDriverType; - persistDriverType = NULL; - } + persistDisk->format = format; } cleanup: if (origsrc) { disk->src = origsrc; - disk->driverType = origdriver; + disk->format = origdriver; } if (need_unlink && unlink(source)) VIR_WARN("unable to unlink just-created %s", source); VIR_FREE(device); VIR_FREE(source); - VIR_FREE(driverType); VIR_FREE(persistSource); - VIR_FREE(persistDriverType); return ret; } @@ -10772,17 +10758,12 @@ qemuDomainSnapshotUndoSingleDiskActive(struct qemud_driver *driver, bool need_unlink) { char *source = NULL; - char *driverType = NULL; char *persistSource = NULL; - char *persistDriverType = NULL; struct stat st; if (!(source = strdup(origdisk->src)) || - (origdisk->driverType && - !(driverType = strdup(origdisk->driverType))) || (persistDisk && - (!(persistSource = strdup(source)) || - (driverType && !(persistDriverType = strdup(driverType)))))) { + !(persistSource = strdup(source)))) { virReportOOMError(); goto cleanup; } @@ -10802,27 +10783,17 @@ qemuDomainSnapshotUndoSingleDiskActive(struct qemud_driver *driver, VIR_FREE(disk->src); disk->src = source; source = NULL; - VIR_FREE(disk->driverType); - if (driverType) { - disk->driverType = driverType; - driverType = NULL; - } + disk->format = origdisk->format; if (persistDisk) { VIR_FREE(persistDisk->src); persistDisk->src = persistSource; persistSource = NULL; - VIR_FREE(persistDisk->driverType); - if (persistDriverType) { - persistDisk->driverType = persistDriverType; - persistDriverType = NULL; - } + persistDisk->format = origdisk->format; } cleanup: VIR_FREE(source); - VIR_FREE(driverType); VIR_FREE(persistSource); - VIR_FREE(persistDriverType); } /* The domain is expected to be locked and active. */ diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index a738b19..8f86fb2 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -44,6 +44,7 @@ #include "virnetdevbridge.h" #include "virnetdevtap.h" #include "device_conf.h" +#include "storage_file.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -105,10 +106,10 @@ int qemuDomainChangeEjectableMedia(struct qemud_driver *driver, if (disk->src) { const char *format = NULL; if (disk->type != VIR_DOMAIN_DISK_TYPE_DIR) { - if (disk->driverType) - format = disk->driverType; - else if (origdisk->driverType) - format = origdisk->driverType; + if (disk->format > 0) + format = virStorageFileFormatTypeToString(disk->format); + else if (origdisk->format > 0) + format = virStorageFileFormatTypeToString(origdisk->format); } ret = qemuMonitorChangeMedia(priv->mon, driveAlias, diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 29b776d..1030146 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -3846,7 +3846,8 @@ vboxAttachDrives(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) VIR_DEBUG("disk(%d) src: %s", i, def->disks[i]->src); VIR_DEBUG("disk(%d) dst: %s", i, def->disks[i]->dst); VIR_DEBUG("disk(%d) driverName: %s", i, def->disks[i]->driverName); - VIR_DEBUG("disk(%d) driverType: %s", i, def->disks[i]->driverType); + VIR_DEBUG("disk(%d) driverType: %s", i, + virStorageFileFormatTypeToString(def->disks[i]->format)); VIR_DEBUG("disk(%d) cachemode: %d", i, def->disks[i]->cachemode); VIR_DEBUG("disk(%d) readonly: %s", i, (def->disks[i]->readonly ? "True" : "False")); @@ -4125,7 +4126,8 @@ vboxAttachDrives(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) VIR_DEBUG("disk(%d) src: %s", i, def->disks[i]->src); VIR_DEBUG("disk(%d) dst: %s", i, def->disks[i]->dst); VIR_DEBUG("disk(%d) driverName: %s", i, def->disks[i]->driverName); - VIR_DEBUG("disk(%d) driverType: %s", i, def->disks[i]->driverType); + VIR_DEBUG("disk(%d) driverType: %s", i, + virStorageFileFormatTypeToString(def->disks[i]->format)); VIR_DEBUG("disk(%d) cachemode: %d", i, def->disks[i]->cachemode); VIR_DEBUG("disk(%d) readonly: %s", i, (def->disks[i]->readonly ? "True" : "False")); diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c index 23c138b..efc5d51 100644 --- a/src/xenxs/xen_sxpr.c +++ b/src/xenxs/xen_sxpr.c @@ -36,6 +36,7 @@ #include "count-one-bits.h" #include "xenxs_private.h" #include "xen_sxpr.h" +#include "storage_file.h" /* Get a domain id from a S-expression string */ int xenGetDomIdFromSxprString(const char *sexpr, int xendConfigVersion) @@ -427,6 +428,8 @@ xenParseSxprDisks(virDomainDefPtr def, if (STREQ (disk->driverName, "tap") || STREQ (disk->driverName, "tap2")) { + char *driverType = NULL; + offset = strchr(src, ':'); if (!offset) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -434,17 +437,19 @@ xenParseSxprDisks(virDomainDefPtr def, goto error; } - if (VIR_ALLOC_N(disk->driverType, (offset-src)+1)< 0) + if (!(driverType = strndup(src, offset - src))) goto no_memory; - if (virStrncpy(disk->driverType, src, offset-src, - (offset-src)+1) == NULL) { + if (STREQ(driverType, "aio")) + disk->format = VIR_STORAGE_FILE_RAW; + else + disk->format = + virStorageFileFormatTypeFromString(driverType); + VIR_FREE(driverType); + if (disk->format <= 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Driver type %s too big for destination"), - src); + _("Unknown driver type %s"), src); goto error; } - if (STREQ(disk->driverType, "aio")) - memcpy(disk->driverType, "raw", strlen("raw")); src = offset + 1; /* Its possible to use blktap driver for block devs @@ -1833,9 +1838,12 @@ xenFormatSxprDisk(virDomainDiskDefPtr def, if (def->driverName) { if (STREQ(def->driverName, "tap") || STREQ(def->driverName, "tap2")) { - const char *type = def->driverType ? def->driverType : "aio"; - if (STREQ(type, "raw")) + const char *type; + + if (!def->format || def->format == VIR_STORAGE_FILE_RAW) type = "aio"; + else + type = virStorageFileFormatTypeToString(def->format); virBufferEscapeSexpr(buf, "(uname '%s:", def->driverName); virBufferEscapeSexpr(buf, "%s:", type); virBufferEscapeSexpr(buf, "%s')", def->src); diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c index 41e6744..2e560a6 100644 --- a/src/xenxs/xen_xm.c +++ b/src/xenxs/xen_xm.c @@ -37,6 +37,7 @@ #include "xen_xm.h" #include "xen_sxpr.h" #include "domain_conf.h" +#include "storage_file.h" /* Convenience method to grab a long int from the config file object */ static int xenXMConfigGetBool(virConfPtr conf, @@ -554,20 +555,25 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, /* And the sub-type for tap:XXX: type */ if (disk->driverName && STREQ(disk->driverName, "tap")) { + char *driverType; + if (!(tmp = strchr(disk->src, ':'))) goto skipdisk; - if (VIR_ALLOC_N(disk->driverType, (tmp - disk->src) + 1) < 0) + + if (!(driverType = strndup(disk->src, tmp - disk->src))) goto no_memory; - if (virStrncpy(disk->driverType, disk->src, - (tmp - disk->src), - (tmp - disk->src) + 1) == NULL) { + if (STREQ(driverType, "aio")) + disk->format = VIR_STORAGE_FILE_RAW; + else + disk->format = + virStorageFileFormatTypeFromString(driverType); + VIR_FREE(driverType); + if (disk->format <= 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Driver type %s too big for destination"), + _("Unknown driver type %s"), disk->src); goto cleanup; } - if (STREQ(disk->driverType, "aio")) - memcpy(disk->driverType, "raw", strlen("raw")); /* Strip the prefix we found off the source file name */ memmove(disk->src, disk->src+(tmp-disk->src)+1, @@ -1204,10 +1210,13 @@ static int xenFormatXMDisk(virConfValuePtr list, virConfValuePtr val, tmp; if(disk->src) { - if (disk->driverName) { - const char *type = disk->driverType ? disk->driverType : "aio"; - if (STREQ(type, "raw")) + if (disk->format) { + const char *type; + + if (disk->format == VIR_STORAGE_FILE_RAW) type = "aio"; + else + type = virStorageFileFormatTypeToString(disk->format); virBufferAsprintf(&buf, "%s:", disk->driverName); if (STREQ(disk->driverName, "tap")) virBufferAsprintf(&buf, "%s:", type); -- 1.7.11.4