3:19 p.m.
In commit fbcf7da95, a change was introduced that no longer allowed defining
volumes via XML with a capacity of '0'. Because we check for info.size_arg to be
non-zero, this use-case fails. This patch allows info.size_arg to be zero if no
backing store is specified.
Signed-off-by: Chris J Arges <chris.j.arges(a)canonical.com>
---
src/storage/storage_backend.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index ce59f63..c661662 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -1068,7 +1068,7 @@ virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr conn,
if (info.inputPath)
virCommandAddArg(cmd, info.inputPath);
virCommandAddArg(cmd, info.path);
- if (!info.inputPath && info.size_arg)
+ if (!info.inputPath && (info.size_arg || !info.backingPath))
virCommandAddArgFormat(cmd, "%lluK", info.size_arg);
return cmd;
--
1.9.1
3:19 p.m.
New subject: [libvirt] [PATCH 2/2] tests: add vol-qcow2-zerocapacity test to storagevolxml2argvtest
Add a testcase for the previous change to ensure zero capacity volumes can be
defined without a backing store.
Signed-off-by: Chris J Arges <chris.j.arges(a)canonical.com>
---
tests/storagevolxml2argvdata/qcow2-zerocapacity.argv | 1 +
tests/storagevolxml2argvtest.c | 3 +++
tests/storagevolxml2xmlin/vol-qcow2-zerocapacity.xml | 7 +++++++
3 files changed, 11 insertions(+)
create mode 100644 tests/storagevolxml2argvdata/qcow2-zerocapacity.argv
create mode 100644 tests/storagevolxml2xmlin/vol-qcow2-zerocapacity.xml
diff --git a/tests/storagevolxml2argvdata/qcow2-zerocapacity.argv
b/tests/storagevolxml2argvdata/qcow2-zerocapacity.argv
new file mode 100644
index 0000000..d83b08b
--- /dev/null
+++ b/tests/storagevolxml2argvdata/qcow2-zerocapacity.argv
@@ -0,0 +1 @@
+qemu-img create -f qcow2 -o compat=0.10 0K
diff --git a/tests/storagevolxml2argvtest.c b/tests/storagevolxml2argvtest.c
index ed002ce..393123b 100644
--- a/tests/storagevolxml2argvtest.c
+++ b/tests/storagevolxml2argvtest.c
@@ -284,6 +284,9 @@ mymain(void)
DO_TEST("pool-dir", "vol-qcow2-nocapacity",
"pool-dir", "vol-file",
"qcow2-nocapacity-convert-prealloc", flags, FMT_OPTIONS);
+ DO_TEST("pool-dir", "vol-qcow2-zerocapacity",
+ NULL, NULL,
+ "qcow2-zerocapacity", 0, FMT_COMPAT);
DO_TEST_FULL(false, VIR_VOL_XML_PARSE_OPT_CAPACITY,
"pool-dir", "vol-qcow2-nocapacity-backing", NULL,
NULL,
"qcow2-nocapacity", 0, FMT_OPTIONS);
diff --git a/tests/storagevolxml2xmlin/vol-qcow2-zerocapacity.xml
b/tests/storagevolxml2xmlin/vol-qcow2-zerocapacity.xml
new file mode 100644
index 0000000..1d1e6de
--- /dev/null
+++ b/tests/storagevolxml2xmlin/vol-qcow2-zerocapacity.xml
@@ -0,0 +1,7 @@
+<volume>
+ <name>OtherDemo.img</name>
+ <target>
+ <format type="qcow2"/>
+ </target>
+ <capacity>0</capacity>
+</volume>
--
1.9.1
3:01 p.m.
On 06/30/2015 04:19 PM, Chris J Arges wrote:
In commit fbcf7da95, a change was introduced that no longer allowed
defining
volumes via XML with a capacity of '0'. Because we check for info.size_arg to be
non-zero, this use-case fails. This patch allows info.size_arg to be zero if no
backing store is specified.
Signed-off-by: Chris J Arges <chris.j.arges(a)canonical.com>
---
src/storage/storage_backend.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Found an orphaned patch....
Since that's Jan's fix - he should have a say here, but it seems you're
correct. Also of note is commit id '155ca616e' which changed the check
to add that check for size_arg being non-zero.
The ordering of the date of when the change was created is odd. Commit
id 'fbcf7da95' was committed after '155ca616e', but authored months
before.
I've copied Jan to get his thoughts.
John
diff --git a/src/storage/storage_backend.c
b/src/storage/storage_backend.c
index ce59f63..c661662 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -1068,7 +1068,7 @@ virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr conn,
if (info.inputPath)
virCommandAddArg(cmd, info.inputPath);
virCommandAddArg(cmd, info.path);
- if (!info.inputPath && info.size_arg)
+ if (!info.inputPath && (info.size_arg || !info.backingPath))
virCommandAddArgFormat(cmd, "%lluK", info.size_arg);
return cmd;
6:10 a.m.
On Thu, Jul 23, 2015 at 04:01:22PM -0400, John Ferlan wrote:
On 06/30/2015 04:19 PM, Chris J Arges wrote:
> In commit fbcf7da95, a change was introduced that no longer allowed defining
As John pointed out, it was commit 155ca616e that changed it.
> volumes via XML with a capacity of '0'. Because we check
for info.size_arg to be
> non-zero, this use-case fails. This patch allows info.size_arg to be zero if no
> backing store is specified.
>
I must admit I do not see the use case here.
Should we allow zero-sized volumes with backing stores too?
Other than that the fix looks good to me.
> Signed-off-by: Chris J Arges
<chris.j.arges(a)canonical.com>
> ---
> src/storage/storage_backend.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Found an orphaned patch....
Nice catch, it must've slipped through the cracks.
Since that's Jan's fix - he should have a say here, but it
seems you're
correct. Also of note is commit id '155ca616e' which changed the check
to add that check for size_arg being non-zero.
The ordering of the date of when the change was created is odd. Commit
id 'fbcf7da95' was committed after '155ca616e', but authored months
before.
It was resting in my local git for a while, then rebased against master.
The AuthorDate only says when the commit was started and does not get
updated with every amend/rebase.
Jan
8:25 a.m.
On 07/24/2015 06:10 AM, Ján Tomko wrote:
On Thu, Jul 23, 2015 at 04:01:22PM -0400, John Ferlan wrote:
>
>
> On 06/30/2015 04:19 PM, Chris J Arges wrote:
>> In commit fbcf7da95, a change was introduced that no longer allowed defining
As John pointed out, it was commit 155ca616e that changed it.
>> volumes via XML with a capacity of '0'. Because we check for
info.size_arg to be
>> non-zero, this use-case fails. This patch allows info.size_arg to be zero if no
>> backing store is specified.
>>
I must admit I do not see the use case here.
Should we allow zero-sized volumes with backing stores too?
Other than that the fix looks good to me.
>> Signed-off-by: Chris J Arges <chris.j.arges(a)canonical.com>
>> ---
>> src/storage/storage_backend.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>
> Found an orphaned patch....
>
Nice catch, it must've slipped through the cracks.
> Since that's Jan's fix - he should have a say here, but it seems you're
> correct. Also of note is commit id '155ca616e' which changed the check
> to add that check for size_arg being non-zero.
>
> The ordering of the date of when the change was created is odd. Commit
> id 'fbcf7da95' was committed after '155ca616e', but authored months
before.
It was resting in my local git for a while, then rebased against master.
The AuthorDate only says when the commit was started and does not get
updated with every amend/rebase.
Jan
Hey thanks for taking a look.
At this point do you need me to re-send the patch with any updates?
--chris
10:31 a.m.
On 07/24/2015 09:25 AM, Chris J Arges wrote:
On 07/24/2015 06:10 AM, Ján Tomko wrote:
> On Thu, Jul 23, 2015 at 04:01:22PM -0400, John Ferlan wrote:
>>
>>
>> On 06/30/2015 04:19 PM, Chris J Arges wrote:
>>> In commit fbcf7da95, a change was introduced that no longer allowed defining
>
> As John pointed out, it was commit 155ca616e that changed it.
>
>>> volumes via XML with a capacity of '0'. Because we check for
info.size_arg to be
>>> non-zero, this use-case fails. This patch allows info.size_arg to be zero if
no
>>> backing store is specified.
>>>
>
> I must admit I do not see the use case here.
> Should we allow zero-sized volumes with backing stores too?
> Other than that the fix looks good to me.
>
>>> Signed-off-by: Chris J Arges <chris.j.arges(a)canonical.com>
>>> ---
>>> src/storage/storage_backend.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>
>> Found an orphaned patch....
>>
>
> Nice catch, it must've slipped through the cracks.
>
>> Since that's Jan's fix - he should have a say here, but it seems
you're
>> correct. Also of note is commit id '155ca616e' which changed the check
>> to add that check for size_arg being non-zero.
>>
>> The ordering of the date of when the change was created is odd. Commit
>> id 'fbcf7da95' was committed after '155ca616e', but authored
months before.
>
> It was resting in my local git for a while, then rebased against master.
> The AuthorDate only says when the commit was started and does not get
> updated with every amend/rebase.
>
> Jan
>
Hey thanks for taking a look.
At this point do you need me to re-send the patch with any updates?
No need to do anything - I updated the commit message to use '155ca616e'
and pushed the series.
John
3411
days inactive
3435
days old
5 comments
3 participants
participants (3)
-
Chris J Arges -
John Ferlan -
Ján Tomko