[libvirt] [PATCH 00/22] Misc refactors and cleanups leading to gluster snapshot support
Peter Krempa (22): conf: Implement virStorageVolType enum helper functions test: Implement fake storage pool driver in qemuxml2argv test qemuxml2argv: Add test to verify correct usage of disk type="volume" qemuxml2argv: Add test for disk type='volume' with iSCSI pools qemu: Refactor qemuTranslatePool source qemu: Split out formatting of network disk source URI qemu: Simplify call pattern of qemuBuildDriveURIString qemu: Use qemuBuildNetworkDriveURI to handle http/ftp and friends qemu: Migrate sheepdog source generation into common function qemu: Split out NBD command generation qemu: Unify formatting of RBD sources qemu: Refactor disk source string formatting conf: Support disk source formatting without needing a virDomainDiskDefPtr conf: Clean up virDomainDiskSourceDefFormatInternal conf: Split out seclabel formating code for disk source conf: Export disk source formatter and parser snapshot: conf: Use common parsing and formatting functions for source snapshot: conf: Fix NULL dereference when <driver> element is empty conf: Add functions to copy and free network disk source definitions qemu: snapshot: Detect internal snapshots also for sheepdog and RBD conf: Add helper do clear disk source authentication struct qemu: Clear old translated pool source src/conf/domain_conf.c | 261 ++++++--- src/conf/domain_conf.h | 25 + src/conf/snapshot_conf.c | 56 +- src/conf/snapshot_conf.h | 1 + src/conf/storage_conf.c | 4 + src/conf/storage_conf.h | 2 + src/libvirt_private.syms | 6 + src/qemu/qemu_command.c | 650 +++++++++++---------- src/qemu/qemu_command.h | 6 + src/qemu/qemu_conf.c | 129 ++-- src/qemu/qemu_conf.h | 2 + src/qemu/qemu_driver.c | 3 +- .../qemuxml2argv-disk-source-pool-mode.args | 10 + .../qemuxml2argv-disk-source-pool-mode.xml | 4 +- .../qemuxml2argv-disk-source-pool.args | 8 + .../qemuxml2argv-disk-source-pool.xml | 2 +- tests/qemuxml2argvtest.c | 166 ++++++ 17 files changed, 874 insertions(+), 461 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.args -- 1.8.4.3
Add support for string conversion from and to the virStorageVolType enum. --- src/conf/storage_conf.c | 4 ++++ src/conf/storage_conf.h | 2 ++ src/libvirt_private.syms | 2 ++ 3 files changed, 8 insertions(+) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 8b378c2..1355056 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -99,6 +99,10 @@ VIR_ENUM_IMPL(virStoragePoolAuthType, VIR_STORAGE_POOL_AUTH_LAST, "none", "chap", "ceph") +VIR_ENUM_IMPL(virStorageVol, + VIR_STORAGE_VOL_LAST, + "file", "block", "dir", "network"); + typedef const char *(*virStorageVolFormatToString)(int format); typedef int (*virStorageVolFormatFromString)(const char *format); typedef const char *(*virStorageVolFeatureToString)(int feature); diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index f062bd8..9897c97 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -550,6 +550,8 @@ enum virStoragePartedFsType { }; VIR_ENUM_DECL(virStoragePartedFsType) +VIR_ENUM_DECL(virStorageVol); + # define VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_ACTIVE \ (VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE | \ VIR_CONNECT_LIST_STORAGE_POOLS_INACTIVE) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a705c56..205fe56 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -701,6 +701,8 @@ virStorageVolDefFree; virStorageVolDefParseFile; virStorageVolDefParseNode; virStorageVolDefParseString; +virStorageVolTypeFromString; +virStorageVolTypeToString; # conf/storage_encryption_conf.h -- 1.8.4.3
On 11/25/2013 09:11 AM, Peter Krempa wrote:
Add support for string conversion from and to the virStorageVolType enum. --- src/conf/storage_conf.c | 4 ++++ src/conf/storage_conf.h | 2 ++ src/libvirt_private.syms | 2 ++ 3 files changed, 8 insertions(+)
You can drop most of this patch; commit 1b5c8d4 added the same.
+++ b/src/libvirt_private.syms @@ -701,6 +701,8 @@ virStorageVolDefFree; virStorageVolDefParseFile; virStorageVolDefParseNode; virStorageVolDefParseString; +virStorageVolTypeFromString; +virStorageVolTypeToString;
This may be the only salvageable part; if so, squash it into the patch where you first need the symbols exported. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
To support testing of "volume" disk backing, we need to implement a few disk driver backend functions. The fake storage driver uses files in storagepoolxml2xmlout/POOLNAME.xml as XML files for pool definitions and volume names are in format "VOL_TYPE+VOL_PATH". By default type "block" is assumed (for iSCSI test compatibility). The choice of this approach along with implemented functions was made so that <disk type='volume'> can be tested in the xml2argv test. --- tests/qemuxml2argvtest.c | 162 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 162 insertions(+) diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index a290062..07640db 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -18,6 +18,7 @@ # include "qemu/qemu_command.h" # include "qemu/qemu_domain.h" # include "datatypes.h" +# include "conf/storage_conf.h" # include "cpu/cpu_map.h" # include "virstring.h" @@ -75,6 +76,161 @@ static virSecretDriver fakeSecretDriver = { .secretUndefine = NULL, }; + +# define STORAGE_POOL_XML_PATH "storagepoolxml2xmlout/" +static const unsigned char fakeUUID[VIR_UUID_BUFLEN] = "fakeuuid"; + +static virStoragePoolPtr +fakeStoragePoolLookupByName(virConnectPtr conn, + const char *name) +{ + char *xmlpath = NULL; + virStoragePoolPtr ret = NULL; + + if (STRNEQ(name, "inactive")) { + if (virAsprintf(&xmlpath, "%s%s.xml", + STORAGE_POOL_XML_PATH, + name) < 0) + return NULL; + + if (!virFileExists(xmlpath)) { + virReportError(VIR_ERR_NO_STORAGE_POOL, + "File '%s' not found", xmlpath); + goto cleanup; + } + } + + ret = virGetStoragePool(conn, name, fakeUUID, NULL, NULL); + +cleanup: + VIR_FREE(xmlpath); + return ret; +} + + +static virStorageVolPtr +fakeStorageVolLookupByName(virStoragePoolPtr pool, + const char *name) +{ + char **volinfo = NULL; + virStorageVolPtr ret = NULL; + + if (STREQ(pool->name, "inactive")) { + virReportError(VIR_ERR_OPERATION_INVALID, + "storage pool '%s' is not active", pool->name); + return NULL; + } + + if (STREQ(name, "nonexistent")) { + virReportError(VIR_ERR_NO_STORAGE_VOL, + "no storage vol with matching name '%s'", name); + return NULL; + } + + if (!strchr(name, '+')) + goto fallback; + + if (!(volinfo = virStringSplit(name, "+", 2))) + return NULL; + + if (!volinfo[1]) + goto fallback; + + ret = virGetStorageVol(pool->conn, pool->name, volinfo[1], volinfo[0], + NULL, NULL); + +cleanup: + virStringFreeList(volinfo); + return ret; + +fallback: + ret = virGetStorageVol(pool->conn, pool->name, name, "block", NULL, NULL); + goto cleanup; +} + +static int +fakeStorageVolGetInfo(virStorageVolPtr vol, + virStorageVolInfoPtr info) +{ + memset(info, 0, sizeof(info)); + + info->type = virStorageVolTypeFromString(vol->key); + + if (info->type < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "Invalid volume type '%s'", vol->key); + return -1; + } + + return 0; +} + + +static char * +fakeStorageVolGetPath(virStorageVolPtr vol) +{ + char *ret = NULL; + + ignore_value(virAsprintf(&ret, "/some/%s/device/%s", vol->key, vol->name)); + + return ret; +} + + +static char * +fakeStoragePoolDumpXML(virStoragePoolPtr pool, + unsigned int flags_unused ATTRIBUTE_UNUSED) +{ + char *xmlpath = NULL; + char *xmlbuf = NULL; + + if (STREQ(pool->name, "inactive")) { + virReportError(VIR_ERR_NO_STORAGE_POOL, NULL); + return NULL; + } + + if (virAsprintf(&xmlpath, "%s%s.xml", + STORAGE_POOL_XML_PATH, + pool->name) < 0) + return NULL; + + if (virtTestLoadFile(xmlpath, &xmlbuf) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "failed to load XML file '%s'", + xmlpath); + goto cleanup; + } + +cleanup: + VIR_FREE(xmlpath); + + return xmlbuf; +} + +static int +fakeStorageClose(virConnectPtr conn ATTRIBUTE_UNUSED) +{ + return 0; +} + +static int +fakeStoragePoolIsActive(virStoragePoolPtr pool ATTRIBUTE_UNUSED) +{ + return 1; +} + + +static virStorageDriver fakeStorageDriver = { + .name = "fake_storage", + .storageClose = fakeStorageClose, + .storagePoolLookupByName = fakeStoragePoolLookupByName, + .storageVolLookupByName = fakeStorageVolLookupByName, + .storagePoolGetXMLDesc = fakeStoragePoolDumpXML, + .storageVolGetPath = fakeStorageVolGetPath, + .storageVolGetInfo = fakeStorageVolGetInfo, + .storagePoolIsActive = fakeStoragePoolIsActive, +}; + typedef enum { FLAG_EXPECT_ERROR = 1 << 0, FLAG_EXPECT_FAILURE = 1 << 1, @@ -103,6 +259,7 @@ static int testCompareXMLToArgvFiles(const char *xml, if (!(conn = virGetConnect())) goto out; conn->secretDriver = &fakeSecretDriver; + conn->storageDriver = &fakeStorageDriver; if (!(vmdef = virDomainDefParseFile(xml, driver.caps, driver.xmlopt, QEMU_EXPECTED_VIRT_TYPES, @@ -165,6 +322,11 @@ static int testCompareXMLToArgvFiles(const char *xml, } } + for (i = 0; i < vmdef->ndisks; i++) { + if (qemuTranslateDiskSourcePool(conn, vmdef->disks[i]) < 0) + goto out; + } + if (!(cmd = qemuBuildCommandLine(conn, &driver, vmdef, &monitor_chr, (flags & FLAG_JSON), extraFlags, migrateFrom, migrateFd, NULL, -- 1.8.4.3
Tweak the existing file to test command line generator too. --- tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.args | 8 ++++++++ tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.xml | 2 +- tests/qemuxml2argvtest.c | 2 ++ 3 files changed, 11 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.args diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.args new file mode 100644 index 0000000..da87ad9 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.args @@ -0,0 +1,8 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults \ +-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -drive \ +file=/some/block/device/cdrom,if=none,media=cdrom,id=drive-ide0-0-1 -device \ +ide-drive,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1 -drive \ +file=/tmp/idedisk.img,if=none,id=drive-ide0-0-2 -device \ +ide-drive,bus=ide.0,unit=2,drive=drive-ide0-0-2,id=ide0-0-2 -device \ +virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.xml index 465a539..6ca5cf7 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.xml @@ -15,7 +15,7 @@ <devices> <emulator>/usr/bin/qemu</emulator> <disk type='volume' device='cdrom'> - <source pool='blk-pool0' volume='blk-pool0-vol0' startupPolicy='optional'> + <source pool='pool-disk' volume='block+cdrom'> <seclabel model='selinux' relabel='yes'> <label>system_u:system_r:public_content_t:s0</label> </seclabel> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 07640db..b90babf 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -770,6 +770,8 @@ mymain(void) DO_TEST("disk-aio", QEMU_CAPS_DRIVE, QEMU_CAPS_DRIVE_AIO, QEMU_CAPS_DRIVE_CACHE_V2, QEMU_CAPS_DRIVE_FORMAT); + DO_TEST("disk-source-pool", + QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); DO_TEST("disk-ioeventfd", QEMU_CAPS_DRIVE, QEMU_CAPS_VIRTIO_IOEVENTFD, QEMU_CAPS_VIRTIO_TX_ALG, QEMU_CAPS_DEVICE, -- 1.8.4.3
Tweak the existing file so that it can be tested for command line corectness. --- tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.args | 10 ++++++++++ tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.xml | 4 ++-- tests/qemuxml2argvtest.c | 2 ++ 3 files changed, 14 insertions(+), 2 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.args diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.args new file mode 100644 index 0000000..8f6a3dd --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.args @@ -0,0 +1,10 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults \ +-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -drive \ +file=/some/block/device/unit:0:0:1,if=none,media=cdrom,id=drive-ide0-0-1 -device \ +ide-drive,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1 -drive \ +file=iscsi://iscsi.example.com:3260/demo-target/2,if=none,media=cdrom,id=drive-ide0-0-2 \ +-device ide-drive,bus=ide.0,unit=2,drive=drive-ide0-0-2,id=ide0-0-2 -drive \ +file=/tmp/idedisk.img,if=none,id=drive-ide0-0-3 -device \ +ide-drive,bus=ide.0,unit=3,drive=drive-ide0-0-3,id=ide0-0-3 -device \ +virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.xml index b907633..9f90293 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.xml @@ -15,7 +15,7 @@ <devices> <emulator>/usr/bin/qemu</emulator> <disk type='volume' device='cdrom'> - <source pool='blk-pool0' volume='blk-pool0-vol0' mode='host' startupPolicy='optional'> + <source pool='pool-iscsi-auth' volume='unit:0:0:1' mode='host'> <seclabel model='selinux' relabel='yes'> <label>system_u:system_r:public_content_t:s0</label> </seclabel> @@ -25,7 +25,7 @@ <address type='drive' controller='0' bus='0' target='0' unit='1'/> </disk> <disk type='volume' device='cdrom'> - <source pool='blk-pool0' volume='blk-pool0-vol1' mode='direct' startupPolicy='optional'> + <source pool='pool-iscsi' volume='unit:0:0:2' mode='direct'> <seclabel model='selinux' relabel='yes'> <label>system_u:system_r:public_content_t:s0</label> </seclabel> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index b90babf..2fcd707 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -772,6 +772,8 @@ mymain(void) QEMU_CAPS_DRIVE_CACHE_V2, QEMU_CAPS_DRIVE_FORMAT); DO_TEST("disk-source-pool", QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); + DO_TEST("disk-source-pool-mode", + QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); DO_TEST("disk-ioeventfd", QEMU_CAPS_DRIVE, QEMU_CAPS_VIRTIO_IOEVENTFD, QEMU_CAPS_VIRTIO_TX_ALG, QEMU_CAPS_DEVICE, -- 1.8.4.3
The function was a mess originally making decisions on volume type instead of pool type. Refactor it so that it doesn't require a special formatter function in qemu and use typecasted strings to avoid possible future omittings. --- src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 74 +++------------------------- src/qemu/qemu_conf.c | 125 ++++++++++++++++++++++++++++++++--------------- src/qemu/qemu_conf.h | 2 + 5 files changed, 96 insertions(+), 107 deletions(-) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 4561ccc..a5ef2ca 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -686,6 +686,7 @@ struct _virDomainDiskSourcePoolDef { char *volume; /* volume name */ int voltype; /* enum virStorageVolType, internal only */ int pooltype; /* enum virStoragePoolType, internal only */ + int actualtype; /* enum virDomainDiskType, internal only */ int mode; /* enum virDomainDiskSourcePoolMode */ }; typedef virDomainDiskSourcePoolDef *virDomainDiskSourcePoolDefPtr; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 205fe56..aeb3568 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -693,6 +693,7 @@ virStoragePoolSourceFree; virStoragePoolSourceListFormat; virStoragePoolSourceListNewSource; virStoragePoolTypeFromString; +virStoragePoolTypeToString; virStorageVolDefFindByKey; virStorageVolDefFindByName; virStorageVolDefFindByPath; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 8dc7e43..2a4390e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3775,67 +3775,6 @@ qemuBuildNBDString(virConnectPtr conn, virDomainDiskDefPtr disk, virBufferPtr op return 0; } -static int -qemuBuildVolumeString(virConnectPtr conn, - virDomainDiskDefPtr disk, - virBufferPtr opt) -{ - int ret = -1; - - switch (disk->srcpool->voltype) { - case VIR_STORAGE_VOL_DIR: - if (!disk->readonly) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("cannot create virtual FAT disks in read-write mode")); - goto cleanup; - } - if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) - virBufferEscape(opt, ',', ",", "file=fat:floppy:%s,", disk->src); - else - virBufferEscape(opt, ',', ",", "file=fat:%s,", disk->src); - break; - case VIR_STORAGE_VOL_BLOCK: - if (disk->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("tray status 'open' is invalid for " - "block type volume")); - goto cleanup; - } - if (disk->srcpool->pooltype == VIR_STORAGE_POOL_ISCSI) { - if (disk->srcpool->mode == - VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DIRECT) { - if (qemuBuildISCSIString(conn, disk, opt) < 0) - goto cleanup; - virBufferAddChar(opt, ','); - } else if (disk->srcpool->mode == - VIR_DOMAIN_DISK_SOURCE_POOL_MODE_HOST) { - virBufferEscape(opt, ',', ",", "file=%s,", disk->src); - } - } else { - virBufferEscape(opt, ',', ",", "file=%s,", disk->src); - } - break; - case VIR_STORAGE_VOL_FILE: - if (disk->auth.username) { - if (qemuBuildISCSIString(conn, disk, opt) < 0) - goto cleanup; - virBufferAddChar(opt, ','); - } else { - virBufferEscape(opt, ',', ",", "file=%s,", disk->src); - } - break; - case VIR_STORAGE_VOL_NETWORK: - /* Keep the compiler quiet, qemuTranslateDiskSourcePool already - * reported the unsupported error. - */ - break; - } - - ret = 0; - -cleanup: - return ret; -} char * qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, @@ -3849,6 +3788,7 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, virDomainDiskGeometryTransTypeToString(disk->geometry.trans); int idx = virDiskNameToIndex(disk->dst); int busid = -1, unitid = -1; + int actualType = qemuDiskGetActualType(disk); if (idx < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -3932,12 +3872,13 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, /* disk->src is NULL when we use nbd disks */ if ((disk->src || - (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK && + (actualType == VIR_DOMAIN_DISK_TYPE_NETWORK && disk->protocol == VIR_DOMAIN_DISK_PROTOCOL_NBD)) && !((disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY || disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) && disk->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN)) { - if (disk->type == VIR_DOMAIN_DISK_TYPE_DIR) { + + if (actualType == VIR_DOMAIN_DISK_TYPE_DIR) { /* QEMU only supports magic FAT format for now */ if (disk->format > 0 && disk->format != VIR_STORAGE_FILE_FAT) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -3955,7 +3896,7 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, disk->src); else virBufferEscape(&opt, ',', ",", "file=fat:%s,", disk->src); - } else if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK) { + } else if (actualType == VIR_DOMAIN_DISK_TYPE_NETWORK) { switch (disk->protocol) { case VIR_DOMAIN_DISK_PROTOCOL_NBD: if (qemuBuildNBDString(conn, disk, &opt) < 0) @@ -4023,11 +3964,8 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, virBufferEscape(&opt, ',', ",", "%s,", disk->src); break; } - } else if (disk->type == VIR_DOMAIN_DISK_TYPE_VOLUME) { - if (qemuBuildVolumeString(conn, disk, &opt) < 0) - goto error; } else { - if ((disk->type == VIR_DOMAIN_DISK_TYPE_BLOCK) && + if ((actualType == VIR_DOMAIN_DISK_TYPE_BLOCK) && (disk->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("tray status 'open' is invalid for " diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 5803850..e098c13 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1298,6 +1298,17 @@ cleanup: return ret; } + +int +qemuDiskGetActualType(virDomainDiskDefPtr def) +{ + if (def->type == VIR_DOMAIN_DISK_TYPE_VOLUME) + return def->srcpool->actualtype; + + return def->type; +} + + int qemuTranslateDiskSourcePool(virConnectPtr conn, virDomainDiskDefPtr def) @@ -1319,70 +1330,106 @@ qemuTranslateDiskSourcePool(virConnectPtr conn, if (!(pool = virStoragePoolLookupByName(conn, def->srcpool->pool))) return -1; + if (virStoragePoolIsActive(pool) != 1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("storage pool '%s' containing volume '%s' " + "is not active"), + def->srcpool->pool, def->srcpool->volume); + goto cleanup; + } + if (!(vol = virStorageVolLookupByName(pool, def->srcpool->volume))) goto cleanup; if (virStorageVolGetInfo(vol, &info) < 0) goto cleanup; - if (def->startupPolicy && - info.type != VIR_STORAGE_VOL_FILE) { + if (!(poolxml = virStoragePoolGetXMLDesc(pool, 0))) + goto cleanup; + + if (!(pooldef = virStoragePoolDefParseString(poolxml))) + goto cleanup; + + def->srcpool->pooltype = pooldef->type; + def->srcpool->voltype = info.type; + + if (def->srcpool->mode && pooldef->type != VIR_STORAGE_POOL_ISCSI) { virReportError(VIR_ERR_XML_ERROR, "%s", - _("'startupPolicy' is only valid for 'file' type volume")); + _("disk source mode is only valid when " + "storage pool is of iscsi type")); goto cleanup; } - switch (info.type) { - case VIR_STORAGE_VOL_FILE: - case VIR_STORAGE_VOL_DIR: + switch ((enum virStoragePoolType) pooldef->type) { + case VIR_STORAGE_POOL_DIR: + case VIR_STORAGE_POOL_FS: + case VIR_STORAGE_POOL_NETFS: + case VIR_STORAGE_POOL_LOGICAL: + case VIR_STORAGE_POOL_DISK: + case VIR_STORAGE_POOL_SCSI: if (!(def->src = virStorageVolGetPath(vol))) goto cleanup; - break; - case VIR_STORAGE_VOL_BLOCK: - if (!(poolxml = virStoragePoolGetXMLDesc(pool, 0))) - goto cleanup; - - if (!(pooldef = virStoragePoolDefParseString(poolxml))) - goto cleanup; - if (def->srcpool->mode && pooldef->type != VIR_STORAGE_POOL_ISCSI) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("disk source mode is only valid when " - "storage pool is of iscsi type")); + switch (info.type) { + case VIR_STORAGE_VOL_FILE: + def->srcpool->actualtype = VIR_DOMAIN_DISK_TYPE_FILE; + break; + + case VIR_STORAGE_VOL_DIR: + def->srcpool->actualtype = VIR_DOMAIN_DISK_TYPE_DIR; + break; + + case VIR_STORAGE_VOL_BLOCK: + def->srcpool->actualtype = VIR_DOMAIN_DISK_TYPE_BLOCK; + break; + + case VIR_STORAGE_VOL_NETWORK: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected storage volume type '%s' " + "for storage pool type '%s'"), + virStorageVolTypeToString(info.type), + virStoragePoolTypeToString(pooldef->type)); goto cleanup; } - def->srcpool->pooltype = pooldef->type; - if (pooldef->type == VIR_STORAGE_POOL_ISCSI) { - /* Default to use the LUN's path on host */ - if (!def->srcpool->mode) - def->srcpool->mode = VIR_DOMAIN_DISK_SOURCE_POOL_MODE_HOST; - - if (def->srcpool->mode == - VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DIRECT) { - if (qemuAddISCSIPoolSourceHost(def, pooldef) < 0) - goto cleanup; - } else if (def->srcpool->mode == - VIR_DOMAIN_DISK_SOURCE_POOL_MODE_HOST) { - if (!(def->src = virStorageVolGetPath(vol))) - goto cleanup; - } + break; + + case VIR_STORAGE_POOL_ISCSI: + switch (def->srcpool->mode) { + case VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DEFAULT: + case VIR_DOMAIN_DISK_SOURCE_POOL_MODE_LAST: + def->srcpool->mode = VIR_DOMAIN_DISK_SOURCE_POOL_MODE_HOST; + def->srcpool->actualtype = VIR_DOMAIN_DISK_TYPE_BLOCK; + /* fallthrough */ + case VIR_DOMAIN_DISK_SOURCE_POOL_MODE_HOST: + if (!(def->src = virStorageVolGetPath(vol))) + goto cleanup; + break; + + case VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DIRECT: + def->srcpool->actualtype = VIR_DOMAIN_DISK_TYPE_NETWORK; + def->protocol = VIR_DOMAIN_DISK_PROTOCOL_ISCSI; if (qemuTranslateDiskSourcePoolAuth(def, pooldef) < 0) goto cleanup; - } else { - if (!(def->src = virStorageVolGetPath(vol))) + + if (qemuAddISCSIPoolSourceHost(def, pooldef) < 0) goto cleanup; + break; } - break; - case VIR_STORAGE_VOL_NETWORK: - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Using network volume as disk source is not supported")); + + case VIR_STORAGE_POOL_MPATH: + case VIR_STORAGE_POOL_RBD: + case VIR_STORAGE_POOL_SHEEPDOG: + case VIR_STORAGE_POOL_LAST: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("using '%s' pools for backing 'volume' disks " + "isn't yet supported"), + virStoragePoolTypeToString(pooldef->type)); goto cleanup; } - def->srcpool->voltype = info.type; ret = 0; cleanup: if (ret < 0) diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index f9ff7af..b9786b1 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -304,6 +304,8 @@ int qemuSetUnprivSGIO(virDomainDeviceDefPtr dev); int qemuDriverAllocateID(virQEMUDriverPtr driver); virDomainXMLOptionPtr virQEMUDriverCreateXMLConf(virQEMUDriverPtr driver); +int qemuDiskGetActualType(virDomainDiskDefPtr def); + int qemuTranslateDiskSourcePool(virConnectPtr conn, virDomainDiskDefPtr def); -- 1.8.4.3
The snapshot code will need to use qemu-style formatted URIs of network disks. Split out the code to avoid duplication. --- src/qemu/qemu_command.c | 146 ++++++++++++++++++++++++++++-------------------- src/qemu/qemu_command.h | 6 ++ 2 files changed, 91 insertions(+), 61 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 2a4390e..91b1bd2 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3626,106 +3626,125 @@ error: return -1; } -static int -qemuBuildDriveURIString(virConnectPtr conn, - virDomainDiskDefPtr disk, virBufferPtr opt, - const char *scheme, virSecretUsageType secretUsageType) -{ - int ret = -1; - int port = 0; - char *secret = NULL; - char *tmpscheme = NULL; - char *volimg = NULL; - char *sock = NULL; - char *user = NULL; - char *builturi = NULL; - const char *transp = NULL; - virURI uri = { - .port = port /* just to clear rest of bits */ - }; +char * +qemuBuildNetworkDriveURI(int protocol, + const char *src, + size_t nhosts, + virDomainDiskHostDefPtr hosts, + const char *username, + const char *secret) +{ + char *ret = NULL; + virURIPtr uri = NULL; - if (disk->nhosts != 1) { + if (nhosts != 1) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("%s accepts only one host"), scheme); - return -1; + _("protocol '%s' accepts only one host"), + virDomainDiskProtocolTypeToString(protocol)); + return NULL; } - virBufferAddLit(opt, "file="); - transp = virDomainDiskProtocolTransportTypeToString(disk->hosts->transport); + if (VIR_ALLOC(uri) < 0) + return NULL; - if (disk->hosts->transport == VIR_DOMAIN_DISK_PROTO_TRANS_TCP) { - if (VIR_STRDUP(tmpscheme, scheme) < 0) + if (hosts->transport == VIR_DOMAIN_DISK_PROTO_TRANS_TCP) { + if (VIR_STRDUP(uri->scheme, virDomainDiskProtocolTypeToString(protocol)) < 0) goto cleanup; } else { - if (virAsprintf(&tmpscheme, "%s+%s", scheme, transp) < 0) + if (virAsprintf(&uri->scheme, "%s+%s", + virDomainDiskProtocolTypeToString(protocol), + virDomainDiskProtocolTransportTypeToString(hosts->transport)) < 0) goto cleanup; } - if (disk->src && virAsprintf(&volimg, "/%s", disk->src) < 0) + if (src && + virAsprintf(&uri->path, "/%s", src) < 0) + goto cleanup; + + if (hosts->port && + virStrToLong_i(hosts->port, NULL, 10, &uri->port) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to parse port number '%s'"), + hosts->port); + goto cleanup; + } + + if (hosts->socket && + virAsprintf(&uri->query, "socket=%s", hosts->socket) < 0) goto cleanup; - if (disk->hosts->port) { - port = atoi(disk->hosts->port); + if (username) { + if (secret) { + if (virAsprintf(&uri->user, "%s:%s", username, secret) < 0) + goto cleanup; + } else { + if (VIR_STRDUP(uri->user, username) < 0) + goto cleanup; + } } - if (disk->hosts->socket && - virAsprintf(&sock, "socket=%s", disk->hosts->socket) < 0) + if (VIR_STRDUP(uri->server, hosts->name) < 0) goto cleanup; + ret = virURIFormat(uri); + +cleanup: + virURIFree(uri); + + return ret; +} + + +static int +qemuBuildDriveURIString(virConnectPtr conn, + virDomainDiskDefPtr disk, + virBufferPtr opt, + int protocol, + virSecretUsageType secretUsageType) +{ + char *secret = NULL; + char *builturi = NULL; + int ret = -1; + + virBufferAddLit(opt, "file="); + if (disk->auth.username && secretUsageType != VIR_SECRET_USAGE_TYPE_NONE) { /* Get the secret string using the virDomainDiskDef */ - if (!(secret = qemuGetSecretString(conn, scheme, false, + if (!(secret = qemuGetSecretString(conn, + virDomainDiskProtocolTypeToString(protocol), + false, disk->auth.secretType, disk->auth.username, disk->auth.secret.uuid, disk->auth.secret.usage, secretUsageType))) goto cleanup; - if (virAsprintf(&user, "%s:%s", disk->auth.username, secret) < 0) - goto cleanup; } - uri.scheme = tmpscheme; /* gluster+<transport> */ - uri.server = disk->hosts->name; - uri.user = user; - uri.port = port; - uri.path = volimg; - uri.query = sock; + if (!(builturi = qemuBuildNetworkDriveURI(protocol, + disk->src, + disk->nhosts, + disk->hosts, + disk->auth.username, + secret))) + goto cleanup; - builturi = virURIFormat(&uri); virBufferEscape(opt, ',', ",", "%s", builturi); ret = 0; cleanup: - VIR_FREE(builturi); - VIR_FREE(tmpscheme); - VIR_FREE(volimg); - VIR_FREE(sock); VIR_FREE(secret); - VIR_FREE(user); + VIR_FREE(builturi); return ret; } -static int -qemuBuildGlusterString(virConnectPtr conn, virDomainDiskDefPtr disk, virBufferPtr opt) -{ - return qemuBuildDriveURIString(conn, disk, opt, "gluster", - VIR_SECRET_USAGE_TYPE_NONE); -} #define QEMU_DEFAULT_NBD_PORT "10809" static int -qemuBuildISCSIString(virConnectPtr conn, virDomainDiskDefPtr disk, virBufferPtr opt) -{ - return qemuBuildDriveURIString(conn, disk, opt, "iscsi", - VIR_SECRET_USAGE_TYPE_ISCSI); -} - -static int qemuBuildNBDString(virConnectPtr conn, virDomainDiskDefPtr disk, virBufferPtr opt) { const char *transp; @@ -3741,7 +3760,8 @@ qemuBuildNBDString(virConnectPtr conn, virDomainDiskDefPtr disk, virBufferPtr op && !disk->hosts->name) || (disk->hosts->transport == VIR_DOMAIN_DISK_PROTO_TRANS_UNIX && disk->hosts->socket && disk->hosts->socket[0] != '/')) - return qemuBuildDriveURIString(conn, disk, opt, "nbd", + return qemuBuildDriveURIString(conn, disk, opt, + VIR_DOMAIN_DISK_PROTOCOL_NBD, VIR_SECRET_USAGE_TYPE_NONE); virBufferAddLit(opt, "file=nbd:"); @@ -3910,12 +3930,16 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, virBufferAddChar(&opt, ','); break; case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER: - if (qemuBuildGlusterString(conn, disk, &opt) < 0) + if (qemuBuildDriveURIString(conn, disk, &opt, + VIR_DOMAIN_DISK_PROTOCOL_GLUSTER, + VIR_SECRET_USAGE_TYPE_NONE) < 0) goto error; virBufferAddChar(&opt, ','); break; case VIR_DOMAIN_DISK_PROTOCOL_ISCSI: - if (qemuBuildISCSIString(conn, disk, &opt) < 0) + if (qemuBuildDriveURIString(conn, disk, &opt, + VIR_DOMAIN_DISK_PROTOCOL_ISCSI, + VIR_SECRET_USAGE_TYPE_ISCSI) < 0) goto error; virBufferAddChar(&opt, ','); break; diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 3277ba4..66c23cc 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -180,6 +180,12 @@ char * qemuBuildHubDevStr(virDomainDefPtr def, char * qemuBuildRedirdevDevStr(virDomainDefPtr def, virDomainRedirdevDefPtr dev, virQEMUCapsPtr qemuCaps); +char *qemuBuildNetworkDriveURI(int proto, + const char *src, + size_t nhosts, + virDomainDiskHostDefPtr hosts, + const char *username, + const char *secret); int qemuNetworkIfaceConnect(virDomainDefPtr def, virConnectPtr conn, -- 1.8.4.3
Automatically assign secret type from the disk source definition and pull in adding of the comma. Then update callers to keep generated output the same. --- src/qemu/qemu_command.c | 34 +++++++++++++--------------------- 1 file changed, 13 insertions(+), 21 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 91b1bd2..2bb9b79 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3699,9 +3699,7 @@ cleanup: static int qemuBuildDriveURIString(virConnectPtr conn, virDomainDiskDefPtr disk, - virBufferPtr opt, - int protocol, - virSecretUsageType secretUsageType) + virBufferPtr opt) { char *secret = NULL; char *builturi = NULL; @@ -3709,20 +3707,22 @@ qemuBuildDriveURIString(virConnectPtr conn, virBufferAddLit(opt, "file="); - if (disk->auth.username && secretUsageType != VIR_SECRET_USAGE_TYPE_NONE) { + if (disk->protocol == VIR_DOMAIN_DISK_PROTOCOL_ISCSI && + disk->auth.username) { /* Get the secret string using the virDomainDiskDef */ if (!(secret = qemuGetSecretString(conn, - virDomainDiskProtocolTypeToString(protocol), + virDomainDiskProtocolTypeToString(disk->protocol), false, disk->auth.secretType, disk->auth.username, disk->auth.secret.uuid, disk->auth.secret.usage, - secretUsageType))) + VIR_SECRET_USAGE_TYPE_ISCSI))) goto cleanup; } - if (!(builturi = qemuBuildNetworkDriveURI(protocol, + + if (!(builturi = qemuBuildNetworkDriveURI(disk->protocol, disk->src, disk->nhosts, disk->hosts, @@ -3731,6 +3731,7 @@ qemuBuildDriveURIString(virConnectPtr conn, goto cleanup; virBufferEscape(opt, ',', ",", "%s", builturi); + virBufferAddChar(opt, ','); ret = 0; @@ -3760,9 +3761,7 @@ qemuBuildNBDString(virConnectPtr conn, virDomainDiskDefPtr disk, virBufferPtr op && !disk->hosts->name) || (disk->hosts->transport == VIR_DOMAIN_DISK_PROTO_TRANS_UNIX && disk->hosts->socket && disk->hosts->socket[0] != '/')) - return qemuBuildDriveURIString(conn, disk, opt, - VIR_DOMAIN_DISK_PROTOCOL_NBD, - VIR_SECRET_USAGE_TYPE_NONE); + return qemuBuildDriveURIString(conn, disk, opt); virBufferAddLit(opt, "file=nbd:"); @@ -3792,6 +3791,8 @@ qemuBuildNBDString(virConnectPtr conn, virDomainDiskDefPtr disk, virBufferPtr op if (disk->src) virBufferEscape(opt, ',', ",", ":exportname=%s", disk->src); + virBufferAddChar(opt, ','); + return 0; } @@ -3921,7 +3922,6 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, case VIR_DOMAIN_DISK_PROTOCOL_NBD: if (qemuBuildNBDString(conn, disk, &opt) < 0) goto error; - virBufferAddChar(&opt, ','); break; case VIR_DOMAIN_DISK_PROTOCOL_RBD: virBufferAddLit(&opt, "file="); @@ -3929,19 +3929,11 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, goto error; virBufferAddChar(&opt, ','); break; + case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER: - if (qemuBuildDriveURIString(conn, disk, &opt, - VIR_DOMAIN_DISK_PROTOCOL_GLUSTER, - VIR_SECRET_USAGE_TYPE_NONE) < 0) - goto error; - virBufferAddChar(&opt, ','); - break; case VIR_DOMAIN_DISK_PROTOCOL_ISCSI: - if (qemuBuildDriveURIString(conn, disk, &opt, - VIR_DOMAIN_DISK_PROTOCOL_ISCSI, - VIR_SECRET_USAGE_TYPE_ISCSI) < 0) + if (qemuBuildDriveURIString(conn, disk, &opt) < 0) goto error; - virBufferAddChar(&opt, ','); break; case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG: -- 1.8.4.3
Prepare the function to integrate other protocols and start folding other network protocols into a common place. --- src/qemu/qemu_command.c | 199 +++++++++++++++++++++++++++++------------------- 1 file changed, 122 insertions(+), 77 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 2bb9b79..b1c7803 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3627,6 +3627,61 @@ error: } +static int +qemuNetworkDriveGetPort(int protocol, + const char *port) +{ + int ret = 0; + + if (port) { + if (virStrToLong_i(port, NULL, 10, &ret) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to parse port number '%s'"), + port); + return -1; + } + + return ret; + } + + switch ((enum virDomainDiskProtocol) protocol) { + case VIR_DOMAIN_DISK_PROTOCOL_HTTP: + return 80; + + case VIR_DOMAIN_DISK_PROTOCOL_HTTPS: + return 443; + + case VIR_DOMAIN_DISK_PROTOCOL_FTP: + return 21; + + case VIR_DOMAIN_DISK_PROTOCOL_FTPS: + return 990; + + case VIR_DOMAIN_DISK_PROTOCOL_TFTP: + return 69; + + case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG: + return 7000; + + case VIR_DOMAIN_DISK_PROTOCOL_NBD: + return 10809; + + case VIR_DOMAIN_DISK_PROTOCOL_ISCSI: + case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER: + /* no default port specified */ + return 0; + + case VIR_DOMAIN_DISK_PROTOCOL_RBD: + case VIR_DOMAIN_DISK_PROTOCOL_LAST: + /* not aplicable */ + return -1; + } + + return -1; +} + + + char * qemuBuildNetworkDriveURI(int protocol, const char *src, @@ -3638,56 +3693,74 @@ qemuBuildNetworkDriveURI(int protocol, char *ret = NULL; virURIPtr uri = NULL; - if (nhosts != 1) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("protocol '%s' accepts only one host"), - virDomainDiskProtocolTypeToString(protocol)); - return NULL; - } - - if (VIR_ALLOC(uri) < 0) - return NULL; + switch ((enum virDomainDiskProtocol) protocol) { + case VIR_DOMAIN_DISK_PROTOCOL_HTTP: + case VIR_DOMAIN_DISK_PROTOCOL_HTTPS: + case VIR_DOMAIN_DISK_PROTOCOL_FTP: + case VIR_DOMAIN_DISK_PROTOCOL_FTPS: + case VIR_DOMAIN_DISK_PROTOCOL_TFTP: + case VIR_DOMAIN_DISK_PROTOCOL_ISCSI: + case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER: + case VIR_DOMAIN_DISK_PROTOCOL_NBD: + if (nhosts != 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("protocol '%s' accepts only one host"), + virDomainDiskProtocolTypeToString(protocol)); + goto cleanup; + } - if (hosts->transport == VIR_DOMAIN_DISK_PROTO_TRANS_TCP) { - if (VIR_STRDUP(uri->scheme, virDomainDiskProtocolTypeToString(protocol)) < 0) - goto cleanup; - } else { - if (virAsprintf(&uri->scheme, "%s+%s", - virDomainDiskProtocolTypeToString(protocol), - virDomainDiskProtocolTransportTypeToString(hosts->transport)) < 0) - goto cleanup; - } + if (VIR_ALLOC(uri) < 0) + goto cleanup; - if (src && - virAsprintf(&uri->path, "/%s", src) < 0) - goto cleanup; + if (hosts->transport == VIR_DOMAIN_DISK_PROTO_TRANS_TCP) { + if (VIR_STRDUP(uri->scheme, + virDomainDiskProtocolTypeToString(protocol)) < 0) + goto cleanup; + } else { + if (virAsprintf(&uri->scheme, "%s+%s", + virDomainDiskProtocolTypeToString(protocol), + virDomainDiskProtocolTransportTypeToString(hosts->transport)) < 0) + goto cleanup; + } - if (hosts->port && - virStrToLong_i(hosts->port, NULL, 10, &uri->port) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("failed to parse port number '%s'"), - hosts->port); - goto cleanup; - } + if ((uri->port = qemuNetworkDriveGetPort(protocol, hosts->port)) < 0) + goto cleanup; - if (hosts->socket && - virAsprintf(&uri->query, "socket=%s", hosts->socket) < 0) - goto cleanup; + if (src && + virAsprintf(&uri->path, "%s%s", + src[0] == '/' ? "" : "/", + src) < 0) + goto cleanup; - if (username) { - if (secret) { - if (virAsprintf(&uri->user, "%s:%s", username, secret) < 0) + if (hosts->socket && + virAsprintf(&uri->query, "socket=%s", hosts->socket) < 0) goto cleanup; - } else { - if (VIR_STRDUP(uri->user, username) < 0) + + if (username) { + if (secret) { + if (virAsprintf(&uri->user, "%s:%s", username, secret) < 0) + goto cleanup; + } else { + if (VIR_STRDUP(uri->user, username) < 0) + goto cleanup; + } + } + + if (VIR_STRDUP(uri->server, hosts->name) < 0) goto cleanup; - } - } - if (VIR_STRDUP(uri->server, hosts->name) < 0) - goto cleanup; + ret = virURIFormat(uri); - ret = virURIFormat(uri); + break; + + case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG: + case VIR_DOMAIN_DISK_PROTOCOL_RBD: + case VIR_DOMAIN_DISK_PROTOCOL_LAST: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("network disk protocol '%s' not supported"), + virDomainDiskProtocolTypeToString(protocol)); + goto cleanup; + } cleanup: virURIFree(uri); @@ -3756,11 +3829,9 @@ qemuBuildNBDString(virConnectPtr conn, virDomainDiskDefPtr disk, virBufferPtr op return -1; } - if ((disk->hosts->name && strchr(disk->hosts->name, ':')) - || (disk->hosts->transport == VIR_DOMAIN_DISK_PROTO_TRANS_TCP - && !disk->hosts->name) - || (disk->hosts->transport == VIR_DOMAIN_DISK_PROTO_TRANS_UNIX - && disk->hosts->socket && disk->hosts->socket[0] != '/')) + if ((disk->hosts->name && strchr(disk->hosts->name, ':')) || + (disk->hosts->transport == VIR_DOMAIN_DISK_PROTO_TRANS_TCP && !disk->hosts->name) || + (disk->hosts->transport == VIR_DOMAIN_DISK_PROTO_TRANS_UNIX && disk->hosts->socket && disk->hosts->socket[0] != '/')) return qemuBuildDriveURIString(conn, disk, opt); virBufferAddLit(opt, "file=nbd:"); @@ -3930,6 +4001,11 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, virBufferAddChar(&opt, ','); break; + case VIR_DOMAIN_DISK_PROTOCOL_TFTP: + case VIR_DOMAIN_DISK_PROTOCOL_FTPS: + case VIR_DOMAIN_DISK_PROTOCOL_FTP: + case VIR_DOMAIN_DISK_PROTOCOL_HTTPS: + case VIR_DOMAIN_DISK_PROTOCOL_HTTP: case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER: case VIR_DOMAIN_DISK_PROTOCOL_ISCSI: if (qemuBuildDriveURIString(conn, disk, &opt) < 0) @@ -3948,37 +4024,6 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, virBufferEscape(&opt, ',', ",", "%s,", disk->src); } break; - - case VIR_DOMAIN_DISK_PROTOCOL_HTTP: - virBufferAsprintf(&opt, "file=http://%s:%s", - disk->hosts->name, - disk->hosts->port ? disk->hosts->port : "80"); - virBufferEscape(&opt, ',', ",", "%s,", disk->src); - break; - case VIR_DOMAIN_DISK_PROTOCOL_HTTPS: - virBufferAsprintf(&opt, "file=https://%s:%s", - disk->hosts->name, - disk->hosts->port ? disk->hosts->port : "443"); - virBufferEscape(&opt, ',', ",", "%s,", disk->src); - break; - case VIR_DOMAIN_DISK_PROTOCOL_FTP: - virBufferAsprintf(&opt, "file=ftp://%s:%s", - disk->hosts->name, - disk->hosts->port ? disk->hosts->port : "21"); - virBufferEscape(&opt, ',', ",", "%s,", disk->src); - break; - case VIR_DOMAIN_DISK_PROTOCOL_FTPS: - virBufferAsprintf(&opt, "file=ftps://%s:%s", - disk->hosts->name, - disk->hosts->port ? disk->hosts->port : "990"); - virBufferEscape(&opt, ',', ",", "%s,", disk->src); - break; - case VIR_DOMAIN_DISK_PROTOCOL_TFTP: - virBufferAsprintf(&opt, "file=tftp://%s:%s", - disk->hosts->name, - disk->hosts->port ? disk->hosts->port : "69"); - virBufferEscape(&opt, ',', ",", "%s,", disk->src); - break; } } else { if ((actualType == VIR_DOMAIN_DISK_TYPE_BLOCK) && -- 1.8.4.3
--- src/qemu/qemu_command.c | 37 ++++++++++++++++++++++++------------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b1c7803..82fce0e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3754,6 +3754,29 @@ qemuBuildNetworkDriveURI(int protocol, break; case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG: + if (!src) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("missing disk source for 'sheepdog' protocol")); + goto cleanup; + } + + if (nhosts == 0) { + if (virAsprintf(&ret, "sheepdog:%s", src) < 0) + goto cleanup; + } else if (nhosts == 1) { + if (virAsprintf(&ret, "sheepdog:%s:%s:%s", + hosts->name, + hosts->port ? hosts->port : "7000", + src) < 0) + goto cleanup; + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("protocol 'sheepdog' accepts up to one host")); + goto cleanup; + } + + break; + case VIR_DOMAIN_DISK_PROTOCOL_RBD: case VIR_DOMAIN_DISK_PROTOCOL_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, @@ -4001,6 +4024,7 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, virBufferAddChar(&opt, ','); break; + case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG: case VIR_DOMAIN_DISK_PROTOCOL_TFTP: case VIR_DOMAIN_DISK_PROTOCOL_FTPS: case VIR_DOMAIN_DISK_PROTOCOL_FTP: @@ -4011,19 +4035,6 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, if (qemuBuildDriveURIString(conn, disk, &opt) < 0) goto error; break; - - case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG: - if (disk->nhosts == 0) { - virBufferEscape(&opt, ',', ",", "file=sheepdog:%s,", - disk->src); - } else { - /* only one host is supported now */ - virBufferAsprintf(&opt, "file=sheepdog:%s:%s:", - disk->hosts->name, - disk->hosts->port ? disk->hosts->port : "7000"); - virBufferEscape(&opt, ',', ",", "%s,", disk->src); - } - break; } } else { if ((actualType == VIR_DOMAIN_DISK_TYPE_BLOCK) && -- 1.8.4.3
--- src/qemu/qemu_command.c | 117 +++++++++++++++++++++++++----------------------- 1 file changed, 61 insertions(+), 56 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 82fce0e..afb8fe6 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3680,7 +3680,7 @@ qemuNetworkDriveGetPort(int protocol, return -1; } - +#define QEMU_DEFAULT_NBD_PORT "10809" char * qemuBuildNetworkDriveURI(int protocol, @@ -3691,9 +3691,67 @@ qemuBuildNetworkDriveURI(int protocol, const char *secret) { char *ret = NULL; + virBuffer buf = VIR_BUFFER_INITIALIZER; virURIPtr uri = NULL; switch ((enum virDomainDiskProtocol) protocol) { + case VIR_DOMAIN_DISK_PROTOCOL_NBD: + if (nhosts != 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("protocol '%s' accepts only one host"), + virDomainDiskProtocolTypeToString(protocol)); + goto cleanup; + } + + if (!((hosts->name && strchr(hosts->name, ':')) || + (hosts->transport == VIR_DOMAIN_DISK_PROTO_TRANS_TCP && + !hosts->name) || + (hosts->transport == VIR_DOMAIN_DISK_PROTO_TRANS_UNIX && + hosts->socket && + hosts->socket[0] != '/'))) { + + virBufferAddLit(&buf, "nbd:"); + + switch (hosts->transport) { + case VIR_DOMAIN_DISK_PROTO_TRANS_TCP: + virBufferStrcat(&buf, hosts->name, NULL); + virBufferAsprintf(&buf, ":%s", + hosts->port ? hosts->port : + QEMU_DEFAULT_NBD_PORT); + break; + + case VIR_DOMAIN_DISK_PROTO_TRANS_UNIX: + if (!hosts->socket) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("socket attribute required for " + "nix transport")); + goto cleanup; + } + + virBufferAsprintf(&buf, "unix:%s", hosts->socket); + break; + + default: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("nbd does not support transport '%s'"), + virDomainDiskProtocolTransportTypeToString(hosts->transport)); + goto cleanup; + } + + if (src) + virBufferAsprintf(&buf, ":exportname=%s", src); + + if (virBufferError(&buf) < 0) { + virReportOOMError(); + goto cleanup; + } + + ret = virBufferContentAndReset(&buf); + goto cleanup; + } + /* fallthrough */ + /* NBD code uses same formatting scheme as others in some cases */ + case VIR_DOMAIN_DISK_PROTOCOL_HTTP: case VIR_DOMAIN_DISK_PROTOCOL_HTTPS: case VIR_DOMAIN_DISK_PROTOCOL_FTP: @@ -3701,7 +3759,6 @@ qemuBuildNetworkDriveURI(int protocol, case VIR_DOMAIN_DISK_PROTOCOL_TFTP: case VIR_DOMAIN_DISK_PROTOCOL_ISCSI: case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER: - case VIR_DOMAIN_DISK_PROTOCOL_NBD: if (nhosts != 1) { virReportError(VIR_ERR_INTERNAL_ERROR, _("protocol '%s' accepts only one host"), @@ -3786,6 +3843,7 @@ qemuBuildNetworkDriveURI(int protocol, } cleanup: + virBufferFreeAndReset(&buf); virURIFree(uri); return ret; @@ -3839,56 +3897,6 @@ cleanup: } -#define QEMU_DEFAULT_NBD_PORT "10809" - -static int -qemuBuildNBDString(virConnectPtr conn, virDomainDiskDefPtr disk, virBufferPtr opt) -{ - const char *transp; - - if (disk->nhosts != 1) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("nbd accepts only one host")); - return -1; - } - - if ((disk->hosts->name && strchr(disk->hosts->name, ':')) || - (disk->hosts->transport == VIR_DOMAIN_DISK_PROTO_TRANS_TCP && !disk->hosts->name) || - (disk->hosts->transport == VIR_DOMAIN_DISK_PROTO_TRANS_UNIX && disk->hosts->socket && disk->hosts->socket[0] != '/')) - return qemuBuildDriveURIString(conn, disk, opt); - - virBufferAddLit(opt, "file=nbd:"); - - switch (disk->hosts->transport) { - case VIR_DOMAIN_DISK_PROTO_TRANS_TCP: - if (disk->hosts->name) - virBufferEscape(opt, ',', ",", "%s", disk->hosts->name); - virBufferEscape(opt, ',', ",", ":%s", - disk->hosts->port ? disk->hosts->port : - QEMU_DEFAULT_NBD_PORT); - break; - case VIR_DOMAIN_DISK_PROTO_TRANS_UNIX: - if (!disk->hosts->socket) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("socket attribute required for unix transport")); - return -1; - } - virBufferEscape(opt, ',', ",", "unix:%s", disk->hosts->socket); - break; - default: - transp = virDomainDiskProtocolTransportTypeToString(disk->hosts->transport); - virReportError(VIR_ERR_INTERNAL_ERROR, - _("nbd does not support transport '%s'"), transp); - break; - } - - if (disk->src) - virBufferEscape(opt, ',', ",", ":exportname=%s", disk->src); - - virBufferAddChar(opt, ','); - - return 0; -} char * @@ -4013,10 +4021,6 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, virBufferEscape(&opt, ',', ",", "file=fat:%s,", disk->src); } else if (actualType == VIR_DOMAIN_DISK_TYPE_NETWORK) { switch (disk->protocol) { - case VIR_DOMAIN_DISK_PROTOCOL_NBD: - if (qemuBuildNBDString(conn, disk, &opt) < 0) - goto error; - break; case VIR_DOMAIN_DISK_PROTOCOL_RBD: virBufferAddLit(&opt, "file="); if (qemuBuildRBDString(conn, disk, &opt) < 0) @@ -4024,6 +4028,7 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, virBufferAddChar(&opt, ','); break; + case VIR_DOMAIN_DISK_PROTOCOL_NBD: case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG: case VIR_DOMAIN_DISK_PROTOCOL_TFTP: case VIR_DOMAIN_DISK_PROTOCOL_FTPS: -- 1.8.4.3
--- src/qemu/qemu_command.c | 155 +++++++++++++++++++----------------------------- 1 file changed, 61 insertions(+), 94 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index afb8fe6..9704c90 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3253,72 +3253,6 @@ cleanup: return secret; } -static int -qemuBuildRBDString(virConnectPtr conn, - virDomainDiskDefPtr disk, - virBufferPtr opt) -{ - size_t i; - int ret = 0; - char *secret = NULL; - - if (strchr(disk->src, ':')) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("':' not allowed in RBD source volume name '%s'"), - disk->src); - return -1; - } - - virBufferEscape(opt, ',', ",", "rbd:%s", disk->src); - if (disk->auth.username) { - virBufferEscape(opt, '\\', ":", ":id=%s", disk->auth.username); - /* Get the secret string using the virDomainDiskDef - * NOTE: qemu/librbd wants it base64 encoded - */ - if (!(secret = qemuGetSecretString(conn, "rbd", true, - disk->auth.secretType, - disk->auth.username, - disk->auth.secret.uuid, - disk->auth.secret.usage, - VIR_SECRET_USAGE_TYPE_CEPH))) - goto error; - - - virBufferEscape(opt, '\\', ":", - ":key=%s:auth_supported=cephx\\;none", - secret); - } else { - virBufferAddLit(opt, ":auth_supported=none"); - } - - if (disk->nhosts > 0) { - virBufferAddLit(opt, ":mon_host="); - for (i = 0; i < disk->nhosts; ++i) { - if (i) { - virBufferAddLit(opt, "\\;"); - } - - /* assume host containing : is ipv6 */ - if (strchr(disk->hosts[i].name, ':')) { - virBufferEscape(opt, '\\', ":", "[%s]", disk->hosts[i].name); - } else { - virBufferAsprintf(opt, "%s", disk->hosts[i].name); - } - if (disk->hosts[i].port) { - virBufferAsprintf(opt, "\\:%s", disk->hosts[i].port); - } - } - } - -cleanup: - VIR_FREE(secret); - - return ret; - -error: - ret = -1; - goto cleanup; -} static int qemuAddRBDHost(virDomainDiskDefPtr disk, char *hostport) { @@ -3693,6 +3627,7 @@ qemuBuildNetworkDriveURI(int protocol, char *ret = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; virURIPtr uri = NULL; + size_t i; switch ((enum virDomainDiskProtocol) protocol) { case VIR_DOMAIN_DISK_PROTOCOL_NBD: @@ -3835,10 +3770,51 @@ qemuBuildNetworkDriveURI(int protocol, break; case VIR_DOMAIN_DISK_PROTOCOL_RBD: + if (strchr(src, ':')) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("':' not allowed in RBD source volume name '%s'"), + src); + goto cleanup; + } + + virBufferStrcat(&buf, "rbd:", src, NULL); + + if (username) { + virBufferEscape(&buf, '\\', ":", ":id=%s", username); + virBufferEscape(&buf, '\\', ":", + ":key=%s:auth_supported=cephx\\;none", + secret); + } else { + virBufferAddLit(&buf, ":auth_supported=none"); + } + + if (nhosts > 0) { + virBufferAddLit(&buf, ":mon_host="); + for (i = 0; i < nhosts; i++) { + if (i) + virBufferAddLit(&buf, "\\;"); + + /* assume host containing : is ipv6 */ + if (strchr(hosts[i].name, ':')) + virBufferEscape(&buf, '\\', ":", "[%s]", hosts[i].name); + else + virBufferAsprintf(&buf, "%s", hosts[i].name); + + if (hosts[i].port) + virBufferAsprintf(&buf, "\\:%s", hosts[i].port); + } + } + + if (virBufferError(&buf) < 0) { + virReportOOMError(); + goto cleanup; + } + + ret = virBufferContentAndReset(&buf); + break; + + case VIR_DOMAIN_DISK_PROTOCOL_LAST: - virReportError(VIR_ERR_INTERNAL_ERROR, - _("network disk protocol '%s' not supported"), - virDomainDiskProtocolTypeToString(protocol)); goto cleanup; } @@ -3861,17 +3837,26 @@ qemuBuildDriveURIString(virConnectPtr conn, virBufferAddLit(opt, "file="); - if (disk->protocol == VIR_DOMAIN_DISK_PROTOCOL_ISCSI && + if ((disk->protocol == VIR_DOMAIN_DISK_PROTOCOL_ISCSI || + disk->protocol == VIR_DOMAIN_DISK_PROTOCOL_RBD) && disk->auth.username) { - /* Get the secret string using the virDomainDiskDef */ + bool encode = false; + int secretType = VIR_SECRET_USAGE_TYPE_ISCSI; + + if (disk->protocol == VIR_DOMAIN_DISK_PROTOCOL_RBD) { + /* qemu requires the secret to be encoded for RBD */ + encode = true; + secretType = VIR_SECRET_USAGE_TYPE_CEPH; + } + if (!(secret = qemuGetSecretString(conn, virDomainDiskProtocolTypeToString(disk->protocol), - false, + encode, disk->auth.secretType, disk->auth.username, disk->auth.secret.uuid, disk->auth.secret.usage, - VIR_SECRET_USAGE_TYPE_ISCSI))) + secretType))) goto cleanup; } @@ -4019,28 +4004,10 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, disk->src); else virBufferEscape(&opt, ',', ",", "file=fat:%s,", disk->src); - } else if (actualType == VIR_DOMAIN_DISK_TYPE_NETWORK) { - switch (disk->protocol) { - case VIR_DOMAIN_DISK_PROTOCOL_RBD: - virBufferAddLit(&opt, "file="); - if (qemuBuildRBDString(conn, disk, &opt) < 0) - goto error; - virBufferAddChar(&opt, ','); - break; - case VIR_DOMAIN_DISK_PROTOCOL_NBD: - case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG: - case VIR_DOMAIN_DISK_PROTOCOL_TFTP: - case VIR_DOMAIN_DISK_PROTOCOL_FTPS: - case VIR_DOMAIN_DISK_PROTOCOL_FTP: - case VIR_DOMAIN_DISK_PROTOCOL_HTTPS: - case VIR_DOMAIN_DISK_PROTOCOL_HTTP: - case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER: - case VIR_DOMAIN_DISK_PROTOCOL_ISCSI: - if (qemuBuildDriveURIString(conn, disk, &opt) < 0) - goto error; - break; - } + } else if (actualType == VIR_DOMAIN_DISK_TYPE_NETWORK) { + if (qemuBuildDriveURIString(conn, disk, &opt) < 0) + goto error; } else { if ((actualType == VIR_DOMAIN_DISK_TYPE_BLOCK) && (disk->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN)) { -- 1.8.4.3
--- src/qemu/qemu_command.c | 128 ++++++++++++++++++++++++++++++++---------------- 1 file changed, 86 insertions(+), 42 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9704c90..7a6e322 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3827,19 +3827,62 @@ cleanup: static int -qemuBuildDriveURIString(virConnectPtr conn, - virDomainDiskDefPtr disk, - virBufferPtr opt) +qemuGetDriveSourceString(int type, + const char *src, + int protocol, + size_t nhosts, + virDomainDiskHostDefPtr hosts, + const char *username, + const char *secret, + char **path) { + *path = NULL; + + switch ((enum virDomainDiskType) type) { + case VIR_DOMAIN_DISK_TYPE_BLOCK: + case VIR_DOMAIN_DISK_TYPE_FILE: + case VIR_DOMAIN_DISK_TYPE_DIR: + if (!src) + return 1; + + if (VIR_STRDUP(*path, src) < 0) + return -1; + + break; + + case VIR_DOMAIN_DISK_TYPE_NETWORK: + if (!(*path = qemuBuildNetworkDriveURI(protocol, + src, + nhosts, + hosts, + username, + secret))) + return -1; + break; + + case VIR_DOMAIN_DISK_TYPE_VOLUME: + case VIR_DOMAIN_DISK_TYPE_LAST: + break; + } + + return 0; +} + +static int +qemuDomainDiskGetSourceString(virConnectPtr conn, + virDomainDiskDefPtr disk, + char **source) +{ + int actualType = qemuDiskGetActualType(disk); char *secret = NULL; - char *builturi = NULL; int ret = -1; - virBufferAddLit(opt, "file="); + *source = NULL; - if ((disk->protocol == VIR_DOMAIN_DISK_PROTOCOL_ISCSI || - disk->protocol == VIR_DOMAIN_DISK_PROTOCOL_RBD) && - disk->auth.username) { + if (actualType == VIR_DOMAIN_DISK_TYPE_NETWORK && + disk->auth.username && + (disk->protocol == VIR_DOMAIN_DISK_PROTOCOL_ISCSI || + disk->protocol == VIR_DOMAIN_DISK_PROTOCOL_RBD)) { bool encode = false; int secretType = VIR_SECRET_USAGE_TYPE_ISCSI; @@ -3860,32 +3903,23 @@ qemuBuildDriveURIString(virConnectPtr conn, goto cleanup; } - - if (!(builturi = qemuBuildNetworkDriveURI(disk->protocol, - disk->src, - disk->nhosts, - disk->hosts, - disk->auth.username, - secret))) - goto cleanup; - - virBufferEscape(opt, ',', ",", "%s", builturi); - virBufferAddChar(opt, ','); - - ret = 0; + ret = qemuGetDriveSourceString(qemuDiskGetActualType(disk), + disk->src, + disk->protocol, + disk->nhosts, + disk->hosts, + disk->auth.username, + secret, + source); cleanup: VIR_FREE(secret); - VIR_FREE(builturi); - return ret; } - - char * -qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, +qemuBuildDriveStr(virConnectPtr conn, virDomainDiskDefPtr disk, bool bootable, virQEMUCapsPtr qemuCaps) @@ -3896,6 +3930,7 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, virDomainDiskGeometryTransTypeToString(disk->geometry.trans); int idx = virDiskNameToIndex(disk->dst); int busid = -1, unitid = -1; + char *source = NULL; int actualType = qemuDiskGetActualType(disk); if (idx < 0) { @@ -3978,15 +4013,18 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, break; } - /* disk->src is NULL when we use nbd disks */ - if ((disk->src || - (actualType == VIR_DOMAIN_DISK_TYPE_NETWORK && - disk->protocol == VIR_DOMAIN_DISK_PROTOCOL_NBD)) && + if (qemuDomainDiskGetSourceString(conn, disk, &source) < 0) + goto error; + + if (source && !((disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY || disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) && disk->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN)) { - if (actualType == VIR_DOMAIN_DISK_TYPE_DIR) { + virBufferAddLit(&opt, "file="); + + switch (actualType) { + case VIR_DOMAIN_DISK_TYPE_DIR: /* QEMU only supports magic FAT format for now */ if (disk->format > 0 && disk->format != VIR_STORAGE_FILE_FAT) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -3994,31 +4032,37 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, virStorageFileFormatTypeToString(disk->format)); goto error; } + if (!disk->readonly) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot create virtual FAT disks in read-write mode")); goto error; } + + virBufferAddLit(&opt, "fat:"); + if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) - virBufferEscape(&opt, ',', ",", "file=fat:floppy:%s,", - disk->src); - else - virBufferEscape(&opt, ',', ",", "file=fat:%s,", disk->src); + virBufferAddLit(&opt, "floppy:"); - } else if (actualType == VIR_DOMAIN_DISK_TYPE_NETWORK) { - if (qemuBuildDriveURIString(conn, disk, &opt) < 0) - goto error; - } else { - if ((actualType == VIR_DOMAIN_DISK_TYPE_BLOCK) && - (disk->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN)) { + break; + + case VIR_DOMAIN_DISK_TYPE_BLOCK: + if (disk->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("tray status 'open' is invalid for " "block type disk")); goto error; } - virBufferEscape(&opt, ',', ",", "file=%s,", disk->src); + + break; + + default: + break; } + + virBufferEscape(&opt, ',', ",", "%s,", source); } + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) virBufferAddLit(&opt, "if=none"); else -- 1.8.4.3
The <source> element formatting function was expecting a virDomainDiskDefPtr to store the data. As snapshots are not using this data structure to hold the data, we need to add an internal function which splits out individual fields separately. --- src/conf/domain_conf.c | 131 +++++++++++++++++++++++++++++-------------------- 1 file changed, 79 insertions(+), 52 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7b0e3ea..a6d7600 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -14352,28 +14352,38 @@ virDomainDiskBlockIoDefFormat(virBufferPtr buf, } static int -virDomainDiskSourceDefFormat(virBufferPtr buf, - virDomainDiskDefPtr def, - unsigned int flags) +virDomainDiskSourceDefFormatInternal(virBufferPtr buf, + int type, + const char *src, + int policy, + int protocol, + size_t nhosts, + virDomainDiskHostDefPtr hosts, + size_t nseclabels, + virSecurityDeviceLabelDefPtr *seclabels, + virDomainDiskSourcePoolDefPtr srcpool, + unsigned int flags) { - int n; - const char *startupPolicy = virDomainStartupPolicyTypeToString(def->startupPolicy); + size_t n; + const char *startupPolicy = NULL; - if (def->src || def->nhosts > 0 || def->srcpool || - def->startupPolicy) { - switch (def->type) { + if (policy) + startupPolicy = virDomainStartupPolicyTypeToString(policy); + + if (src || nhosts > 0 || srcpool || startupPolicy) { + switch (type) { case VIR_DOMAIN_DISK_TYPE_FILE: - virBufferAddLit(buf, " <source"); - if (def->src) - virBufferEscapeString(buf, " file='%s'", def->src); - if (def->startupPolicy) + virBufferAddLit(buf," <source"); + if (src) + virBufferEscapeString(buf, " file='%s'", src); + if (startupPolicy) virBufferEscapeString(buf, " startupPolicy='%s'", startupPolicy); - if (def->nseclabels) { + if (nseclabels) { virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 8); - for (n = 0; n < def->nseclabels; n++) - virSecurityDeviceLabelDefFormat(buf, def->seclabels[n], + for (n = 0; n < nseclabels; n++) + virSecurityDeviceLabelDefFormat(buf, seclabels[n], flags); virBufferAdjustIndent(buf, -8); virBufferAddLit(buf, " </source>\n"); @@ -14383,15 +14393,15 @@ virDomainDiskSourceDefFormat(virBufferPtr buf, break; case VIR_DOMAIN_DISK_TYPE_BLOCK: virBufferAddLit(buf, " <source"); - virBufferEscapeString(buf, " dev='%s'", def->src); - if (def->startupPolicy) + virBufferEscapeString(buf, " dev='%s'", src); + if (startupPolicy) virBufferEscapeString(buf, " startupPolicy='%s'", startupPolicy); - if (def->nseclabels) { + if (nseclabels) { virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 8); - for (n = 0; n < def->nseclabels; n++) - virSecurityDeviceLabelDefFormat(buf, def->seclabels[n], + for (n = 0; n < nseclabels; n++) + virSecurityDeviceLabelDefFormat(buf, seclabels[n], flags); virBufferAdjustIndent(buf, -8); virBufferAddLit(buf, " </source>\n"); @@ -14400,41 +14410,38 @@ virDomainDiskSourceDefFormat(virBufferPtr buf, } break; case VIR_DOMAIN_DISK_TYPE_DIR: - virBufferEscapeString(buf, " <source dir='%s'", - def->src); - if (def->startupPolicy) + virBufferEscapeString(buf, " <source dir='%s'", src); + if (startupPolicy) virBufferEscapeString(buf, " startupPolicy='%s'", startupPolicy); virBufferAddLit(buf, "/>\n"); break; case VIR_DOMAIN_DISK_TYPE_NETWORK: virBufferAsprintf(buf, " <source protocol='%s'", - virDomainDiskProtocolTypeToString(def->protocol)); - if (def->src) { - virBufferEscapeString(buf, " name='%s'", def->src); - } - if (def->nhosts == 0) { + virDomainDiskProtocolTypeToString(protocol)); + if (src) + virBufferEscapeString(buf, " name='%s'", src); + + if (nhosts == 0) { virBufferAddLit(buf, "/>\n"); } else { - size_t i; - virBufferAddLit(buf, ">\n"); - for (i = 0; i < def->nhosts; i++) { + for (n = 0; n < nhosts; n++) { virBufferAddLit(buf, " <host"); - if (def->hosts[i].name) { - virBufferEscapeString(buf, " name='%s'", def->hosts[i].name); - } - if (def->hosts[i].port) { + if (hosts[n].name) + virBufferEscapeString(buf, " name='%s'", hosts[n].name); + + if (hosts[n].port) virBufferEscapeString(buf, " port='%s'", - def->hosts[i].port); - } - if (def->hosts[i].transport) { + hosts[n].port); + + if (hosts[n].transport) virBufferAsprintf(buf, " transport='%s'", - virDomainDiskProtocolTransportTypeToString(def->hosts[i].transport)); - } - if (def->hosts[i].socket) { - virBufferEscapeString(buf, " socket='%s'", def->hosts[i].socket); - } + virDomainDiskProtocolTransportTypeToString(hosts[n].transport)); + + if (hosts[n].socket) + virBufferEscapeString(buf, " socket='%s'", hosts[n].socket); + virBufferAddLit(buf, "/>\n"); } virBufferAddLit(buf, " </source>\n"); @@ -14443,21 +14450,21 @@ virDomainDiskSourceDefFormat(virBufferPtr buf, case VIR_DOMAIN_DISK_TYPE_VOLUME: virBufferAddLit(buf, " <source"); - if (def->srcpool) { + if (srcpool) { virBufferAsprintf(buf, " pool='%s' volume='%s'", - def->srcpool->pool, def->srcpool->volume); - if (def->srcpool->mode) + srcpool->pool, srcpool->volume); + if (srcpool->mode) virBufferAsprintf(buf, " mode='%s'", - virDomainDiskSourcePoolModeTypeToString(def->srcpool->mode)); + virDomainDiskSourcePoolModeTypeToString(srcpool->mode)); } - if (def->startupPolicy) + if (startupPolicy) virBufferEscapeString(buf, " startupPolicy='%s'", startupPolicy); - if (def->nseclabels) { + if (nseclabels) { virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 8); - for (n = 0; n < def->nseclabels; n++) - virSecurityDeviceLabelDefFormat(buf, def->seclabels[n], + for (n = 0; n < nseclabels; n++) + virSecurityDeviceLabelDefFormat(buf, seclabels[n], flags); virBufferAdjustIndent(buf, -8); virBufferAddLit(buf, " </source>\n"); @@ -14468,7 +14475,7 @@ virDomainDiskSourceDefFormat(virBufferPtr buf, default: virReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected disk type %s"), - virDomainDiskTypeToString(def->type)); + virDomainDiskTypeToString(type)); return -1; } } @@ -14476,6 +14483,26 @@ virDomainDiskSourceDefFormat(virBufferPtr buf, return 0; } + +static int +virDomainDiskSourceDefFormat(virBufferPtr buf, + virDomainDiskDefPtr def, + unsigned int flags) +{ + return virDomainDiskSourceDefFormatInternal(buf, + def->type, + def->src, + def->startupPolicy, + def->protocol, + def->nhosts, + def->hosts, + def->nseclabels, + def->seclabels, + def->srcpool, + flags); +} + + static int virDomainDiskDefFormat(virBufferPtr buf, virDomainDiskDefPtr def, -- 1.8.4.3
Avoid if statements when used with virBufferEscapeString which automaticaly omits the whole string. Also add some line breaks to visualy separate the code. --- src/conf/domain_conf.c | 50 +++++++++++++++++++++----------------------------- 1 file changed, 21 insertions(+), 29 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a6d7600..03663e4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -14373,54 +14373,50 @@ virDomainDiskSourceDefFormatInternal(virBufferPtr buf, if (src || nhosts > 0 || srcpool || startupPolicy) { switch (type) { case VIR_DOMAIN_DISK_TYPE_FILE: - virBufferAddLit(buf," <source"); - if (src) - virBufferEscapeString(buf, " file='%s'", src); - if (startupPolicy) - virBufferEscapeString(buf, " startupPolicy='%s'", - startupPolicy); + virBufferAddLit(buf, " <source"); + virBufferEscapeString(buf, " file='%s'", src); + virBufferEscapeString(buf, " startupPolicy='%s'", startupPolicy); + if (nseclabels) { virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 8); for (n = 0; n < nseclabels; n++) - virSecurityDeviceLabelDefFormat(buf, seclabels[n], - flags); + virSecurityDeviceLabelDefFormat(buf, seclabels[n], flags); virBufferAdjustIndent(buf, -8); virBufferAddLit(buf, " </source>\n"); } else { virBufferAddLit(buf, "/>\n"); } break; + case VIR_DOMAIN_DISK_TYPE_BLOCK: virBufferAddLit(buf, " <source"); virBufferEscapeString(buf, " dev='%s'", src); - if (startupPolicy) - virBufferEscapeString(buf, " startupPolicy='%s'", - startupPolicy); + virBufferEscapeString(buf, " startupPolicy='%s'", startupPolicy); + if (nseclabels) { virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 8); for (n = 0; n < nseclabels; n++) - virSecurityDeviceLabelDefFormat(buf, seclabels[n], - flags); + virSecurityDeviceLabelDefFormat(buf, seclabels[n], flags); virBufferAdjustIndent(buf, -8); virBufferAddLit(buf, " </source>\n"); } else { virBufferAddLit(buf, "/>\n"); } break; + case VIR_DOMAIN_DISK_TYPE_DIR: - virBufferEscapeString(buf, " <source dir='%s'", src); - if (startupPolicy) - virBufferEscapeString(buf, " startupPolicy='%s'", - startupPolicy); + virBufferAddLit(buf, " <source"); + virBufferEscapeString(buf, " dir='%s'", src); + virBufferEscapeString(buf, " startupPolicy='%s'", startupPolicy); virBufferAddLit(buf, "/>\n"); break; + case VIR_DOMAIN_DISK_TYPE_NETWORK: virBufferAsprintf(buf, " <source protocol='%s'", virDomainDiskProtocolTypeToString(protocol)); - if (src) - virBufferEscapeString(buf, " name='%s'", src); + virBufferEscapeString(buf, " name='%s'", src); if (nhosts == 0) { virBufferAddLit(buf, "/>\n"); @@ -14428,25 +14424,21 @@ virDomainDiskSourceDefFormatInternal(virBufferPtr buf, virBufferAddLit(buf, ">\n"); for (n = 0; n < nhosts; n++) { virBufferAddLit(buf, " <host"); - if (hosts[n].name) - virBufferEscapeString(buf, " name='%s'", hosts[n].name); - - if (hosts[n].port) - virBufferEscapeString(buf, " port='%s'", - hosts[n].port); + virBufferEscapeString(buf, " name='%s'", hosts[n].name); + virBufferEscapeString(buf, " port='%s'", hosts[n].port); if (hosts[n].transport) virBufferAsprintf(buf, " transport='%s'", virDomainDiskProtocolTransportTypeToString(hosts[n].transport)); - if (hosts[n].socket) - virBufferEscapeString(buf, " socket='%s'", hosts[n].socket); + virBufferEscapeString(buf, " socket='%s'", hosts[n].socket); virBufferAddLit(buf, "/>\n"); } virBufferAddLit(buf, " </source>\n"); } break; + case VIR_DOMAIN_DISK_TYPE_VOLUME: virBufferAddLit(buf, " <source"); @@ -14457,8 +14449,7 @@ virDomainDiskSourceDefFormatInternal(virBufferPtr buf, virBufferAsprintf(buf, " mode='%s'", virDomainDiskSourcePoolModeTypeToString(srcpool->mode)); } - if (startupPolicy) - virBufferEscapeString(buf, " startupPolicy='%s'", startupPolicy); + virBufferEscapeString(buf, " startupPolicy='%s'", startupPolicy); if (nseclabels) { virBufferAddLit(buf, ">\n"); @@ -14472,6 +14463,7 @@ virDomainDiskSourceDefFormatInternal(virBufferPtr buf, virBufferAddLit(buf, "/>\n"); } break; + default: virReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected disk type %s"), -- 1.8.4.3
The code is common for all the various disk types. Split it out to a common function. --- src/conf/domain_conf.c | 62 ++++++++++++++++++++++++-------------------------- 1 file changed, 30 insertions(+), 32 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 03663e4..d000f97 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -14351,6 +14351,32 @@ virDomainDiskBlockIoDefFormat(virBufferPtr buf, } } + +/* virDomainDiskSourceDefFormatSeclabel: + * + * This function automaticaly closes the <source> element and formats any + * possible seclabels. + */ +static void +virDomainDiskSourceDefFormatSeclabel(virBufferPtr buf, + size_t nseclabels, + virSecurityDeviceLabelDefPtr *seclabels, + unsigned int flags) +{ + size_t n; + + if (nseclabels) { + virBufferAddLit(buf, ">\n"); + virBufferAdjustIndent(buf, 8); + for (n = 0; n < nseclabels; n++) + virSecurityDeviceLabelDefFormat(buf, seclabels[n], flags); + virBufferAdjustIndent(buf, -8); + virBufferAddLit(buf, " </source>\n"); + } else { + virBufferAddLit(buf, "/>\n"); + } +} + static int virDomainDiskSourceDefFormatInternal(virBufferPtr buf, int type, @@ -14377,33 +14403,15 @@ virDomainDiskSourceDefFormatInternal(virBufferPtr buf, virBufferEscapeString(buf, " file='%s'", src); virBufferEscapeString(buf, " startupPolicy='%s'", startupPolicy); - if (nseclabels) { - virBufferAddLit(buf, ">\n"); - virBufferAdjustIndent(buf, 8); - for (n = 0; n < nseclabels; n++) - virSecurityDeviceLabelDefFormat(buf, seclabels[n], flags); - virBufferAdjustIndent(buf, -8); - virBufferAddLit(buf, " </source>\n"); - } else { - virBufferAddLit(buf, "/>\n"); - } - break; + virDomainDiskSourceDefFormatSeclabel(buf, nseclabels, seclabels, flags); + break; case VIR_DOMAIN_DISK_TYPE_BLOCK: virBufferAddLit(buf, " <source"); virBufferEscapeString(buf, " dev='%s'", src); virBufferEscapeString(buf, " startupPolicy='%s'", startupPolicy); - if (nseclabels) { - virBufferAddLit(buf, ">\n"); - virBufferAdjustIndent(buf, 8); - for (n = 0; n < nseclabels; n++) - virSecurityDeviceLabelDefFormat(buf, seclabels[n], flags); - virBufferAdjustIndent(buf, -8); - virBufferAddLit(buf, " </source>\n"); - } else { - virBufferAddLit(buf, "/>\n"); - } + virDomainDiskSourceDefFormatSeclabel(buf, nseclabels, seclabels, flags); break; case VIR_DOMAIN_DISK_TYPE_DIR: @@ -14451,17 +14459,7 @@ virDomainDiskSourceDefFormatInternal(virBufferPtr buf, } virBufferEscapeString(buf, " startupPolicy='%s'", startupPolicy); - if (nseclabels) { - virBufferAddLit(buf, ">\n"); - virBufferAdjustIndent(buf, 8); - for (n = 0; n < nseclabels; n++) - virSecurityDeviceLabelDefFormat(buf, seclabels[n], - flags); - virBufferAdjustIndent(buf, -8); - virBufferAddLit(buf, " </source>\n"); - } else { - virBufferAddLit(buf, "/>\n"); - } + virDomainDiskSourceDefFormatSeclabel(buf, nseclabels, seclabels, flags); break; default: -- 1.8.4.3
This code will be reused in the snapshot disk definition parser. --- src/conf/domain_conf.c | 4 ++-- src/conf/domain_conf.h | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d000f97..09ea51f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4737,7 +4737,7 @@ cleanup: } -static int +int virDomainDiskSourceDefParse(xmlNodePtr node, int type, char **source, @@ -14377,7 +14377,7 @@ virDomainDiskSourceDefFormatSeclabel(virBufferPtr buf, } } -static int +int virDomainDiskSourceDefFormatInternal(virBufferPtr buf, int type, const char *src, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index a5ef2ca..e9800a5 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2345,6 +2345,18 @@ int virDomainDefFormatInternal(virDomainDefPtr def, unsigned int flags, virBufferPtr buf); +int virDomainDiskSourceDefFormatInternal(virBufferPtr buf, + int type, + const char *src, + int policy, + int protocol, + size_t nhosts, + virDomainDiskHostDefPtr hosts, + size_t nseclabels, + virSecurityDeviceLabelDefPtr *seclabels, + virDomainDiskSourcePoolDefPtr srcpool, + unsigned int flags); + int virDomainDefCompatibleDevice(virDomainDefPtr def, virDomainDeviceDefPtr dev); @@ -2379,6 +2391,14 @@ virDomainDiskDefPtr virDomainDiskRemove(virDomainDefPtr def, size_t i); virDomainDiskDefPtr virDomainDiskRemoveByName(virDomainDefPtr def, const char *name); +int virDomainDiskSourceDefParse(xmlNodePtr node, + int type, + char **source, + int *proto, + size_t *nhosts, + virDomainDiskHostDefPtr *hosts, + virDomainDiskSourcePoolDefPtr *srcpool); + bool virDomainHasDiskMirror(virDomainObjPtr vm); int virDomainNetFindIdx(virDomainDefPtr def, virDomainNetDefPtr net); -- 1.8.4.3
Disk source elements for snapshots were using separate code from our config parser. As snapshots can be stored on more than just regular files, we will need the universal parser to allow us to expose a variety of snapshot disk targets. This patch reuses the config parsers and formatters to do the job. This initial support only changes the code without any visible XML change. --- src/conf/snapshot_conf.c | 70 +++++++++++++++++++++++++++++++++--------------- src/conf/snapshot_conf.h | 1 + 2 files changed, 49 insertions(+), 22 deletions(-) diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index 94a74d2..7258daa 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -128,27 +128,42 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node, } } - cur = node->children; - while (cur) { - if (cur->type == XML_ELEMENT_NODE) { - if (!def->file && - xmlStrEqual(cur->name, BAD_CAST "source")) { - def->file = virXMLPropString(cur, "file"); - } else if (!def->format && - xmlStrEqual(cur->name, BAD_CAST "driver")) { - char *driver = virXMLPropString(cur, "type"); - def->format = virStorageFileFormatTypeFromString(driver); - if (def->format <= 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unknown disk snapshot driver '%s'"), - driver); - VIR_FREE(driver); - goto cleanup; - } + def->type = -1; + + for (cur = node->children; cur; cur = cur->next) { + if (cur->type != XML_ELEMENT_NODE) + continue; + + if (!def->file && + xmlStrEqual(cur->name, BAD_CAST "source")) { + + int backingtype = def->type; + + if (backingtype < 0) + backingtype = VIR_DOMAIN_DISK_TYPE_FILE; + + if (virDomainDiskSourceDefParse(cur, + backingtype, + &def->file, + NULL, + NULL, + NULL, + NULL) < 0) + goto cleanup; + + } else if (!def->format && + xmlStrEqual(cur->name, BAD_CAST "driver")) { + char *driver = virXMLPropString(cur, "type"); + def->format = virStorageFileFormatTypeFromString(driver); + if (def->format <= 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown disk snapshot driver '%s'"), + driver); VIR_FREE(driver); + goto cleanup; } + VIR_FREE(driver); } - cur = cur->next; } if (!def->snapshot && (def->file || def->format)) @@ -577,6 +592,8 @@ static void virDomainSnapshotDiskDefFormat(virBufferPtr buf, virDomainSnapshotDiskDefPtr disk) { + int type = disk->type; + if (!disk->name) return; @@ -584,6 +601,13 @@ virDomainSnapshotDiskDefFormat(virBufferPtr buf, if (disk->snapshot > 0) virBufferAsprintf(buf, " snapshot='%s'", virDomainSnapshotLocationTypeToString(disk->snapshot)); + + if (type < 0) + type = VIR_DOMAIN_DISK_TYPE_FILE; + else + virBufferAsprintf(buf, " type='%s'", + virDomainDiskTypeToString(type)); + if (!disk->file && disk->format == 0) { virBufferAddLit(buf, "/>\n"); return; @@ -591,12 +615,14 @@ virDomainSnapshotDiskDefFormat(virBufferPtr buf, virBufferAddLit(buf, ">\n"); - virBufferAdjustIndent(buf, 6); if (disk->format > 0) - virBufferEscapeString(buf, "<driver type='%s'/>\n", + virBufferEscapeString(buf, " <driver type='%s'/>\n", virStorageFileFormatTypeToString(disk->format)); - virBufferEscapeString(buf, "<source file='%s'/>\n", disk->file); - virBufferAdjustIndent(buf, -6); + virDomainDiskSourceDefFormatInternal(buf, + type, + disk->file, + 0, 0, 0, NULL, 0, NULL, NULL, 0); + virBufferAddLit(buf, " </disk>\n"); } diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h index ff3dca2..241d63c 100644 --- a/src/conf/snapshot_conf.h +++ b/src/conf/snapshot_conf.h @@ -51,6 +51,7 @@ struct _virDomainSnapshotDiskDef { char *name; /* name matching the <target dev='...' of the domain */ int index; /* index within snapshot->dom->disks that matches name */ int snapshot; /* enum virDomainSnapshotLocation */ + int type; /* enum virDomainDiskType */ char *file; /* new source file when snapshot is external */ int format; /* enum virStorageFileFormat */ }; -- 1.8.4.3
Consider the following valid snapshot XML as the <driver> element is allowed to be empty in the domainsnapshot.rng schema: $ cat snap.xml <domainsnapshot> <disks> <disk name='vda' snapshot='external'> <source file='/tmp/foo'/> <driver/> </disk> </disks> </domainsnapshot> produces the following error: $ virsh snapshot-create domain snap.xml error: internal error: unknown disk snapshot driver '(null)' The driver type is parsed as NULL from the XML as the attribute is not present and then directly used to produce the error message. With this patch the attempt to parse the driver type is skipped if not present to avoid changing the schema to forbid the empty driver element. --- src/conf/snapshot_conf.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index 7258daa..5958f13 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -154,15 +154,17 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node, } else if (!def->format && xmlStrEqual(cur->name, BAD_CAST "driver")) { char *driver = virXMLPropString(cur, "type"); - def->format = virStorageFileFormatTypeFromString(driver); - if (def->format <= 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unknown disk snapshot driver '%s'"), - driver); + if (driver) { + def->format = virStorageFileFormatTypeFromString(driver); + if (def->format <= 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown disk snapshot driver '%s'"), + driver); + VIR_FREE(driver); + goto cleanup; + } VIR_FREE(driver); - goto cleanup; } - VIR_FREE(driver); } } -- 1.8.4.3
To simplify operations on virDomainDiskHostDef arrays we will need deep copy and freeing functions. Add and properly export them. --- src/conf/domain_conf.c | 55 +++++++++++++++++++++++++++++++++++++++++++++--- src/conf/domain_conf.h | 3 +++ src/libvirt_private.syms | 2 ++ 3 files changed, 57 insertions(+), 3 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 09ea51f..b272cb1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1216,9 +1216,7 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def) VIR_FREE(def->seclabels); } - for (i = 0; i < def->nhosts; i++) - virDomainDiskHostDefClear(&def->hosts[i]); - VIR_FREE(def->hosts); + virDomainDiskHostDefFree(def->nhosts, def->hosts); VIR_FREE(def); } @@ -1233,6 +1231,57 @@ void virDomainDiskHostDefClear(virDomainDiskHostDefPtr def) VIR_FREE(def->socket); } + +void +virDomainDiskHostDefFree(size_t nhosts, + virDomainDiskHostDefPtr hosts) +{ + size_t i; + + if (!hosts) + return; + + for (i = 0; i < nhosts; i++) + virDomainDiskHostDefClear(&hosts[i]); + + VIR_FREE(hosts); +} + + +virDomainDiskHostDefPtr +virDomainDiskHostDefCopy(size_t nhosts, + virDomainDiskHostDefPtr hosts) +{ + virDomainDiskHostDefPtr ret = NULL; + size_t i; + + if (VIR_ALLOC_N(ret, nhosts) < 0) + goto error; + + for (i = 0; i < nhosts; i++) { + virDomainDiskHostDefPtr src = &hosts[i]; + virDomainDiskHostDefPtr dst = &ret[i]; + + dst->transport = src->transport; + + if (VIR_STRDUP(dst->name, src->name) < 0) + goto error; + + if (VIR_STRDUP(dst->port, src->port) < 0) + goto error; + + if (VIR_STRDUP(dst->socket, src->socket) < 0) + goto error; + } + + return ret; + +error: + virDomainDiskHostDefFree(nhosts, ret); + return NULL; +} + + void virDomainControllerDefFree(virDomainControllerDefPtr def) { if (!def) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index e9800a5..ee018f0 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2211,6 +2211,9 @@ void virDomainInputDefFree(virDomainInputDefPtr def); void virDomainDiskDefFree(virDomainDiskDefPtr def); void virDomainLeaseDefFree(virDomainLeaseDefPtr def); void virDomainDiskHostDefClear(virDomainDiskHostDefPtr def); +void virDomainDiskHostDefFree(size_t nhosts, virDomainDiskHostDefPtr hosts); +virDomainDiskHostDefPtr virDomainDiskHostDefCopy(size_t nhosts, + virDomainDiskHostDefPtr hosts); int virDomainDeviceFindControllerModel(virDomainDefPtr def, virDomainDeviceInfoPtr info, int controllerType); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index aeb3568..f952a12 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -198,6 +198,8 @@ virDomainDiskFindByBusAndDst; virDomainDiskGeometryTransTypeFromString; virDomainDiskGeometryTransTypeToString; virDomainDiskHostDefClear; +virDomainDiskHostDefCopy; +virDomainDiskHostDefFree; virDomainDiskIndexByName; virDomainDiskInsert; virDomainDiskInsertPreAlloced; -- 1.8.4.3
When doing an internal snapshot on a VM with sheepdog or RBD disks we would not set a flag to mark the domain is using internal snapshots and might end up creating a mixed snapshot. Move the setting of the variable to avoid this problem. --- src/qemu/qemu_driver.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8a1eefd..d2dbaf5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11764,6 +11764,8 @@ qemuDomainSnapshotPrepare(virDomainObjPtr vm, virDomainSnapshotDefPtr def, switch (disk->snapshot) { case VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL: + found_internal = true; + if (def->state != VIR_DOMAIN_DISK_SNAPSHOT && dom_disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK && (dom_disk->protocol == VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG || @@ -11787,7 +11789,6 @@ qemuDomainSnapshotPrepare(virDomainObjPtr vm, virDomainSnapshotDefPtr def, disk->name); goto cleanup; } - found_internal = true; break; case VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL: -- 1.8.4.3
Add virDomainDiskAuthClear to help cleaning out the struct in other places too. --- src/conf/domain_conf.c | 17 ++++++++++++++--- src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 1 + 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b272cb1..bb11011 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1201,12 +1201,9 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def) VIR_FREE(def->driverName); virStorageFileFreeMetadata(def->backingChain); VIR_FREE(def->mirror); - VIR_FREE(def->auth.username); VIR_FREE(def->wwn); VIR_FREE(def->vendor); VIR_FREE(def->product); - if (def->auth.secretType == VIR_DOMAIN_DISK_SECRET_TYPE_USAGE) - VIR_FREE(def->auth.secret.usage); virStorageEncryptionFree(def->encryption); virDomainDeviceInfoClear(&def->info); @@ -1217,10 +1214,24 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def) } virDomainDiskHostDefFree(def->nhosts, def->hosts); + virDomainDiskAuthClear(def); VIR_FREE(def); } + +void +virDomainDiskAuthClear(virDomainDiskDefPtr def) +{ + VIR_FREE(def->auth.username); + + if (def->auth.secretType == VIR_DOMAIN_DISK_SECRET_TYPE_USAGE) + VIR_FREE(def->auth.secret.usage); + + def->auth.secretType = VIR_DOMAIN_DISK_SECRET_TYPE_NONE; +} + + void virDomainDiskHostDefClear(virDomainDiskHostDefPtr def) { if (!def) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index ee018f0..4934911 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2210,6 +2210,7 @@ void virDomainGraphicsDefFree(virDomainGraphicsDefPtr def); void virDomainInputDefFree(virDomainInputDefPtr def); void virDomainDiskDefFree(virDomainDiskDefPtr def); void virDomainLeaseDefFree(virDomainLeaseDefPtr def); +void virDomainDiskAuthClear(virDomainDiskDefPtr def); void virDomainDiskHostDefClear(virDomainDiskHostDefPtr def); void virDomainDiskHostDefFree(size_t nhosts, virDomainDiskHostDefPtr hosts); virDomainDiskHostDefPtr virDomainDiskHostDefCopy(size_t nhosts, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index f952a12..f8e774c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -180,6 +180,7 @@ virDomainDeviceFindControllerModel; virDomainDeviceInfoCopy; virDomainDeviceInfoIterate; virDomainDeviceTypeToString; +virDomainDiskAuthClear; virDomainDiskBusTypeToString; virDomainDiskCacheTypeFromString; virDomainDiskCacheTypeToString; -- 1.8.4.3
Clear the old data to avoid leaking it when attempting to re-translate a pool on the same domain object. --- src/qemu/qemu_conf.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index e098c13..1192d23 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1360,6 +1360,10 @@ qemuTranslateDiskSourcePool(virConnectPtr conn, goto cleanup; } + VIR_FREE(def->src); + virDomainDiskHostDefFree(def->nhosts, def->hosts); + virDomainDiskAuthClear(def); + switch ((enum virStoragePoolType) pooldef->type) { case VIR_STORAGE_POOL_DIR: case VIR_STORAGE_POOL_FS: -- 1.8.4.3
participants (2)
-
Eric Blake -
Peter Krempa