7:01 a.m.
The documentation states that for shallow block copy the image has to
have the same guest visible content as backing file of the current
image. This condition can be achieved also with a raw file (or a qcow
without a backing file) so remove the condition that would disallow it.
(This patch additionally fixes crash described in
https://bugzilla.redhat.com/show_bug.cgi?id=1215569 since it removes
the code)
---
src/qemu/qemu_driver.c | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 70bf7aa..f979d33 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16815,16 +16815,6 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm,
if (qemuDomainDetermineDiskChain(driver, vm, disk, false, true) < 0)
goto endjob;
- if ((flags & VIR_DOMAIN_BLOCK_COPY_SHALLOW) &&
- mirror->format == VIR_STORAGE_FILE_RAW &&
- disk->src->backingStore->path) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("disk '%s' has backing file, so raw shallow copy
"
- "is not possible"),
- disk->dst);
- goto endjob;
- }
-
/* Prepare the destination file. */
/* XXX Allow non-file mirror destinations */
if (!virStorageSourceIsLocalStorage(mirror)) {
--
2.3.5
9:11 a.m.
On 04/27/2015 08:01 PM, Peter Krempa wrote:
The documentation states that for shallow block copy the image has
to
have the same guest visible content as backing file of the current
image. This condition can be achieved also with a raw file (or a qcow
without a backing file) so remove the condition that would disallow it.
(This patch additionally fixes crash described in
https://bugzilla.redhat.com/show_bug.cgi?id=1215569 since it removes
the code)
---
src/qemu/qemu_driver.c | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 70bf7aa..f979d33 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16815,16 +16815,6 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm,
if (qemuDomainDetermineDiskChain(driver, vm, disk, false, true) < 0)
goto endjob;
- if ((flags & VIR_DOMAIN_BLOCK_COPY_SHALLOW) &&
- mirror->format == VIR_STORAGE_FILE_RAW &&
- disk->src->backingStore->path) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("disk '%s' has backing file, so raw shallow copy
"
- "is not possible"),
- disk->dst);
- goto endjob;
- }
-
Although a shallow blockcopy of file without backing file is
semantically correct, but still feel a little weird.
And, a shallow blockcommit of file without backing file will failed with
error
"error: invalid argument: top '/var/lib/libvirt/images/raw.img' in chain
for 'vda' has no backing file"
Should libvirt post error when try a shallow blockcopy of file without
backing file, just as shallow blockcommit?
/* Prepare the destination file. */
/* XXX Allow non-file mirror destinations */
if (!virStorageSourceIsLocalStorage(mirror)) {
--
Regards
shyu
10:38 a.m.
On Mon, Apr 27, 2015 at 22:11:40 +0800, Shanzhi Yu wrote:
On 04/27/2015 08:01 PM, Peter Krempa wrote:
> The documentation states that for shallow block copy the image has to
> have the same guest visible content as backing file of the current
> image. This condition can be achieved also with a raw file (or a qcow
> without a backing file) so remove the condition that would disallow it.
>
> (This patch additionally fixes crash described in
> https://bugzilla.redhat.com/show_bug.cgi?id=1215569 since it removes
> the code)
> ---
> src/qemu/qemu_driver.c | 10 ----------
> 1 file changed, 10 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 70bf7aa..f979d33 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -16815,16 +16815,6 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm,
> if (qemuDomainDetermineDiskChain(driver, vm, disk, false, true) < 0)
> goto endjob;
>
> - if ((flags & VIR_DOMAIN_BLOCK_COPY_SHALLOW) &&
> - mirror->format == VIR_STORAGE_FILE_RAW &&
> - disk->src->backingStore->path) {
> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> - _("disk '%s' has backing file, so raw shallow
copy "
> - "is not possible"),
> - disk->dst);
> - goto endjob;
> - }
> -
Although a shallow blockcopy of file without backing file is
semantically correct, but still feel a little weird.
And, a shallow blockcommit of file without backing file will failed with
error
"error: invalid argument: top '/var/lib/libvirt/images/raw.img' in chain
for 'vda' has no backing file"
Should libvirt post error when try a shallow blockcopy of file without
backing file, just as shallow blockcommit?
I cannot reproduce the error above, could you please post steps to do
that? or perhaps debug log from libvirt?
When copying from a single layer image with the --shallow flag I'm able
to successfully copy both into a raw and a qcow2 file.
> /* Prepare the destination file. */
> /* XXX Allow non-file mirror destinations */
> if (!virStorageSourceIsLocalStorage(mirror)) {
Peter
11:08 p.m.
On 04/27/2015 11:38 PM, Peter Krempa wrote:
On Mon, Apr 27, 2015 at 22:11:40 +0800, Shanzhi Yu wrote:
>
> On 04/27/2015 08:01 PM, Peter Krempa wrote:
>> The documentation states that for shallow block copy the image has to
>> have the same guest visible content as backing file of the current
>> image. This condition can be achieved also with a raw file (or a qcow
>> without a backing file) so remove the condition that would disallow it.
>>
>> (This patch additionally fixes crash described in
>> https://bugzilla.redhat.com/show_bug.cgi?id=1215569 since it removes
>> the code)
>> ---
>> src/qemu/qemu_driver.c | 10 ----------
>> 1 file changed, 10 deletions(-)
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index 70bf7aa..f979d33 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -16815,16 +16815,6 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm,
>> if (qemuDomainDetermineDiskChain(driver, vm, disk, false, true) < 0)
>> goto endjob;
>>
>> - if ((flags & VIR_DOMAIN_BLOCK_COPY_SHALLOW) &&
>> - mirror->format == VIR_STORAGE_FILE_RAW &&
>> - disk->src->backingStore->path) {
>> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> - _("disk '%s' has backing file, so raw
shallow copy "
>> - "is not possible"),
>> - disk->dst);
>> - goto endjob;
>> - }
>> -
> Although a shallow blockcopy of file without backing file is
> semantically correct, but still feel a little weird.
> And, a shallow blockcommit of file without backing file will failed with
> error
> "error: invalid argument: top '/var/lib/libvirt/images/raw.img' in
chain
> for 'vda' has no backing file"
>
> Should libvirt post error when try a shallow blockcopy of file without
> backing file, just as shallow blockcommit?
I cannot reproduce the error above, could you please post steps to do
that? or perhaps debug log from libvirt?
It is easy to reproduce. Suppose live guest xml looks like:
# virsh dumpxml vm1|grep disk -A 8
<disk type='file' device='disk'>
<driver name='qemu' type='qcow2'/>
<source file='/var/lib/libvirt/images/raw.1430143276'/>
<backingStore type='file' index='1'>
<format type='raw'/>
<source file='/var/lib/libvirt/images/raw.img'/>
<backingStore/>
</backingStore>
<target dev='vda' bus='virtio'/>
<alias name='virtio-disk0'/>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x03'
function='0x0'/>
</disk>
Then try a shallow blockcommit of file has no backing file
# virsh blockcommit vm1 vda --top vda[1] --shallow
error: invalid argument: top '/var/lib/libvirt/images/raw.img' in chain
for 'vda' has no backing file
When copying from a single layer image with the --shallow flag I'm able
to successfully copy both into a raw and a qcow2 file.
I also can do it.
But a shallow block copy of file which has backing file into a raw file
does not make any sense, right?
>> /* Prepare the destination file. */
>> /* XXX Allow non-file mirror destinations */
>> if (!virStorageSourceIsLocalStorage(mirror)) {
Peter
--
Regards
shyu
9:40 p.m.
On 04/27/2015 10:08 PM, Shanzhi Yu wrote:
>> Should libvirt post error when try a shallow blockcopy of
file without
>> backing file, just as shallow blockcommit?
> I cannot reproduce the error above, could you please post steps to do
> that? or perhaps debug log from libvirt?
It is easy to reproduce. Suppose live guest xml looks like:
# virsh dumpxml vm1|grep disk -A 8
<disk type='file' device='disk'>
<driver name='qemu' type='qcow2'/>
<source file='/var/lib/libvirt/images/raw.1430143276'/>
<backingStore type='file' index='1'>
<format type='raw'/>
<source file='/var/lib/libvirt/images/raw.img'/>
<backingStore/>
</backingStore>
<target dev='vda' bus='virtio'/>
<alias name='virtio-disk0'/>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x03'
function='0x0'/>
</disk>
Then try a shallow blockcommit of file has no backing file
But a shallow block copy of file which has backing file into a raw
file
does not make any sense, right?
A shallow block copy of:
A <- B
into a pre-existing file C, where C starts life with contents identical
to A, makes sense.
A shallow block copy where _libvirt_ creates C does not make sense. The
key is that the moment you use the REUSE_EXTERNAL flag, _you_ are now
responsible for providing a valid destination (and anything is
supported; libvirt should not get in your way).
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
2:06 p.m.
On 04/27/2015 06:01 AM, Peter Krempa wrote:
The documentation states that for shallow block copy the image has
to
have the same guest visible content as backing file of the current
image. This condition can be achieved also with a raw file (or a qcow
without a backing file) so remove the condition that would disallow it.
(This patch additionally fixes crash described in
https://bugzilla.redhat.com/show_bug.cgi?id=1215569 since it removes
the code)
---
src/qemu/qemu_driver.c | 10 ----------
1 file changed, 10 deletions(-)
This removes a bit of a safety valve that tries to prevent users from
accidentally creating corrupt images when they use the API incorrectly,
but as the safety valve didn't always work and we could actually crash,
and as the documentation has already warned the user about using it
correctly, I can probably live with it.
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 70bf7aa..f979d33 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16815,16 +16815,6 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm,
if (qemuDomainDetermineDiskChain(driver, vm, disk, false, true) < 0)
goto endjob;
- if ((flags & VIR_DOMAIN_BLOCK_COPY_SHALLOW) &&
- mirror->format == VIR_STORAGE_FILE_RAW &&
- disk->src->backingStore->path) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("disk '%s' has backing file, so raw shallow copy
"
- "is not possible"),
- disk->dst);
- goto endjob;
- }
However, one thing I'm not sure about: a shallow block copy with the
REUSE_EXTERNAL flag always makes sense (raw or not, because the user of
that flag knows what they are doing). But a shallow block copy without
reusing an external file does not make sense - if libvirt is creating
the destination file, then we KNOW that the destination file does not
match the guest contents (except for the corner case where the guest is
not using the block and therefore it remains in the default state of all
0s because it is unwritten). I'm not convinced yet that this is the
right patch.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
3498
days inactive
3500
days old
5 comments
3 participants
participants (3)
-
Eric Blake -
Peter Krempa -
Shanzhi Yu