12:24 p.m.
v1:http://www.redhat.com/archives/libvir-list/2016-June/msg00081.html
(patches 4, 6, and 7)
Patch 4 is now patch 1
Patch 6 was reviewed, but this patch alters it not need backingType
Patch 7 reworked to accommodate patch 1 changes
John Ferlan (3):
storage: Adjust qemu-img switches check
storage: Create helper to set backing for CreateQemuImg code
storage: Create helper to set options for CreateQemuImg code
src/storage/storage_backend.c | 223 ++++++++++-----------
tests/storagevolxml2argvdata/qcow2-flag.argv | 2 -
.../qcow2-nobacking-convert-flag.argv | 2 -
.../qcow2-nobacking-convert-none.argv | 2 -
.../qcow2-nobacking-flag.argv | 1 -
.../qcow2-nobacking-none.argv | 1 -
tests/storagevolxml2argvdata/qcow2-none.argv | 1 -
tests/storagevolxml2argvtest.c | 22 +-
8 files changed, 107 insertions(+), 147 deletions(-)
delete mode 100644 tests/storagevolxml2argvdata/qcow2-flag.argv
delete mode 100644 tests/storagevolxml2argvdata/qcow2-nobacking-convert-flag.argv
delete mode 100644 tests/storagevolxml2argvdata/qcow2-nobacking-convert-none.argv
delete mode 100644 tests/storagevolxml2argvdata/qcow2-nobacking-flag.argv
delete mode 100644 tests/storagevolxml2argvdata/qcow2-nobacking-none.argv
delete mode 100644 tests/storagevolxml2argvdata/qcow2-none.argv
--
2.5.5
12:24 p.m.
New subject: [libvirt] [PATCH v2 1/3] storage: Adjust qemu-img switches check
Since we support QEMU 0.12 and later, checking for support of specific flags
added prior to that isn't necessary.
Thus start with the base of having the "-o options" available for the
qemu-img create option and then determine whether we have the compat
option for qcow2 files (which would be necessary up through qemu 2.0
where the default changes to compat 0.11).
Adjust test to no long check for NONE and FLAG options as well was removing
results of tests that would use that option.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/storage/storage_backend.c | 91 +++++++---------------
tests/storagevolxml2argvdata/qcow2-flag.argv | 2 -
.../qcow2-nobacking-convert-flag.argv | 2 -
.../qcow2-nobacking-convert-none.argv | 2 -
.../qcow2-nobacking-flag.argv | 1 -
.../qcow2-nobacking-none.argv | 1 -
tests/storagevolxml2argvdata/qcow2-none.argv | 1 -
tests/storagevolxml2argvtest.c | 22 +-----
8 files changed, 29 insertions(+), 93 deletions(-)
delete mode 100644 tests/storagevolxml2argvdata/qcow2-flag.argv
delete mode 100644 tests/storagevolxml2argvdata/qcow2-nobacking-convert-flag.argv
delete mode 100644 tests/storagevolxml2argvdata/qcow2-nobacking-convert-none.argv
delete mode 100644 tests/storagevolxml2argvdata/qcow2-nobacking-flag.argv
delete mode 100644 tests/storagevolxml2argvdata/qcow2-nobacking-none.argv
delete mode 100644 tests/storagevolxml2argvdata/qcow2-none.argv
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index 8f03a6e..ff3f8c1 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -869,10 +869,15 @@ virStoragePloopResize(virStorageVolDefPtr vol,
return ret;
}
+/* Flag values shared w/ storagevolxml2argvtest.c.
+ *
+ * QEMU_IMG_BACKING_FORMAT_OPTIONS (added in qemu 0.11)
+ * QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT
+ * was made necessary due to 2.0 change to change the default
+ * qcow2 file format from 0.10 to 1.1.
+ */
enum {
- QEMU_IMG_BACKING_FORMAT_NONE = 0,
- QEMU_IMG_BACKING_FORMAT_FLAG,
- QEMU_IMG_BACKING_FORMAT_OPTIONS,
+ QEMU_IMG_BACKING_FORMAT_OPTIONS = 0,
QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT,
};
@@ -904,46 +909,18 @@ virStorageBackendQemuImgSupportsCompat(const char *qemuimg)
static int
virStorageBackendQEMUImgBackingFormat(const char *qemuimg)
{
- char *help = NULL;
- char *start;
- char *end;
- char *tmp;
- int ret = -1;
- int exitstatus;
- virCommandPtr cmd = virCommandNewArgList(qemuimg, "-h", NULL);
-
- virCommandAddEnvString(cmd, "LC_ALL=C");
- virCommandSetOutputBuffer(cmd, &help);
- virCommandClearCaps(cmd);
-
- /* qemuimg doesn't return zero exit status on -h,
- * therefore we need to provide pointer for storing
- * exit status, although we don't parse it any later */
- if (virCommandRun(cmd, &exitstatus) < 0)
- goto cleanup;
-
- if ((start = strstr(help, " create ")) == NULL ||
- (end = strstr(start, "\n")) == NULL) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("unable to parse qemu-img output '%s'"),
- help);
- goto cleanup;
- }
- if (((tmp = strstr(start, "-F fmt")) && tmp < end) ||
- ((tmp = strstr(start, "-F backing_fmt")) && tmp < end)) {
- ret = QEMU_IMG_BACKING_FORMAT_FLAG;
- } else if ((tmp = strstr(start, "[-o options]")) && tmp < end)
{
- if (virStorageBackendQemuImgSupportsCompat(qemuimg))
- ret = QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT;
- else
- ret = QEMU_IMG_BACKING_FORMAT_OPTIONS;
- } else {
- ret = QEMU_IMG_BACKING_FORMAT_NONE;
- }
+ /* As of QEMU 0.11 the [-o options] support was added via qemu
+ * commit id '9ea2ea71', so we start with that base and figure
+ * out what else we have */
+ int ret = QEMU_IMG_BACKING_FORMAT_OPTIONS;
+
+ /* QEMU 2.0 changed to using a format that only QEMU 1.1 and newer
+ * understands. Since we still support QEMU 0.12 and newer, we need
+ * to be able to handle the previous format as can be set via a
+ * compat=0.10 option. */
+ if (virStorageBackendQemuImgSupportsCompat(qemuimg))
+ ret = QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT;
- cleanup:
- virCommandFree(cmd);
- VIR_FREE(help);
return ret;
}
@@ -1218,29 +1195,17 @@ virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr conn,
if (info.backingPath)
virCommandAddArgList(cmd, "-b", info.backingPath, NULL);
- if (imgformat >= QEMU_IMG_BACKING_FORMAT_OPTIONS) {
- if (info.format == VIR_STORAGE_FILE_QCOW2 && !info.compat &&
- imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT)
- info.compat = "0.10";
+ if (info.format == VIR_STORAGE_FILE_QCOW2 && !info.compat &&
+ imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT)
+ info.compat = "0.10";
- if (virStorageBackendCreateQemuImgOpts(&opts, info) < 0) {
- virCommandFree(cmd);
- return NULL;
- }
- if (opts)
- virCommandAddArgList(cmd, "-o", opts, NULL);
- VIR_FREE(opts);
- } else {
- if (info.backingPath) {
- if (imgformat == QEMU_IMG_BACKING_FORMAT_FLAG)
- virCommandAddArgList(cmd, "-F", backingType, NULL);
- else
- VIR_DEBUG("Unable to set backing store format for %s with %s",
- info.path, create_tool);
- }
- if (info.encryption)
- virCommandAddArg(cmd, "-e");
+ if (virStorageBackendCreateQemuImgOpts(&opts, info) < 0) {
+ virCommandFree(cmd);
+ return NULL;
}
+ if (opts)
+ virCommandAddArgList(cmd, "-o", opts, NULL);
+ VIR_FREE(opts);
if (info.inputPath)
virCommandAddArg(cmd, info.inputPath);
diff --git a/tests/storagevolxml2argvdata/qcow2-flag.argv
b/tests/storagevolxml2argvdata/qcow2-flag.argv
deleted file mode 100644
index 5f87ad5..0000000
--- a/tests/storagevolxml2argvdata/qcow2-flag.argv
+++ /dev/null
@@ -1,2 +0,0 @@
-qemu-img create -f qcow2 -b /dev/null -F raw \
--e /var/lib/libvirt/images/OtherDemo.img 5242880K
diff --git a/tests/storagevolxml2argvdata/qcow2-nobacking-convert-flag.argv
b/tests/storagevolxml2argvdata/qcow2-nobacking-convert-flag.argv
deleted file mode 100644
index 57bdd4a..0000000
--- a/tests/storagevolxml2argvdata/qcow2-nobacking-convert-flag.argv
+++ /dev/null
@@ -1,2 +0,0 @@
-qemu-img convert -f raw -O qcow2 -e /var/lib/libvirt/images/sparse.img \
-/var/lib/libvirt/images/OtherDemo.img
diff --git a/tests/storagevolxml2argvdata/qcow2-nobacking-convert-none.argv
b/tests/storagevolxml2argvdata/qcow2-nobacking-convert-none.argv
deleted file mode 100644
index 57bdd4a..0000000
--- a/tests/storagevolxml2argvdata/qcow2-nobacking-convert-none.argv
+++ /dev/null
@@ -1,2 +0,0 @@
-qemu-img convert -f raw -O qcow2 -e /var/lib/libvirt/images/sparse.img \
-/var/lib/libvirt/images/OtherDemo.img
diff --git a/tests/storagevolxml2argvdata/qcow2-nobacking-flag.argv
b/tests/storagevolxml2argvdata/qcow2-nobacking-flag.argv
deleted file mode 100644
index 62ad85f..0000000
--- a/tests/storagevolxml2argvdata/qcow2-nobacking-flag.argv
+++ /dev/null
@@ -1 +0,0 @@
-qemu-img create -f qcow2 -e /var/lib/libvirt/images/OtherDemo.img 5242880K
diff --git a/tests/storagevolxml2argvdata/qcow2-nobacking-none.argv
b/tests/storagevolxml2argvdata/qcow2-nobacking-none.argv
deleted file mode 100644
index 62ad85f..0000000
--- a/tests/storagevolxml2argvdata/qcow2-nobacking-none.argv
+++ /dev/null
@@ -1 +0,0 @@
-qemu-img create -f qcow2 -e /var/lib/libvirt/images/OtherDemo.img 5242880K
diff --git a/tests/storagevolxml2argvdata/qcow2-none.argv
b/tests/storagevolxml2argvdata/qcow2-none.argv
deleted file mode 100644
index 10b7175..0000000
--- a/tests/storagevolxml2argvdata/qcow2-none.argv
+++ /dev/null
@@ -1 +0,0 @@
-qemu-img create -f qcow2 -b /dev/null -e /var/lib/libvirt/images/OtherDemo.img 5242880K
diff --git a/tests/storagevolxml2argvtest.c b/tests/storagevolxml2argvtest.c
index 393123b..af7de0b 100644
--- a/tests/storagevolxml2argvtest.c
+++ b/tests/storagevolxml2argvtest.c
@@ -168,9 +168,7 @@ testCompareXMLToArgvHelper(const void *data)
}
enum {
- FMT_NONE = 0,
- FMT_FLAG,
- FMT_OPTIONS,
+ FMT_OPTIONS = 0,
FMT_COMPAT,
};
@@ -217,24 +215,6 @@ mymain(void)
DO_TEST_FAIL("pool-dir", "vol-qcow2",
"pool-dir", "vol-file",
"qcow2-convert-prealloc", flags, FMT_OPTIONS);
- DO_TEST("pool-dir", "vol-qcow2",
- NULL, NULL,
- "qcow2-flag", 0, FMT_FLAG);
- DO_TEST("pool-dir", "vol-qcow2-nobacking",
- NULL, NULL,
- "qcow2-nobacking-flag", 0, FMT_FLAG);
- DO_TEST("pool-dir", "vol-qcow2-nobacking",
- "pool-dir", "vol-file",
- "qcow2-nobacking-convert-flag", 0, FMT_FLAG);
- DO_TEST("pool-dir", "vol-qcow2",
- NULL, NULL,
- "qcow2-none", 0, FMT_NONE);
- DO_TEST("pool-dir", "vol-qcow2-nobacking",
- NULL, NULL,
- "qcow2-nobacking-none", 0, FMT_NONE);
- DO_TEST("pool-dir", "vol-qcow2-nobacking",
- "pool-dir", "vol-file",
- "qcow2-nobacking-convert-none", 0, FMT_NONE);
DO_TEST("pool-dir", "vol-qcow2-lazy",
NULL, NULL,
"qcow2-lazy", 0, FMT_OPTIONS);
--
2.5.5
1:50 a.m.
New subject: [libvirt] [PATCH v2 1/3] storage: Adjust qemu-img switches check
On Mon, Jun 06, 2016 at 13:24:50 -0400, John Ferlan wrote:
Since we support QEMU 0.12 and later, checking for support of
specific flags
added prior to that isn't necessary.
Thus start with the base of having the "-o options" available for the
qemu-img create option and then determine whether we have the compat
option for qcow2 files (which would be necessary up through qemu 2.0
where the default changes to compat 0.11).
Adjust test to no long check for NONE and FLAG options as well was removing
results of tests that would use that option.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/storage/storage_backend.c | 91 +++++++---------------
tests/storagevolxml2argvdata/qcow2-flag.argv | 2 -
.../qcow2-nobacking-convert-flag.argv | 2 -
.../qcow2-nobacking-convert-none.argv | 2 -
.../qcow2-nobacking-flag.argv | 1 -
.../qcow2-nobacking-none.argv | 1 -
tests/storagevolxml2argvdata/qcow2-none.argv | 1 -
tests/storagevolxml2argvtest.c | 22 +-----
8 files changed, 29 insertions(+), 93 deletions(-)
delete mode 100644 tests/storagevolxml2argvdata/qcow2-flag.argv
delete mode 100644 tests/storagevolxml2argvdata/qcow2-nobacking-convert-flag.argv
delete mode 100644 tests/storagevolxml2argvdata/qcow2-nobacking-convert-none.argv
delete mode 100644 tests/storagevolxml2argvdata/qcow2-nobacking-flag.argv
delete mode 100644 tests/storagevolxml2argvdata/qcow2-nobacking-none.argv
delete mode 100644 tests/storagevolxml2argvdata/qcow2-none.argv
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index 8f03a6e..ff3f8c1 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -869,10 +869,15 @@ virStoragePloopResize(virStorageVolDefPtr vol,
return ret;
}
+/* Flag values shared w/ storagevolxml2argvtest.c.
+ *
+ * QEMU_IMG_BACKING_FORMAT_OPTIONS (added in qemu 0.11)
+ * QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT
+ * was made necessary due to 2.0 change to change the default
+ * qcow2 file format from 0.10 to 1.1.
+ */
enum {
- QEMU_IMG_BACKING_FORMAT_NONE = 0,
- QEMU_IMG_BACKING_FORMAT_FLAG,
- QEMU_IMG_BACKING_FORMAT_OPTIONS,
+ QEMU_IMG_BACKING_FORMAT_OPTIONS = 0,
QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT,
So if these are declared as flags they either should be binary or-able
or be used as indexes into a virBitmap as we do for qemu caps.
This use doesn't make sense as you can effectively return just one
capability.
};
@@ -904,46 +909,18 @@ virStorageBackendQemuImgSupportsCompat(const char *qemuimg)
static int
virStorageBackendQEMUImgBackingFormat(const char *qemuimg)
{
- char *help = NULL;
- char *start;
- char *end;
- char *tmp;
- int ret = -1;
- int exitstatus;
- virCommandPtr cmd = virCommandNewArgList(qemuimg, "-h", NULL);
-
- virCommandAddEnvString(cmd, "LC_ALL=C");
- virCommandSetOutputBuffer(cmd, &help);
- virCommandClearCaps(cmd);
-
- /* qemuimg doesn't return zero exit status on -h,
- * therefore we need to provide pointer for storing
- * exit status, although we don't parse it any later */
- if (virCommandRun(cmd, &exitstatus) < 0)
- goto cleanup;
-
- if ((start = strstr(help, " create ")) == NULL ||
- (end = strstr(start, "\n")) == NULL) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("unable to parse qemu-img output '%s'"),
- help);
- goto cleanup;
- }
- if (((tmp = strstr(start, "-F fmt")) && tmp < end) ||
- ((tmp = strstr(start, "-F backing_fmt")) && tmp < end)) {
- ret = QEMU_IMG_BACKING_FORMAT_FLAG;
- } else if ((tmp = strstr(start, "[-o options]")) && tmp < end)
{
- if (virStorageBackendQemuImgSupportsCompat(qemuimg))
- ret = QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT;
- else
- ret = QEMU_IMG_BACKING_FORMAT_OPTIONS;
- } else {
- ret = QEMU_IMG_BACKING_FORMAT_NONE;
- }
+ /* As of QEMU 0.11 the [-o options] support was added via qemu
+ * commit id '9ea2ea71', so we start with that base and figure
+ * out what else we have */
+ int ret = QEMU_IMG_BACKING_FORMAT_OPTIONS;
If this flag is always set (and 0) it doesn't make sense to keep it.
+
+ /* QEMU 2.0 changed to using a format that only QEMU 1.1 and newer
+ * understands. Since we still support QEMU 0.12 and newer, we need
+ * to be able to handle the previous format as can be set via a
+ * compat=0.10 option. */
+ if (virStorageBackendQemuImgSupportsCompat(qemuimg))
+ ret = QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT;
- cleanup:
- virCommandFree(cmd);
- VIR_FREE(help);
return ret;
}
5:28 a.m.
New subject: [libvirt] [PATCH v2 1/3] storage: Adjust qemu-img switches check
On 06/07/2016 02:50 AM, Peter Krempa wrote:
On Mon, Jun 06, 2016 at 13:24:50 -0400, John Ferlan wrote:
> Since we support QEMU 0.12 and later, checking for support of specific flags
> added prior to that isn't necessary.
>
> Thus start with the base of having the "-o options" available for the
> qemu-img create option and then determine whether we have the compat
> option for qcow2 files (which would be necessary up through qemu 2.0
> where the default changes to compat 0.11).
>
> Adjust test to no long check for NONE and FLAG options as well was removing
> results of tests that would use that option.
>
> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
> ---
> src/storage/storage_backend.c | 91 +++++++---------------
> tests/storagevolxml2argvdata/qcow2-flag.argv | 2 -
> .../qcow2-nobacking-convert-flag.argv | 2 -
> .../qcow2-nobacking-convert-none.argv | 2 -
> .../qcow2-nobacking-flag.argv | 1 -
> .../qcow2-nobacking-none.argv | 1 -
> tests/storagevolxml2argvdata/qcow2-none.argv | 1 -
> tests/storagevolxml2argvtest.c | 22 +-----
> 8 files changed, 29 insertions(+), 93 deletions(-)
> delete mode 100644 tests/storagevolxml2argvdata/qcow2-flag.argv
> delete mode 100644 tests/storagevolxml2argvdata/qcow2-nobacking-convert-flag.argv
> delete mode 100644 tests/storagevolxml2argvdata/qcow2-nobacking-convert-none.argv
> delete mode 100644 tests/storagevolxml2argvdata/qcow2-nobacking-flag.argv
> delete mode 100644 tests/storagevolxml2argvdata/qcow2-nobacking-none.argv
> delete mode 100644 tests/storagevolxml2argvdata/qcow2-none.argv
>
> diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
> index 8f03a6e..ff3f8c1 100644
> --- a/src/storage/storage_backend.c
> +++ b/src/storage/storage_backend.c
> @@ -869,10 +869,15 @@ virStoragePloopResize(virStorageVolDefPtr vol,
> return ret;
> }
>
> +/* Flag values shared w/ storagevolxml2argvtest.c.
> + *
> + * QEMU_IMG_BACKING_FORMAT_OPTIONS (added in qemu 0.11)
> + * QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT
> + * was made necessary due to 2.0 change to change the default
> + * qcow2 file format from 0.10 to 1.1.
> + */
> enum {
> - QEMU_IMG_BACKING_FORMAT_NONE = 0,
> - QEMU_IMG_BACKING_FORMAT_FLAG,
> - QEMU_IMG_BACKING_FORMAT_OPTIONS,
> + QEMU_IMG_BACKING_FORMAT_OPTIONS = 0,
> QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT,
So if these are declared as flags they either should be binary or-able
or be used as indexes into a virBitmap as we do for qemu caps.
True, but that wasn't the goal of this change. If we really wanted to
get fancy and go for the overkill we could add a lot of overhead and
change the code into a flag based model. Since we don't care "that much"
version to version of qemu-img what would be the use of that?
Seems to me the "original intent" was to be simplistic - I don't see any
reason or need to change that. The code is checking to see if something
is available and returning an ever increasing value indicating it was
available. The caller doesn't check flags, it checks values. I can
document that, but I figured it was obvious.
So returning 0 for instance means we have the basic functionality,
returning 1 means we can use a bit more than the basics, and the next
step would be to return 2 indicating even more functionality that we
want is available.
Although I do note a minor adjustment
+ if (info.format == VIR_STORAGE_FILE_QCOW2 && !info.compat &&
+ imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT)
+ info.compat = "0.10";
should use ">=" on the second check.
This use doesn't make sense as you can effectively return just one
capability.
It does if you consider the next step... Does "qemu-img create -o ? -f
luks" return failure or success. If success, then the next greatest
thing we care about is supported and everything we used to care about
also works.
John
> };
>
> @@ -904,46 +909,18 @@ virStorageBackendQemuImgSupportsCompat(const char *qemuimg)
> static int
> virStorageBackendQEMUImgBackingFormat(const char *qemuimg)
> {
> - char *help = NULL;
> - char *start;
> - char *end;
> - char *tmp;
> - int ret = -1;
> - int exitstatus;
> - virCommandPtr cmd = virCommandNewArgList(qemuimg, "-h", NULL);
> -
> - virCommandAddEnvString(cmd, "LC_ALL=C");
> - virCommandSetOutputBuffer(cmd, &help);
> - virCommandClearCaps(cmd);
> -
> - /* qemuimg doesn't return zero exit status on -h,
> - * therefore we need to provide pointer for storing
> - * exit status, although we don't parse it any later */
> - if (virCommandRun(cmd, &exitstatus) < 0)
> - goto cleanup;
> -
> - if ((start = strstr(help, " create ")) == NULL ||
> - (end = strstr(start, "\n")) == NULL) {
> - virReportError(VIR_ERR_INTERNAL_ERROR,
> - _("unable to parse qemu-img output '%s'"),
> - help);
> - goto cleanup;
> - }
> - if (((tmp = strstr(start, "-F fmt")) && tmp < end) ||
> - ((tmp = strstr(start, "-F backing_fmt")) && tmp < end))
{
> - ret = QEMU_IMG_BACKING_FORMAT_FLAG;
> - } else if ((tmp = strstr(start, "[-o options]")) && tmp <
end) {
> - if (virStorageBackendQemuImgSupportsCompat(qemuimg))
> - ret = QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT;
> - else
> - ret = QEMU_IMG_BACKING_FORMAT_OPTIONS;
> - } else {
> - ret = QEMU_IMG_BACKING_FORMAT_NONE;
> - }
> + /* As of QEMU 0.11 the [-o options] support was added via qemu
> + * commit id '9ea2ea71', so we start with that base and figure
> + * out what else we have */
> + int ret = QEMU_IMG_BACKING_FORMAT_OPTIONS;
If this flag is always set (and 0) it doesn't make sense to keep it.
> +
> + /* QEMU 2.0 changed to using a format that only QEMU 1.1 and newer
> + * understands. Since we still support QEMU 0.12 and newer, we need
> + * to be able to handle the previous format as can be set via a
> + * compat=0.10 option. */
> + if (virStorageBackendQemuImgSupportsCompat(qemuimg))
> + ret = QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT;
>
> - cleanup:
> - virCommandFree(cmd);
> - VIR_FREE(help);
> return ret;
> }
>
12:24 p.m.
New subject: [libvirt] [PATCH v2 2/3] storage: Create helper to set backing for CreateQemuImg code
Create a helper virStorageBackendCreateQemuImgSetBacking to perform the
backing store set
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/storage/storage_backend.c | 116 +++++++++++++++++++++++-------------------
1 file changed, 64 insertions(+), 52 deletions(-)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index ff3f8c1..41c7bda 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -1060,6 +1060,65 @@ virStorageBackendCreateQemuImgSetInput(virStorageVolDefPtr
inputvol,
}
+static int
+virStorageBackendCreateQemuImgSetBacking(virStoragePoolObjPtr pool,
+ virStorageVolDefPtr vol,
+ virStorageVolDefPtr inputvol,
+ struct _virStorageBackendQemuImgInfo *info)
+{
+ int accessRetCode = -1;
+ char *absolutePath = NULL;
+
+ info->backingFormat = vol->target.backingStore->format;
+ info->backingPath = vol->target.backingStore->path;
+
+ if (info->preallocate) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("metadata preallocation conflicts with backing"
+ " store"));
+ return -1;
+ }
+
+ /* XXX: Not strictly required: qemu-img has an option a different
+ * backing store, not really sure what use it serves though, and it
+ * may cause issues with lvm. Untested essentially.
+ */
+ if (inputvol && inputvol->target.backingStore &&
+ STRNEQ_NULLABLE(inputvol->target.backingStore->path,
+ info->backingPath)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("a different backing store cannot be specified."));
+ return -1;
+ }
+
+ if (!virStorageFileFormatTypeToString(info->backingFormat)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unknown storage vol backing store type %d"),
+ info->backingFormat);
+ return -1;
+ }
+
+ /* Convert relative backing store paths to absolute paths for access
+ * validation.
+ */
+ if ('/' != *(info->backingPath) &&
+ virAsprintf(&absolutePath, "%s/%s", pool->def->target.path,
+ info->backingPath) < 0)
+ return -1;
+ accessRetCode = access(absolutePath ? absolutePath :
+ info->backingPath, R_OK);
+ VIR_FREE(absolutePath);
+ if (accessRetCode != 0) {
+ virReportSystemError(errno,
+ _("inaccessible backing store volume %s"),
+ info->backingPath);
+ return -1;
+ }
+
+ return 0;
+}
+
+
/* Create a qemu-img virCommand from the supplied binary path,
* volume definitions and imgformat
@@ -1075,7 +1134,6 @@ virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr conn,
{
virCommandPtr cmd = NULL;
const char *type;
- const char *backingType = NULL;
char *opts = NULL;
struct _virStorageBackendQemuImgInfo info = {
.format = vol->target.format,
@@ -1120,54 +1178,10 @@ virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr conn,
virStorageBackendCreateQemuImgSetInput(inputvol, &info) < 0)
return NULL;
- if (vol->target.backingStore) {
- int accessRetCode = -1;
- char *absolutePath = NULL;
-
- info.backingFormat = vol->target.backingStore->format;
- info.backingPath = vol->target.backingStore->path;
-
- if (info.preallocate) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("metadata preallocation conflicts with backing"
- " store"));
- return NULL;
- }
-
- /* XXX: Not strictly required: qemu-img has an option a different
- * backing store, not really sure what use it serves though, and it
- * may cause issues with lvm. Untested essentially.
- */
- if (inputvol && inputvol->target.backingStore &&
- STRNEQ_NULLABLE(inputvol->target.backingStore->path, info.backingPath))
{
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("a different backing store cannot be
specified."));
- return NULL;
- }
-
- if (!(backingType = virStorageFileFormatTypeToString(info.backingFormat))) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("unknown storage vol backing store type %d"),
- info.backingFormat);
- return NULL;
- }
-
- /* Convert relative backing store paths to absolute paths for access
- * validation.
- */
- if ('/' != *(info.backingPath) &&
- virAsprintf(&absolutePath, "%s/%s",
pool->def->target.path,
- info.backingPath) < 0)
- return NULL;
- accessRetCode = access(absolutePath ? absolutePath : info.backingPath, R_OK);
- VIR_FREE(absolutePath);
- if (accessRetCode != 0) {
- virReportSystemError(errno,
- _("inaccessible backing store volume %s"),
- info.backingPath);
- return NULL;
- }
- }
+ if (vol->target.backingStore &&
+ virStorageBackendCreateQemuImgSetBacking(pool, vol, inputvol,
+ &info) < 0)
+ return NULL;
if (info.encryption &&
virStorageBackendCreateQemuImgCheckEncryption(info.format, type,
@@ -1181,10 +1195,8 @@ virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr conn,
cmd = virCommandNew(create_tool);
/* ignore the backing volume when we're converting a volume */
- if (info.inputPath) {
+ if (info.inputPath)
info.backingPath = NULL;
- backingType = NULL;
- }
if (info.inputPath)
virCommandAddArgList(cmd, "convert", "-f",
info.inputFormatStr,
--
2.5.5
12:24 p.m.
New subject: [libvirt] [PATCH v2 3/3] storage: Create helper to set options for CreateQemuImg code
Create a helper virStorageBackendCreateQemuImgSetOptions to set either
the qemu-img -o options or the previous mechanism using -F
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/storage/storage_backend.c | 30 +++++++++++++++++++++---------
1 file changed, 21 insertions(+), 9 deletions(-)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index 41c7bda..62791b1 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -1119,6 +1119,26 @@ virStorageBackendCreateQemuImgSetBacking(virStoragePoolObjPtr
pool,
}
+static int
+virStorageBackendCreateQemuImgSetOptions(virCommandPtr cmd,
+ int imgformat,
+ struct _virStorageBackendQemuImgInfo info)
+{
+ char *opts = NULL;
+
+ if (info.format == VIR_STORAGE_FILE_QCOW2 && !info.compat &&
+ imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT)
+ info.compat = "0.10";
+
+ if (virStorageBackendCreateQemuImgOpts(&opts, info) < 0)
+ return -1;
+ if (opts)
+ virCommandAddArgList(cmd, "-o", opts, NULL);
+ VIR_FREE(opts);
+
+ return 0;
+}
+
/* Create a qemu-img virCommand from the supplied binary path,
* volume definitions and imgformat
@@ -1134,7 +1154,6 @@ virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr conn,
{
virCommandPtr cmd = NULL;
const char *type;
- char *opts = NULL;
struct _virStorageBackendQemuImgInfo info = {
.format = vol->target.format,
.path = vol->target.path,
@@ -1207,17 +1226,10 @@ virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr conn,
if (info.backingPath)
virCommandAddArgList(cmd, "-b", info.backingPath, NULL);
- if (info.format == VIR_STORAGE_FILE_QCOW2 && !info.compat &&
- imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT)
- info.compat = "0.10";
-
- if (virStorageBackendCreateQemuImgOpts(&opts, info) < 0) {
+ if (virStorageBackendCreateQemuImgSetOptions(cmd, imgformat, info) < 0) {
virCommandFree(cmd);
return NULL;
}
- if (opts)
- virCommandAddArgList(cmd, "-o", opts, NULL);
- VIR_FREE(opts);
if (info.inputPath)
virCommandAddArg(cmd, info.inputPath);
--
2.5.5
3120
days inactive
3121
days old
5 comments
2 participants
participants (2)
-
John Ferlan -
Peter Krempa