9:14 a.m.
A subset of the big gluster snapshot series. I accidentaly squashed two patches
together so I'm reposting the subset that was broken for re-review.
Peter Krempa (4):
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
src/conf/domain_conf.h | 1 +
src/libvirt_private.syms | 1 +
src/qemu/qemu_command.c | 76 +--------
src/qemu/qemu_conf.c | 129 +++++++++-----
src/qemu/qemu_conf.h | 2 +
.../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 | 187 +++++++++++++++++++++
10 files changed, 306 insertions(+), 114 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
9:14 a.m.
New subject: [libvirt] [PATCHv2 1/4] test: Implement fake storage pool driver in qemuxml2argv test
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 | 183 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 183 insertions(+)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index a290062..9c8f8e2 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,182 @@ 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%s.xml",
+ abs_srcdir,
+ 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 *
+fakeStoragePoolGetXMLDesc(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%s.xml",
+ abs_srcdir,
+ 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)
+{
+ if (STREQ(pool->name, "inactive"))
+ return 0;
+
+ return 1;
+}
+
+/* Test storage pool implementation
+ *
+ * These functions aid testing of storage pool related stuff when creating a
+ * qemu command .
+ *
+ * There are a few "magic" values to pass to these functions:
+ *
+ * 1) "inactive" as
+ * a pool name for pool lookup creates a inactive pool. All other names are
+ * interpreted as file names for files of storagepooltest and are used as the
+ * definition for the pool. If the file doesn't exist the pool doesn't exist.
+ *
+ * 2) "nonexistent" returns an error while looking up a volume. Otherwise
+ * pattern VOLUME_TYPE+VOLUME_PATH can be used to simulate a volume in an pool.
+ * This creates a fake path for this volume. If the '+' sign is omitted, block
+ * type is assumed.
+ */
+static virStorageDriver fakeStorageDriver = {
+ .name = "fake_storage",
+ .storageClose = fakeStorageClose,
+ .storagePoolLookupByName = fakeStoragePoolLookupByName,
+ .storageVolLookupByName = fakeStorageVolLookupByName,
+ .storagePoolGetXMLDesc = fakeStoragePoolGetXMLDesc,
+ .storageVolGetPath = fakeStorageVolGetPath,
+ .storageVolGetInfo = fakeStorageVolGetInfo,
+ .storagePoolIsActive = fakeStoragePoolIsActive,
+};
+
typedef enum {
FLAG_EXPECT_ERROR = 1 << 0,
FLAG_EXPECT_FAILURE = 1 << 1,
@@ -103,6 +280,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 +343,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
2:27 a.m.
New subject: [libvirt] [PATCHv2 1/4] test: Implement fake storage pool driver in qemuxml2argv test
On 27/11/13 23:14, Peter Krempa wrote:
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 | 183 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 183 insertions(+)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index a290062..9c8f8e2 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,182 @@ 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%s.xml",
+ abs_srcdir,
+ STORAGE_POOL_XML_PATH,
+ name) < 0)
Ok, this solves the build problem.
+ 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 *
+fakeStoragePoolGetXMLDesc(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%s.xml",
+ abs_srcdir,
+ 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)
+{
+ if (STREQ(pool->name, "inactive"))
+ return 0;
+
+ return 1;
+}
+
+/* Test storage pool implementation
+ *
+ * These functions aid testing of storage pool related stuff when creating a
+ * qemu command .
s/command \./command line\./,
+ *
+ * There are a few "magic" values to pass to these functions:
+ *
+ * 1) "inactive" as
+ * a pool name for pool lookup creates a inactive pool. All other names are
s/a/an/,
How about:
a pool name to create an inactive pool.
+ * interpreted as file names for files of storagepooltest and are
used as the
To be more clear, it's XML files in storagepoolxml2xmlout
+ * definition for the pool. If the file doesn't exist the pool
doesn't exist.
+ *
+ * 2) "nonexistent" returns an error while looking up a volume. Otherwise
To be consistent, you may want to keep 1) not broken after "as".
+ * pattern VOLUME_TYPE+VOLUME_PATH can be used to simulate a volume
in an pool.
s/an pool/a pool/,
+ * This creates a fake path for this volume. If the '+' sign
is omitted, block
+ * type is assumed.
Others look fine. ACK if you fix those comments.
Osier
6:13 a.m.
New subject: [libvirt] [PATCHv2 1/4] test: Implement fake storage pool driver in qemuxml2argv test
On 11/28/13 09:27, Osier Yang wrote:
On 27/11/13 23:14, Peter Krempa wrote:
> 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 | 183
> +++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 183 insertions(+)
>
...
Others look fine. ACK if you fix those comments.
I've fixed the comments and pushed this patch. Thanks.
Peter
9:14 a.m.
New subject: [libvirt] [PATCHv2 2/4] qemuxml2argv: Add test to verify correct usage of disk type="volume"
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 9c8f8e2..b941738 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -791,6 +791,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
2:29 a.m.
New subject: [libvirt] [PATCHv2 2/4] qemuxml2argv: Add test to verify correct usage of disk type="volume"
On 27/11/13 23:14, Peter Krempa wrote:
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
Michal's ACK stands.
6:13 a.m.
New subject: [libvirt] [PATCHv2 2/4] qemuxml2argv: Add test to verify correct usage of disk type="volume"
On 11/28/13 09:29, Osier Yang wrote:
On 27/11/13 23:14, Peter Krempa wrote:
> 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
>
>
Michal's ACK stands.
Pushed. Thanks.
Peter
9:14 a.m.
New subject: [libvirt] [PATCHv2 3/4] qemuxml2argv: Add test for disk type='volume' with iSCSI pools
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 b941738..d02633a 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -793,6 +793,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
2:31 a.m.
New subject: [libvirt] [PATCHv2 3/4] qemuxml2argv: Add test for disk type='volume' with iSCSI pools
On 27/11/13 23:14, Peter Krempa wrote:
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'>
Okay, I see why you removed the "startupPolicy" now, it doesn't make sense
for a pool with block type volume.
ACK
6:13 a.m.
New subject: [libvirt] [PATCHv2 3/4] qemuxml2argv: Add test for disk type='volume' with iSCSI pools
On 11/28/13 09:31, Osier Yang wrote:
On 27/11/13 23:14, Peter Krempa wrote:
> 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
>
Okay, I see why you removed the "startupPolicy" now, it doesn't make sense
for a pool with block type volume.
ACK
Pushed; Thanks.
Peter
9:14 a.m.
New subject: [libvirt] [PATCHv2 4/4] qemu: Refactor qemuTranslatePool source
Before this patch, the translation function still needs a second ugly
helper function to actually format the command line for qemu. But if we
do the right stuff in the translation functio, we don't have to bother
with the second function any more.
This patch removes the messy qemuBuildVolumeString() function and
changes qemuTranslatePool() to set stuff up correctly so that the
regular code paths meant for volumes can be used to format the command
line correctly.
For this purpose a new helper "qemuDiskGetActualType()" is introduced to
return the type of the volume in a pool.
As a part of the refactor the qemuTranslatePool function is fixed to do
decisions based on the pool type instead of the volume type. This allows
to separate pool-type-specific stuff more clearly and will ease addition
of other pool types that will require certain other operations to get
the correct pool source.
The previously fixed tests should make sure that we don't break stuff
that was working before.
---
src/conf/domain_conf.h | 1 +
src/libvirt_private.syms | 1 +
src/qemu/qemu_command.c | 76 +++-------------------------
src/qemu/qemu_conf.c | 129 ++++++++++++++++++++++++++++++++---------------
src/qemu/qemu_conf.h | 2 +
5 files changed, 98 insertions(+), 111 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 763417f..b8129a3 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3775,69 +3775,6 @@ qemuBuildNBDString(virConnectPtr conn, virDomainDiskDefPtr disk,
virBufferPtr op
return 0;
}
-static int
-qemuBuildVolumeString(virConnectPtr conn,
- virDomainDiskDefPtr disk,
- virBufferPtr opt)
-{
- int ret = -1;
-
- switch ((virStorageVolType) 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:
- case VIR_STORAGE_VOL_NETDIR:
- case VIR_STORAGE_VOL_LAST:
- /* Keep the compiler quiet, qemuTranslateDiskSourcePool already
- * reported the unsupported error.
- */
- goto cleanup;
- }
-
- ret = 0;
-
-cleanup:
- return ret;
-}
char *
qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
@@ -3851,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,
@@ -3934,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,
@@ -3957,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)
@@ -4025,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 77df370..58a0500 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,72 +1330,108 @@ 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 ((virStorageVolType) 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:
+ case VIR_STORAGE_VOL_NETDIR:
+ 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:
- case VIR_STORAGE_VOL_NETDIR:
- case VIR_STORAGE_VOL_LAST:
- 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_GLUSTER:
+ 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
3:03 a.m.
New subject: [libvirt] [PATCHv2 4/4] qemu: Refactor qemuTranslatePool source
On 27/11/13 23:14, Peter Krempa wrote:
Before this patch, the translation function still needs a second
ugly
helper function to actually format the command line for qemu. But if we
do the right stuff in the translation functio, we don't have to bother
s/functio/function/,
with the second function any more.
This patch removes the messy qemuBuildVolumeString() function and
changes qemuTranslatePool() to set stuff up correctly so that the
s/qemuTranslatePool/qemuTranslateDiskSourcePool/,
regular code paths meant for volumes can be used to format the
command
line correctly.
For this purpose a new helper "qemuDiskGetActualType()" is introduced to
return the type of the volume in a pool.
As a part of the refactor the qemuTranslatePool function is fixed to do
Same as above.
decisions based on the pool type instead of the volume type. This
allows
to separate pool-type-specific stuff more clearly and will ease addition
of other pool types that will require certain other operations to get
the correct pool source.
The previously fixed tests should make sure that we don't break stuff
that was working before.
---
src/conf/domain_conf.h | 1 +
src/libvirt_private.syms | 1 +
src/qemu/qemu_command.c | 76 +++-------------------------
src/qemu/qemu_conf.c | 129 ++++++++++++++++++++++++++++++++---------------
src/qemu/qemu_conf.h | 2 +
5 files changed, 98 insertions(+), 111 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 763417f..b8129a3 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3775,69 +3775,6 @@ qemuBuildNBDString(virConnectPtr conn, virDomainDiskDefPtr disk,
virBufferPtr op
return 0;
}
-static int
-qemuBuildVolumeString(virConnectPtr conn,
- virDomainDiskDefPtr disk,
- virBufferPtr opt)
I admit I was confused when reviewing v2 without the commit log.
-{
- int ret = -1;
-
- switch ((virStorageVolType) 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"));
[1]
I keep my opinion that it should provide an exact error, either "block"
or "volume" is the term we used in the XML and documents. One
will be confused to see "tray status 'open' is invalid for block type
disk",
since it's actually defined as type="volume". It doesn't have to be
the same, but there should be a way to differentiate them.
- 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:
- case VIR_STORAGE_VOL_NETDIR:
- case VIR_STORAGE_VOL_LAST:
- /* Keep the compiler quiet, qemuTranslateDiskSourcePool already
- * reported the unsupported error.
- */
- goto cleanup;
- }
-
- ret = 0;
-
-cleanup:
- return ret;
-}
char *
qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
@@ -3851,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,
@@ -3934,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,
@@ -3957,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)
@@ -4025,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 77df370..58a0500 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,72 +1330,108 @@ 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) {
There is no checking for startupPolicy in qemu driver actually, the checking
is done when parsing, see src/conf/domain_conf.c. That means previously
we report error for volume not of "file" type if it has
"startupPolicy"
defined.
(looks like the checking should be more complex than before with a glance
of the current checkings in parsing code, btw). After this changing, we just
ignore the checking, whether it will cause problem when the code flow
forward
is a question.
For "volume" type disk, we have no way to check when parsing, since you
don't know what's the volume type yet. So it must be do here. I don't think
you will want to mix the checking in command line building, since it only
makes sense for "volume" type disk, and the command line build helper
is already quite big.
+ 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"));
Same as [1]
Others look fine.
Osier
3980
days inactive
3985
days old
11 comments
2 participants
participants (2)
-
Osier Yang -
Peter Krempa