12:26 a.m.
I tried the new snapshot function implemented by Eric Blake.
It works very well for QCOW2 disk image system.
But I often use LVM2 volume for QEMU virtual machines and tried to take
disk snapshot by virsh command ( snapshot-create DOMNAME --disk-only).
So, finally qemu monitor command 'snapshot_blkdev' accepts the LVM2
volume and create QCOW2 snapshot image. In addition, domain's
configuration file is replaced to use snapshot disk image instead of
LVM2 volume.
configuration file
from
....
<disk type='block' device='disk>
<driver name='qemu' type='raw' cache='none'/>
<source dev='dev/VG1/LVM2_dom'/>
....
to
<disk type='block' device='disk>
<driver name='qemu' type='qcow2' cache='none'/>
<source dev='dev/VG1/LVM2_dom.1317357844'/>
After then, the domain runs well till it is shutdowned. I started the
domain, but it does not with following error
virtsh # start LVM2_dom
error: Failed to start domain LVM2_dom
error: 内部エラー Process exited while reading console log output: char
device redirected to /dev/pts/7
qemu: could not open disk image /dev/VG1/LVM2_dom.1317357844: Invalid
argument.
I think that if the volume but qcow2 is given libvirt should be refuse,
e.g. in qemuDomainSnapshotCreateDiskActive() with voulme driver type.
But currently the structures concerning with snapshot or disk has no
member to hold such a volume driver information. In addition, as we want
to add the LVM2 and other volume snapshot function, we hope you add its
information and fix.
Regards
MATSUDA Daiki
1 a.m.
(2011/09/30 14:26), MATSUDA, Daiki wrote:
I tried the new snapshot function implemented by Eric Blake.
It works very well for QCOW2 disk image system.
But I often use LVM2 volume for QEMU virtual machines and tried to take
disk snapshot by virsh command ( snapshot-create DOMNAME --disk-only).
So, finally qemu monitor command 'snapshot_blkdev' accepts the LVM2
volume and create QCOW2 snapshot image. In addition, domain's
configuration file is replaced to use snapshot disk image instead of
LVM2 volume.
configuration file
from
....
<disk type='block' device='disk>
<driver name='qemu' type='raw' cache='none'/>
<source dev='dev/VG1/LVM2_dom'/>
....
to
<disk type='block' device='disk>
<driver name='qemu' type='qcow2' cache='none'/>
<source dev='dev/VG1/LVM2_dom.1317357844'/>
After then, the domain runs well till it is shutdowned. I started the
domain, but it does not with following error
virtsh # start LVM2_dom
error: Failed to start domain LVM2_dom
error: 内部エラー Process exited while reading console log output: char
Sorry, upper is
error: internal error Process exited while reading console log output: char
device redirected to /dev/pts/7
qemu: could not open disk image /dev/VG1/LVM2_dom.1317357844: Invalid
argument.
I think that if the volume but qcow2 is given libvirt should be refuse,
e.g. in qemuDomainSnapshotCreateDiskActive() with voulme driver type.
But currently the structures concerning with snapshot or disk has no
member to hold such a volume driver information. In addition, as we want
to add the LVM2 and other volume snapshot function, we hope you add its
information and fix.
Regards
MATSUDA Daiki
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
3:46 p.m.
On 09/29/2011 11:26 PM, MATSUDA, Daiki wrote:
I tried the new snapshot function implemented by Eric Blake.
It works very well for QCOW2 disk image system.
But I often use LVM2 volume for QEMU virtual machines and tried to take
disk snapshot by virsh command ( snapshot-create DOMNAME --disk-only).
So, finally qemu monitor command 'snapshot_blkdev' accepts the LVM2
volume and create QCOW2 snapshot image. In addition, domain's
configuration file is replaced to use snapshot disk image instead of
LVM2 volume.
It sounds like virsh did what it was told, but that you told it so
little information that it had to fill in the gaps and choose its own
destination file name (hence the .1317357844 suffix after the action).
Your situation sounds like a case where you may want a bit more control
over the destination file name.
configuration file
from
....
<disk type='block' device='disk>
<driver name='qemu' type='raw' cache='none'/>
<source dev='dev/VG1/LVM2_dom'/>
....
to
<disk type='block' device='disk>
<driver name='qemu' type='qcow2' cache='none'/>
<source dev='dev/VG1/LVM2_dom.1317357844'/>
Are you pasting literal chunks, or retyping this? You're missing the /
in front of dev/VG1, so I can't help but wonder what else might have
been mistyped.
After then, the domain runs well till it is shutdowned.
I'm surprised that it got that far - generally, libvirt can't create
random regular files under the /dev/VG1/ device-mapper hierarchy, and if
a file can't be created, then what was open() doing, and what did qemu
actually do? It may be worth looking at lsof says that qemu has open,
if you still have it running. Or it may be that you've found a bug in
libvirt and/or qemu for not accurately reporting failure to create the
snapshot image.
I think we need to step back a bit and look at the bigger picture. Do
you want your new qcow2 file to be its own LVM volume (in which case,
I'd suggest that you pre-create the LVM volume, and pass in the file
name within /dev/VG1/ of the existing block device to use), or are you
okay with it being a regular file (in which case, I'd suggest that you
do not pre-create the file, but that you still pass in the desired
filename to some absolute path that lives outside of /dev/)?
Either way, it sounds like you do _not_ want libvirt to be generating
its own filename, since libvirt only knows how to generate a name in the
same directory as the snapshot is taking place, but your lvm is in a
special directory. To do this, you either need to create an XML file
yourself, and call 'virsh snapshot-create dom --disk-only file', or you
need to have virsh create the xml for you with 'virsh snapshot-create-as
dom --disk-only vda,file=/path/to/file'. You can see the xml that
snapshot-create-as would generate (in case you need to further fine-tune
it for your own use in snapshot-create) via the --print-xml option.
I started the
domain, but it does not with following error
virtsh # start LVM2_dom
error: Failed to start domain LVM2_dom
error: 内部エラー Process exited while reading console log output: char
device redirected to /dev/pts/7
qemu: could not open disk image /dev/VG1/LVM2_dom.1317357844: Invalid
argument.
That makes sense, if that file doesn't exist (but why qemu didn't reject
the snapshot in the first place still remains to be investigated).
I think that if the volume but qcow2 is given libvirt should be refuse,
No, qemu does just fine with a non-qcow2 backing file. The real problem
here is that the new qcow2 file was not created, not the type of the
original file.
e.g. in qemuDomainSnapshotCreateDiskActive() with voulme driver
type.
But currently the structures concerning with snapshot or disk has no
member to hold such a volume driver information. In addition, as we want
to add the LVM2 and other volume snapshot function, we hope you add its
information and fix.
Yes, I have much longer-term plans for refactoring device snapshots to
move the work into more virStorageVolPtr operations, and teach
virDomainSnapshotCreateXML to reuse virStorageVol actions rather than
hard-coding the actions itself, at which point we can then use lvm
snapshots rather than qcow2 snapshots. But that's a lot more effort, so
no promise of how long it will take me to get to that point.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
7:23 p.m.
On 09/29/2011 11:26 PM, MATSUDA, Daiki wrote:
> I tried the new snapshot function implemented by Eric Blake.
>
> It works very well for QCOW2 disk image system.
> But I often use LVM2 volume for QEMU virtual machines and tried to take
> disk snapshot by virsh command ( snapshot-create DOMNAME --disk-only).
> So, finally qemu monitor command 'snapshot_blkdev' accepts the LVM2
> volume and create QCOW2 snapshot image. In addition, domain's
> configuration file is replaced to use snapshot disk image instead of
> LVM2 volume.
It sounds like virsh did what it was told, but that you told it so
little information that it had to fill in the gaps and choose its own
destination file name (hence the .1317357844 suffix after the action).
Your situation sounds like a case where you may want a bit more control
over the destination file name.
virsh outputs
virsh # snapshot-create LVM2_dom --disk-only
Domain snapshot 1317357844 created
And I confirmed that the qcow2 image file is created under /dev/VG1
# file /dev/VG1/LVM2_dom.1317357844
/dev/VG1/LVM2_dom.1317686816: Qemu Image, Format: Qcow , Version: 2
> configuration file
> from
> ....
> <disk type='block' device='disk>
> <driver name='qemu' type='raw' cache='none'/>
> <source dev='dev/VG1/LVM2_dom'/>
> ....
>
> to
> <disk type='block' device='disk>
> <driver name='qemu' type='qcow2' cache='none'/>
> <source dev='dev/VG1/LVM2_dom.1317357844'/>
Are you pasting literal chunks, or retyping this? You're missing the /
in front of dev/VG1, so I can't help but wonder what else might have
been mistyped.
I am sorry. They are my mistyping and correct is '/dev/VG1/LVM2_dom' and
'/dev/VG1/LVM2_dom.1317357844'.
> After then, the domain runs well till it is shutdowned.
I'm surprised that it got that far - generally, libvirt can't create
random regular files under the /dev/VG1/ device-mapper hierarchy, and if
a file can't be created, then what was open() doing, and what did qemu
actually do? It may be worth looking at lsof says that qemu has open,
if you still have it running. Or it may be that you've found a bug in
libvirt and/or qemu for not accurately reporting failure to create the
snapshot image.
But in reality the file is created by qemu-kvm with snapshot_blkdev in
qemu-monitor command. I use libvirt-0.9.6 and qemu-kvm-0.12.12.1.2-2.160
and August's snapshot.
I think we need to step back a bit and look at the bigger picture.
Do
you want your new qcow2 file to be its own LVM volume (in which case,
I'd suggest that you pre-create the LVM volume, and pass in the file
name within /dev/VG1/ of the existing block device to use), or are you
okay with it being a regular file (in which case, I'd suggest that you
do not pre-create the file, but that you still pass in the desired
filename to some absolute path that lives outside of /dev/)?
No, I do not want qcow2 file on LVM volume. I found the bug at simply
tesing. I will never create the snapshot with 'snapshot-create ...
--disk-only' for LVM2 volume, but users will try... So, I think that it
is better not to refuse in libvirt.
Either way, it sounds like you do _not_ want libvirt to be
generating
its own filename, since libvirt only knows how to generate a name in the
same directory as the snapshot is taking place, but your lvm is in a
special directory. To do this, you either need to create an XML file
yourself, and call 'virsh snapshot-create dom --disk-only file', or you
need to have virsh create the xml for you with 'virsh snapshot-create-as
dom --disk-only vda,file=/path/to/file'. You can see the xml that
snapshot-create-as would generate (in case you need to further fine-tune
it for your own use in snapshot-create) via the --print-xml option.
> I started the
> domain, but it does not with following error
> virtsh # start LVM2_dom
> error: Failed to start domain LVM2_dom
> error: 内部エラー Process exited while reading console log output: char
> device redirected to /dev/pts/7
> qemu: could not open disk image /dev/VG1/LVM2_dom.1317357844: Invalid
> argument.
That makes sense, if that file doesn't exist (but why qemu didn't reject
the snapshot in the first place still remains to be investigated).
>
> I think that if the volume but qcow2 is given libvirt should be refuse,
No, qemu does just fine with a non-qcow2 backing file. The real problem
here is that the new qcow2 file was not created, not the type of the
original file.
At least its phenomenon is reproduced easily. So I hope you test.
> e.g. in qemuDomainSnapshotCreateDiskActive() with voulme driver
type.
> But currently the structures concerning with snapshot or disk has no
> member to hold such a volume driver information. In addition, as we want
> to add the LVM2 and other volume snapshot function, we hope you add its
> information and fix.
Yes, I have much longer-term plans for refactoring device snapshots to
move the work into more virStorageVolPtr operations, and teach
virDomainSnapshotCreateXML to reuse virStorageVol actions rather than
hard-coding the actions itself, at which point we can then use lvm
snapshots rather than qcow2 snapshots. But that's a lot more effort, so
no promise of how long it will take me to get to that point.
Okay, if possible, I hope you to show the schedule ?
Regards
MATSUDA, Daiki
10:14 p.m.
Hi, Eric.
On 09/29/2011 11:26 PM, MATSUDA, Daiki wrote:
> I tried the new snapshot function implemented by Eric Blake.
>
> It works very well for QCOW2 disk image system.
> But I often use LVM2 volume for QEMU virtual machines and tried to take
> disk snapshot by virsh command ( snapshot-create DOMNAME --disk-only).
> So, finally qemu monitor command 'snapshot_blkdev' accepts the LVM2
> volume and create QCOW2 snapshot image. In addition, domain's
> configuration file is replaced to use snapshot disk image instead of
> LVM2 volume.
It sounds like virsh did what it was told, but that you told it so
little information that it had to fill in the gaps and choose its own
destination file name (hence the .1317357844 suffix after the action).
Your situation sounds like a case where you may want a bit more control
over the destination file name.
I made the patch for the problem to take the snapshot but qcow2. It use
the 'qemu-img' command, I know that it is not essential solution. But I
think it is better than to link qemu's libraries.
Regards
MATSUDA Daiki
4:07 a.m.
On Mon, Nov 07, 2011 at 01:14:12PM +0900, MATSUDA, Daiki wrote:
Hi, Eric.
> On 09/29/2011 11:26 PM, MATSUDA, Daiki wrote:
>> I tried the new snapshot function implemented by Eric Blake.
>>
>> It works very well for QCOW2 disk image system.
>> But I often use LVM2 volume for QEMU virtual machines and tried to take
>> disk snapshot by virsh command ( snapshot-create DOMNAME --disk-only).
>> So, finally qemu monitor command 'snapshot_blkdev' accepts the LVM2
>> volume and create QCOW2 snapshot image. In addition, domain's
>> configuration file is replaced to use snapshot disk image instead of
>> LVM2 volume.
>
> It sounds like virsh did what it was told, but that you told it so
> little information that it had to fill in the gaps and choose its own
> destination file name (hence the .1317357844 suffix after the action).
> Your situation sounds like a case where you may want a bit more control
> over the destination file name.
I made the patch for the problem to take the snapshot but qcow2. It use
the 'qemu-img' command, I know that it is not essential solution. But I
think it is better than to link qemu's libraries.
There is an internal libvirt API for disk formats in src/util/storage_file.h
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
10:38 a.m.
On 11/06/2011 09:14 PM, MATSUDA, Daiki wrote:
I made the patch for the problem to take the snapshot but qcow2. It
use
the 'qemu-img' command, I know that it is not essential solution. But I
think it is better than to link qemu's libraries.
Regards
@@ -9002,6 +9021,13 @@ qemuDomainSnapshotCreateSingleDiskActive
return -1;
}
+ ret = qemuDomainSnapshotCheckSrcQcow2(src->disk);
+ if (ret) {
+ qemuReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("src image is not qcow2 format"));
+ return ret;
+ }
NACK. There is nothing inherently wrong with the source file not being
a qcow2 file. The whole point of creating a runtime snapshot is that
the original file (of _any_ format) becomes the backing file of a new
qcow2 file, so that the original file now serves as the snapshot.
Forbidding a live snapshot of a raw source file interferes with this intent.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
10:08 p.m.
On 11/06/2011 09:14 PM, MATSUDA, Daiki wrote:
> I made the patch for the problem to take the snapshot but qcow2. It use
> the 'qemu-img' command, I know that it is not essential solution. But I
> think it is better than to link qemu's libraries.
>
> Regards
> @@ -9002,6 +9021,13 @@ qemuDomainSnapshotCreateSingleDiskActive
> return -1;
> }
>
> + ret = qemuDomainSnapshotCheckSrcQcow2(src->disk);
> + if (ret) {
> + qemuReportError(VIR_ERR_INTERNAL_ERROR,
> + "%s", _("src image is not qcow2
format"));
> + return ret;
> + }
NACK. There is nothing inherently wrong with the source file not being
a qcow2 file. The whole point of creating a runtime snapshot is that
the original file (of _any_ format) becomes the backing file of a new
qcow2 file, so that the original file now serves as the snapshot.
Forbidding a live snapshot of a raw source file interferes with this intent.
I have some tested since you replied. Certainly other image file, e.g.
raw type, has no problem after snapshot is taken in qcow2 format.
But in the case that direct disk block, e.g. /dev/sdc, is given for the
guest OS, the same error occurs. It may not be accepted by qemu-kvm.
I do not understand which qemu-kvm or libvirt should be fixed. But at
least libvirt should be stop to take snapshot for block device with
following patch.
Regards
MATSUDA Daiki
5:02 p.m.
On 11/13/2011 09:08 PM, MATSUDA, Daiki wrote:
> NACK. There is nothing inherently wrong with the source file not
being
> a qcow2 file. The whole point of creating a runtime snapshot is that
> the original file (of _any_ format) becomes the backing file of a new
> qcow2 file, so that the original file now serves as the snapshot.
> Forbidding a live snapshot of a raw source file interferes with this intent.
>
I have some tested since you replied. Certainly other image file, e.g.
raw type, has no problem after snapshot is taken in qcow2 format.
But in the case that direct disk block, e.g. /dev/sdc, is given for the
guest OS, the same error occurs. It may not be accepted by qemu-kvm.
I'm not following you. As I already said, there's nothing wrong with a
direct disk block as the backing file of a qcow2 image.
I do not understand which qemu-kvm or libvirt should be fixed. But at
least libvirt should be stop to take snapshot for block device with
following patch.
I still say NACK. Your patch is merely attempting to forbid a useful
case, rather than fix a demonstrated problem.
If I recall correctly, you started this thread when you used 'virsh
snapshot-create --disk-only' without arguments, and thus fell victim to
virsh picking the snapshot name for you, but trying to pick that name
under /dev instead of a more typical location for a non-device file.
But that's a usage error in you not taking time to provide virsh with
the alternate name to use for the qcow2 file, and not a flaw that needs
correcting in libvirt itself. Or even if there is a flaw in libvirt,
the fix is not to forbid snapshots of a raw block device, but to be
smarter about ensuring that any generated qcow2 file name is likely to
be correct (a qcow2 file created under /dev probably only makes sense if
the qcow2 data is being written atop a pre-existing block device, rather
than trying to use open(O_CREAT) to create a regular file in /dev).
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
9:35 a.m.
On Mon, Nov 14, 2011 at 04:02:21PM -0700, Eric Blake wrote:
On 11/13/2011 09:08 PM, MATSUDA, Daiki wrote:
>> NACK. There is nothing inherently wrong with the source file not being
>> a qcow2 file. The whole point of creating a runtime snapshot is that
>> the original file (of _any_ format) becomes the backing file of a new
>> qcow2 file, so that the original file now serves as the snapshot.
>> Forbidding a live snapshot of a raw source file interferes with this intent.
>>
>
> I have some tested since you replied. Certainly other image file, e.g.
> raw type, has no problem after snapshot is taken in qcow2 format.
>
> But in the case that direct disk block, e.g. /dev/sdc, is given for the
> guest OS, the same error occurs. It may not be accepted by qemu-kvm.
I'm not following you. As I already said, there's nothing wrong with a
direct disk block as the backing file of a qcow2 image.
>
> I do not understand which qemu-kvm or libvirt should be fixed. But at
> least libvirt should be stop to take snapshot for block device with
> following patch.
I still say NACK. Your patch is merely attempting to forbid a useful
case, rather than fix a demonstrated problem.
Matsuda-san,
I must admit that I don't understand the problem that you are
attempting to solve either. I have reread the thread from the
beginning, but I don't understand what your goal is. Could you
explain to us more about your use case?
Regards,
Dave Allan
If I recall correctly, you started this thread when you used
'virsh
snapshot-create --disk-only' without arguments, and thus fell victim to
virsh picking the snapshot name for you, but trying to pick that name
under /dev instead of a more typical location for a non-device file.
But that's a usage error in you not taking time to provide virsh with
the alternate name to use for the qcow2 file, and not a flaw that needs
correcting in libvirt itself. Or even if there is a flaw in libvirt,
the fix is not to forbid snapshots of a raw block device, but to be
smarter about ensuring that any generated qcow2 file name is likely to
be correct (a qcow2 file created under /dev probably only makes sense if
the qcow2 data is being written atop a pre-existing block device, rather
than trying to use open(O_CREAT) to create a regular file in /dev).
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
4:48 p.m.
(2011/11/16 0:35), Dave Allan wrote:
On Mon, Nov 14, 2011 at 04:02:21PM -0700, Eric Blake wrote:
> On 11/13/2011 09:08 PM, MATSUDA, Daiki wrote:
>>> NACK. There is nothing inherently wrong with the source file not being
>>> a qcow2 file. The whole point of creating a runtime snapshot is that
>>> the original file (of _any_ format) becomes the backing file of a new
>>> qcow2 file, so that the original file now serves as the snapshot.
>>> Forbidding a live snapshot of a raw source file interferes with this intent.
>>>
>>
>> I have some tested since you replied. Certainly other image file, e.g.
>> raw type, has no problem after snapshot is taken in qcow2 format.
>>
>> But in the case that direct disk block, e.g. /dev/sdc, is given for the
>> guest OS, the same error occurs. It may not be accepted by qemu-kvm.
>
> I'm not following you. As I already said, there's nothing wrong with a
> direct disk block as the backing file of a qcow2 image.
>
>>
>> I do not understand which qemu-kvm or libvirt should be fixed. But at
>> least libvirt should be stop to take snapshot for block device with
>> following patch.
>
> I still say NACK. Your patch is merely attempting to forbid a useful
> case, rather than fix a demonstrated problem.
Matsuda-san,
I must admit that I don't understand the problem that you are
attempting to solve either. I have reread the thread from the
beginning, but I don't understand what your goal is. Could you
explain to us more about your use case?
Regards,
Dave Allan
Mr. Dave.
The problem is that the guest does not boot after 'virsh snapshot-create
.... --disk-only' under the specific situation.
I tested some cases and found it.
1. The guest use the raw disk volume or LVM2 volume, such as following
the config file using raw disk volume created by RHEL 6.1 Virt Manager.
<domain type='kvm'>
<name>abc</name>
<uuid>791ede5a-e617-ca6b-125d-4e9704a2ddc2</uuid>
<memory>524288</memory>
<currentMemory>524288</currentMemory>
<vcpu>1</vcpu>
<os>
<type arch='x86_64' machine='rhel6.1.0'>hvm</type>
<boot dev='hd'/>
</os>
<features>
<acpi/>
<apic/>
<pae/>
</features>
<clock offset='utc'/>
<on_poweroff>destroy</on_poweroff>
<on_reboot>restart</on_reboot>
<on_crash>restart</on_crash>
<devices>
<emulator>/usr/libexec/qemu-kvm</emulator>
<disk type='block' device='disk'>
<driver name='qemu' type='raw' cache='none'
io='native'/>
<source dev='/dev/sdc'/>
<target dev='vda' bus='virtio'/>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x05'
function='0x0'/>
</disk>
<disk type='block' device='cdrom'>
<driver name='qemu' type='raw'/>
<target dev='hdc' bus='ide'/>
<readonly/>
<address type='drive' controller='0' bus='1'
unit='0'/>
</disk>
<controller type='ide' index='0'>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x01'
function='0x1'/>
</controller>
<interface type='bridge'>
<mac address='52:54:00:0d:53:c6'/>
<source bridge='br0'/>
<model type='virtio'/>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x03'
function='0x0'/>
</interface>
<serial type='pty'>
<target port='0'/>
</serial>
<console type='pty'>
<target type='serial' port='0'/>
</console>
<input type='tablet' bus='usb'/>
<input type='mouse' bus='ps2'/>
<graphics type='vnc' port='-1' autoport='yes'/>
<sound model='ich6'>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x04'
function='0x0'/>
</sound>
<video>
<model type='cirrus' vram='9216' heads='1'/>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x02'
function='0x0'/>
</video>
<memballoon model='virtio'>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x06'
function='0x0'/>
</memballoon>
</devices>
</domain>
2. Do the 'virsh snapshot-create abc --disk-only' with libvirt-0.9.6 or
upper.
3. The guest runs well till it is shutdowned.
4. But the guest does not boot with following error.
virtsh # start abc
error: Failed to start domain abc
error: internal error Process exited while reading console log output: char
device redirected to /dev/pts/7
qemu: could not open disk image /dev/sdc.1317357844: Invalid
argument.
5. Then the config file has been replaced by libvirtd. Especially
difference is
<disk type='block' device='disk'>
<driver name='qemu' type='qcow2' cache='none'/>
<source dev='/dev/sdc.1317357844'/>
<target dev='vda' bus='virtio'/>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x05'
function='0x0'/>
</disk>
And 'snapshot-create ... --disk-only' works well for image format file,
e.g. qcow2, raw and etc.
So, I think that the restriction is needed for the taking snapshot
disk-only for the disk volume.
Regards
MATSUDA Daiki
> If I recall correctly, you started this thread when you used
'virsh
> snapshot-create --disk-only' without arguments, and thus fell victim to
> virsh picking the snapshot name for you, but trying to pick that name
> under /dev instead of a more typical location for a non-device file.
> But that's a usage error in you not taking time to provide virsh with
> the alternate name to use for the qcow2 file, and not a flaw that needs
> correcting in libvirt itself. Or even if there is a flaw in libvirt,
> the fix is not to forbid snapshots of a raw block device, but to be
> smarter about ensuring that any generated qcow2 file name is likely to
> be correct (a qcow2 file created under /dev probably only makes sense if
> the qcow2 data is being written atop a pre-existing block device, rather
> than trying to use open(O_CREAT) to create a regular file in /dev).
>
> --
> Eric Blake eblake(a)redhat.com +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>
> --
> libvir-list mailing list
> libvir-list(a)redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
5:18 p.m.
On 11/15/2011 03:48 PM, MATSUDA, Daiki wrote:
I tested some cases and found it.
1. The guest use the raw disk volume or LVM2 volume, such as following
the config file using raw disk volume created by RHEL 6.1 Virt Manager.
<disk type='block' device='disk'>
<driver name='qemu' type='raw' cache='none'
io='native'/>
<source dev='/dev/sdc'/>
<target dev='vda' bus='virtio'/>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x05'
function='0x0'/>
</disk>
Here, I'm assuming /dev/sdc is the LVM volume.
2. Do the 'virsh snapshot-create abc --disk-only' with
libvirt-0.9.6 or
upper.
But here, since you failed to use the --xmlfile option with XML
describing the new qcow2 file name (or used snapshot-create-as with the
--diskspec option), you are asking libvirt to create the name for the
new qcow2 file, and to create it in the same directory as the existing
disk image. That is, you asked libvirt to create /dev/sdc.1317357844
(with that suffix being the timestamp of your attempt).
Oddly enough, the kernel will allow you to create regular files directly
in /dev/, although it seems rather dangerous; more particularly, when
you reboot, since /dev is mounted on devtmpfs, and regular files created
there previously will be lost:
# id
uid=0(root) gid=0(root)
groups=0(root),1(bin),2(daemon),3(sys),4(adm),6(disk),10(wheel)
context=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
# mount | grep '/dev '
devtmpfs on /dev type devtmpfs
(rw,nosuid,relatime,seclabel,size=1895732k,nr_inodes=473933,mode=755)
# echo hi > /dev/sdc.123
# cat /dev/sdc.123
hi
So _this_ is the point where libvirt could be smarter, and say that if
the open("/dev/sdc.1317357844", O_CREAT, 0600) would be creating a
regular file on devtmpfs or any other unusual file system, that the
operation should be rejected. Not because the backing file is a block
device, but because the _new_ qcow2 file is not in a persistent location.
Maybe the kernel should be patched to prohibit open(O_CREAT) from
creating regular files inside devtmpfs?
3. The guest runs well till it is shutdowned.
4. But the guest does not boot with following error.
virtsh # start abc
error: Failed to start domain abc
error: internal error Process exited while reading console log output: char
device redirected to /dev/pts/7
qemu: could not open disk image /dev/sdc.1317357844: Invalid
argument.
Yep, this is evidence that the qcow2 file that was previously created as
a regular file inside devtmpfs is now lost, so you've lost all changes
that were made in the running domain after the snapshot in step 1 but
prior to the shutdown in step 2.
5. Then the config file has been replaced by libvirtd. Especially
difference is
<disk type='block' device='disk'>
<driver name='qemu' type='qcow2' cache='none'/>
<source dev='/dev/sdc.1317357844'/>
<target dev='vda' bus='virtio'/>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x05'
function='0x0'/>
</disk>
That's correct - libvirt is _supposed_ to modify the domain XML with the
name of the qcow2 file it creates. What went wrong is that since you
failed to tell libvirt _what_ file name to use, it invented its own file
name, and happened to invent a name that was on temporary storage and
thus got lost in your situation.
And 'snapshot-create ... --disk-only' works well for image format file,
e.g. qcow2, raw and etc.
It works for any backing file format. Where it needs help is knowing
what file name to use - if your backing file is a block device instead
of a regular file, then it is up to you to help libvirt out by giving it
a sensible file name for the new qcow2 image, either with the --xmlfile
option of snapshot-create, or the --diskspec option of snapshot-create-as.
So, I think that the restriction is needed for the taking snapshot
disk-only for the disk volume.
This is where I disagree with your conclusion. Taking a disk-only
snapshot of block devices works just fine, provided that you tell
libvirt what file name to use for the qcow2 file it creates.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
6:26 p.m.
On 11/15/2011 04:18 PM, Eric Blake wrote:
> 2. Do the 'virsh snapshot-create abc --disk-only' with
libvirt-0.9.6 or
> upper.
But here, since you failed to use the --xmlfile option with XML
describing the new qcow2 file name (or used snapshot-create-as with the
--diskspec option), you are asking libvirt to create the name for the
new qcow2 file, and to create it in the same directory as the existing
disk image. That is, you asked libvirt to create /dev/sdc.1317357844
(with that suffix being the timestamp of your attempt).
>
> And 'snapshot-create ... --disk-only' works well for image format file,
> e.g. qcow2, raw and etc.
It works for any backing file format. Where it needs help is knowing
what file name to use - if your backing file is a block device instead
of a regular file, then it is up to you to help libvirt out by giving it
a sensible file name for the new qcow2 image, either with the --xmlfile
option of snapshot-create, or the --diskspec option of snapshot-create-as.
>
> So, I think that the restriction is needed for the taking snapshot
> disk-only for the disk volume.
This is where I disagree with your conclusion. Taking a disk-only
snapshot of block devices works just fine, provided that you tell
libvirt what file name to use for the qcow2 file it creates.
After working on this some more, I think that identifying problematic
file systems, like devtmpfs, is too tricky to be portable. But I think
we can meet halfway - right now, the libvirt code blindly generates a
filename if one was not provided, regardless of the file type of the
backing file it will be wrapping. But maybe it would be sufficient to
make the command error out if no qcow2 filename is provided and the
backing file is not a regular file. As in this patch:
From 099080c24eb1ed78c836e5823d351ab2980dc523 Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake(a)redhat.com>
Date: Tue, 15 Nov 2011 17:19:20 -0700
Subject: [PATCH] snapshot: refuse to generate names for non-regular backing
files
For whatever reason, the kernel allows you to create a regular
file named /dev/sdc.12345; although this file will disappear the
next time devtmpfs is remounted. If you let libvirt generate
the name of the external snapshot for a disk image originally
using the block device /dev/sdc, then the domain will be rendered
unbootable once the qcow2 file is lost on the next devtmpfs
remount. In this case, the user should have used 'virsh
snapshot-create --xmlfile' or 'virsh snapshot-create-as --diskspec'
to specify the name for the qcow2 file in a sane location, rather
than relying on libvirt generating a name that is most likely to
be wrong. We can help avoid naive mistakes by enforcing that
the user provide the external name for any backing file that is
not a regular file.
* src/conf/domain_conf.c (virDomainSnapshotAlignDisks): Only
generate names if backing file exists as regular file.
Reported by MATSUDA Daiki.
---
src/conf/domain_conf.c | 15 +++++++++++++--
1 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6b78d97..1cef2be 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12203,7 +12203,8 @@
virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def,
qsort(&def->disks[0], def->ndisks, sizeof(def->disks[0]), disksorter);
- /* Generate any default external file names. */
+ /* Generate any default external file names, but only if the
+ * backing file is a regular file. */
for (i = 0; i < def->ndisks; i++) {
virDomainSnapshotDiskDefPtr disk = &def->disks[i];
@@ -12211,14 +12212,24 @@
virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def,
!disk->file) {
const char *original = def->dom->disks[i]->src;
const char *tmp;
+ struct stat sb;
if (!original) {
virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("cannot generate external backup
name "
+ _("cannot generate external
snapshot name "
"for disk '%s' without
source"),
disk->name);
goto cleanup;
}
+ if (stat(original, &sb) < 0 || !S_ISREG(sb.st_mode)) {
+ virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("source for disk '%s' is not a
regular "
+ "file; refusing to generate
external "
+ "snapshot name"),
+ disk->name);
+ goto cleanup;
+ }
+
tmp = strrchr(original, '.');
if (!tmp || strchr(tmp, '/')) {
ignore_value(virAsprintf(&disk->file, "%s.%s",
--
1.7.7.1
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
9:20 p.m.
On Tue, Nov 15, 2011 at 05:26:59PM -0700, Eric Blake wrote:
On 11/15/2011 04:18 PM, Eric Blake wrote:
>> 2. Do the 'virsh snapshot-create abc --disk-only' with libvirt-0.9.6 or
>> upper.
>
> But here, since you failed to use the --xmlfile option with XML
> describing the new qcow2 file name (or used snapshot-create-as with the
> --diskspec option), you are asking libvirt to create the name for the
> new qcow2 file, and to create it in the same directory as the existing
> disk image. That is, you asked libvirt to create /dev/sdc.1317357844
> (with that suffix being the timestamp of your attempt).
>
>>
>> And 'snapshot-create ... --disk-only' works well for image format file,
>> e.g. qcow2, raw and etc.
>
> It works for any backing file format. Where it needs help is knowing
> what file name to use - if your backing file is a block device instead
> of a regular file, then it is up to you to help libvirt out by giving it
> a sensible file name for the new qcow2 image, either with the --xmlfile
> option of snapshot-create, or the --diskspec option of snapshot-create-as.
>
>>
>> So, I think that the restriction is needed for the taking snapshot
>> disk-only for the disk volume.
>
> This is where I disagree with your conclusion. Taking a disk-only
> snapshot of block devices works just fine, provided that you tell
> libvirt what file name to use for the qcow2 file it creates.
After working on this some more, I think that identifying problematic
file systems, like devtmpfs, is too tricky to be portable. But I think
we can meet halfway - right now, the libvirt code blindly generates a
filename if one was not provided, regardless of the file type of the
backing file it will be wrapping. But maybe it would be sufficient to
make the command error out if no qcow2 filename is provided and the
backing file is not a regular file. As in this patch:
Ok, now I understand the situation; thanks to everyone for the
explanations. I'm somewhat uncomfortable using file type as a proxy
for "troublesome filesystem" since although they are linked in this
case, I'm not sure they're linked in all cases. Maybe I'm wrong about
that. If there is no portable way to determine whether a particular
filesystem is going to be a problem, I would just clearly document the
limitations of letting libvirt choose the filename and the importance
of not using that functionality on filesystems that cannot hold a
useful snapshot.
Dave
From 099080c24eb1ed78c836e5823d351ab2980dc523 Mon Sep 17 00:00:00
2001
From: Eric Blake <eblake(a)redhat.com>
Date: Tue, 15 Nov 2011 17:19:20 -0700
Subject: [PATCH] snapshot: refuse to generate names for non-regular backing
files
For whatever reason, the kernel allows you to create a regular
file named /dev/sdc.12345; although this file will disappear the
next time devtmpfs is remounted. If you let libvirt generate
the name of the external snapshot for a disk image originally
using the block device /dev/sdc, then the domain will be rendered
unbootable once the qcow2 file is lost on the next devtmpfs
remount. In this case, the user should have used 'virsh
snapshot-create --xmlfile' or 'virsh snapshot-create-as --diskspec'
to specify the name for the qcow2 file in a sane location, rather
than relying on libvirt generating a name that is most likely to
be wrong. We can help avoid naive mistakes by enforcing that
the user provide the external name for any backing file that is
not a regular file.
* src/conf/domain_conf.c (virDomainSnapshotAlignDisks): Only
generate names if backing file exists as regular file.
Reported by MATSUDA Daiki.
---
src/conf/domain_conf.c | 15 +++++++++++++--
1 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6b78d97..1cef2be 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12203,7 +12203,8 @@
virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def,
qsort(&def->disks[0], def->ndisks, sizeof(def->disks[0]), disksorter);
- /* Generate any default external file names. */
+ /* Generate any default external file names, but only if the
+ * backing file is a regular file. */
for (i = 0; i < def->ndisks; i++) {
virDomainSnapshotDiskDefPtr disk = &def->disks[i];
@@ -12211,14 +12212,24 @@
virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def,
!disk->file) {
const char *original = def->dom->disks[i]->src;
const char *tmp;
+ struct stat sb;
if (!original) {
virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("cannot generate external backup
name "
+ _("cannot generate external
snapshot name "
"for disk '%s' without
source"),
disk->name);
goto cleanup;
}
+ if (stat(original, &sb) < 0 || !S_ISREG(sb.st_mode)) {
+ virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("source for disk '%s' is not a
regular "
+ "file; refusing to generate
external "
+ "snapshot name"),
+ disk->name);
+ goto cleanup;
+ }
+
tmp = strrchr(original, '.');
if (!tmp || strchr(tmp, '/')) {
ignore_value(virAsprintf(&disk->file, "%s.%s",
--
1.7.7.1
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
9:45 a.m.
On 11/15/2011 08:20 PM, Dave Allan wrote:
> After working on this some more, I think that identifying
problematic
> file systems, like devtmpfs, is too tricky to be portable. But I think
> we can meet halfway - right now, the libvirt code blindly generates a
> filename if one was not provided, regardless of the file type of the
> backing file it will be wrapping. But maybe it would be sufficient to
> make the command error out if no qcow2 filename is provided and the
> backing file is not a regular file. As in this patch:
Ok, now I understand the situation; thanks to everyone for the
explanations. I'm somewhat uncomfortable using file type as a proxy
for "troublesome filesystem" since although they are linked in this
case, I'm not sure they're linked in all cases. Maybe I'm wrong about
that. If there is no portable way to determine whether a particular
filesystem is going to be a problem, I would just clearly document the
limitations of letting libvirt choose the filename and the importance
of not using that functionality on filesystems that cannot hold a
useful snapshot.
My patch would not be preventing snapshots of block devices, but merely
requiring that the user supply the qcow2 filename that will wrap the
block device. The problem with just documenting the issue is that
someone will fail to read the documentation, perform a default snapshot
that creates a problematic qcow2 file, then be upset that their domain
fails to boot and that they lost all the changes made since the
snapshot. I'm still in favor of this patch if anyone else thinks it is
worthwhile.
> Subject: [PATCH] snapshot: refuse to generate names for
non-regular backing
> files
>
> For whatever reason, the kernel allows you to create a regular
> file named /dev/sdc.12345; although this file will disappear the
> next time devtmpfs is remounted. If you let libvirt generate
> the name of the external snapshot for a disk image originally
> using the block device /dev/sdc, then the domain will be rendered
> unbootable once the qcow2 file is lost on the next devtmpfs
> remount. In this case, the user should have used 'virsh
> snapshot-create --xmlfile' or 'virsh snapshot-create-as --diskspec'
> to specify the name for the qcow2 file in a sane location, rather
> than relying on libvirt generating a name that is most likely to
> be wrong. We can help avoid naive mistakes by enforcing that
> the user provide the external name for any backing file that is
> not a regular file.
>
> * src/conf/domain_conf.c (virDomainSnapshotAlignDisks): Only
> generate names if backing file exists as regular file.
> Reported by MATSUDA Daiki.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
3:11 a.m.
Checking the source of the disk will exclude the case when the source
disk is block device while the snapshot target is given manually by the
users with "snapshot-create --xmlfile" or "virsh snapshot-create-as
--diskspec". Why not check the snapshot directory where it will be
created?
On 2011-11-16 23:45, Eric Blake wrote:
On 11/15/2011 08:20 PM, Dave Allan wrote:
>> After working on this some more, I think that identifying problematic
>> file systems, like devtmpfs, is too tricky to be portable. But I think
>> we can meet halfway - right now, the libvirt code blindly generates a
>> filename if one was not provided, regardless of the file type of the
>> backing file it will be wrapping. But maybe it would be sufficient to
>> make the command error out if no qcow2 filename is provided and the
>> backing file is not a regular file. As in this patch:
> Ok, now I understand the situation; thanks to everyone for the
> explanations. I'm somewhat uncomfortable using file type as a proxy
> for "troublesome filesystem" since although they are linked in this
> case, I'm not sure they're linked in all cases. Maybe I'm wrong about
> that. If there is no portable way to determine whether a particular
> filesystem is going to be a problem, I would just clearly document the
> limitations of letting libvirt choose the filename and the importance
> of not using that functionality on filesystems that cannot hold a
> useful snapshot.
My patch would not be preventing snapshots of block devices, but merely
requiring that the user supply the qcow2 filename that will wrap the
block device. The problem with just documenting the issue is that
someone will fail to read the documentation, perform a default snapshot
that creates a problematic qcow2 file, then be upset that their domain
fails to boot and that they lost all the changes made since the
snapshot. I'm still in favor of this patch if anyone else thinks it is
worthwhile.
>> Subject: [PATCH] snapshot: refuse to generate names for non-regular backing
>> files
>>
>> For whatever reason, the kernel allows you to create a regular
>> file named /dev/sdc.12345; although this file will disappear the
>> next time devtmpfs is remounted. If you let libvirt generate
>> the name of the external snapshot for a disk image originally
>> using the block device /dev/sdc, then the domain will be rendered
>> unbootable once the qcow2 file is lost on the next devtmpfs
>> remount. In this case, the user should have used 'virsh
>> snapshot-create --xmlfile' or 'virsh snapshot-create-as --diskspec'
>> to specify the name for the qcow2 file in a sane location, rather
>> than relying on libvirt generating a name that is most likely to
>> be wrong. We can help avoid naive mistakes by enforcing that
>> the user provide the external name for any backing file that is
>> not a regular file.
>>
>> * src/conf/domain_conf.c (virDomainSnapshotAlignDisks): Only
>> generate names if backing file exists as regular file.
>> Reported by MATSUDA Daiki.
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
7:45 a.m.
On 11/17/2011 02:11 AM, shu ming wrote:
Checking the source of the disk will exclude the case when the
source
disk is block device while the snapshot target is given manually by the
users with "snapshot-create --xmlfile" or "virsh snapshot-create-as
--diskspec". Why not check the snapshot directory where it will be
created?
Because it is assumed that if you are bothering to take the time to pass
--xmlfile or --diskspec, then you are also aware of the issues of that
file name and can deal with the consequences of naming a bogus location
for the resulting qcow2 file. The patch below is only a crutch to make
the default behavior, with no explicit name, less likely to cause
confusion; while still leaving the user with the full power of the tool
when they go with explicit file names.
>>>
>>> * src/conf/domain_conf.c (virDomainSnapshotAlignDisks): Only
>>> generate names if backing file exists as regular file.
>>> Reported by MATSUDA Daiki.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
9:04 a.m.
On Wed, Nov 16, 2011 at 08:45:34AM -0700, Eric Blake wrote:
On 11/15/2011 08:20 PM, Dave Allan wrote:
>> After working on this some more, I think that identifying problematic
>> file systems, like devtmpfs, is too tricky to be portable. But I think
>> we can meet halfway - right now, the libvirt code blindly generates a
>> filename if one was not provided, regardless of the file type of the
>> backing file it will be wrapping. But maybe it would be sufficient to
>> make the command error out if no qcow2 filename is provided and the
>> backing file is not a regular file. As in this patch:
>
> Ok, now I understand the situation; thanks to everyone for the
> explanations. I'm somewhat uncomfortable using file type as a proxy
> for "troublesome filesystem" since although they are linked in this
> case, I'm not sure they're linked in all cases. Maybe I'm wrong about
> that. If there is no portable way to determine whether a particular
> filesystem is going to be a problem, I would just clearly document the
> limitations of letting libvirt choose the filename and the importance
> of not using that functionality on filesystems that cannot hold a
> useful snapshot.
My patch would not be preventing snapshots of block devices, but merely
requiring that the user supply the qcow2 filename that will wrap the
block device. The problem with just documenting the issue is that
someone will fail to read the documentation, perform a default snapshot
that creates a problematic qcow2 file, then be upset that their domain
fails to boot and that they lost all the changes made since the
snapshot. I'm still in favor of this patch if anyone else thinks it is
worthwhile.
After an offline conversation with Eric about why file type is a good
proxy for a filesystem that is unsuitable for a snapshot, I'm in
agreement with this patch. The argument boils down to
1) block devices are really the only non-regular files that are useful
for backing guest block devices
2) the vast majority of block devices on modern systems live in
filesystems that are unsuitable for snapshots (e.g., devfs)
3) if a user has a use case that requires mknod'ing a block device in
a filesystem capable of storing snapshots, they can override the
libvirt check by supplying a filename which libvirt will honor.
so, ACK to the design.
Dave
>> Subject: [PATCH] snapshot: refuse to generate names for
>> non-regular backing files
>>
>> For whatever reason, the kernel allows you to create a regular
>> file named /dev/sdc.12345; although this file will disappear the
>> next time devtmpfs is remounted. If you let libvirt generate
>> the name of the external snapshot for a disk image originally
>> using the block device /dev/sdc, then the domain will be rendered
>> unbootable once the qcow2 file is lost on the next devtmpfs
>> remount. In this case, the user should have used 'virsh
>> snapshot-create --xmlfile' or 'virsh snapshot-create-as --diskspec'
>> to specify the name for the qcow2 file in a sane location, rather
>> than relying on libvirt generating a name that is most likely to
>> be wrong. We can help avoid naive mistakes by enforcing that
>> the user provide the external name for any backing file that is
>> not a regular file.
>>
>> * src/conf/domain_conf.c (virDomainSnapshotAlignDisks): Only
>> generate names if backing file exists as regular file.
>> Reported by MATSUDA Daiki.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
12:51 p.m.
On 11/17/2011 08:04 AM, Dave Allan wrote:
After an offline conversation with Eric about why file type is a
good
proxy for a filesystem that is unsuitable for a snapshot, I'm in
agreement with this patch. The argument boils down to
1) block devices are really the only non-regular files that are useful
for backing guest block devices
The other non-regular files being directories, chardevs, symlinks,
fifos, and sockets.
2) the vast majority of block devices on modern systems live in
filesystems that are unsuitable for snapshots (e.g., devfs)
3) if a user has a use case that requires mknod'ing a block device in
a filesystem capable of storing snapshots, they can override the
libvirt check by supplying a filename which libvirt will honor.
so, ACK to the design.
I've gone ahead and pushed this patch.
>>> * src/conf/domain_conf.c (virDomainSnapshotAlignDisks):
Only
>>> generate names if backing file exists as regular file.
>>> Reported by MATSUDA Daiki.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
7:51 p.m.
Sorry, I confirmed the snapshot file is not wrong.
I moved the snapshot file created under /dev as /dev/VG1/abc.xxxxxx to
other place and fixed the config file re-writed by libvirt to use moved
snapshot file abc.xxxxxx. So, the domain starts with no problem.
Regards
MATSUDA Daiki
On 11/15/2011 08:20 PM, Dave Allan wrote:
>> After working on this some more, I think that identifying problematic
>> file systems, like devtmpfs, is too tricky to be portable. But I think
>> we can meet halfway - right now, the libvirt code blindly generates a
>> filename if one was not provided, regardless of the file type of the
>> backing file it will be wrapping. But maybe it would be sufficient to
>> make the command error out if no qcow2 filename is provided and the
>> backing file is not a regular file. As in this patch:
>
> Ok, now I understand the situation; thanks to everyone for the
> explanations. I'm somewhat uncomfortable using file type as a proxy
> for "troublesome filesystem" since although they are linked in this
> case, I'm not sure they're linked in all cases. Maybe I'm wrong about
> that. If there is no portable way to determine whether a particular
> filesystem is going to be a problem, I would just clearly document the
> limitations of letting libvirt choose the filename and the importance
> of not using that functionality on filesystems that cannot hold a
> useful snapshot.
My patch would not be preventing snapshots of block devices, but merely
requiring that the user supply the qcow2 filename that will wrap the
block device. The problem with just documenting the issue is that
someone will fail to read the documentation, perform a default snapshot
that creates a problematic qcow2 file, then be upset that their domain
fails to boot and that they lost all the changes made since the
snapshot. I'm still in favor of this patch if anyone else thinks it is
worthwhile.
>> Subject: [PATCH] snapshot: refuse to generate names for non-regular backing
>> files
>>
>> For whatever reason, the kernel allows you to create a regular
>> file named /dev/sdc.12345; although this file will disappear the
>> next time devtmpfs is remounted. If you let libvirt generate
>> the name of the external snapshot for a disk image originally
>> using the block device /dev/sdc, then the domain will be rendered
>> unbootable once the qcow2 file is lost on the next devtmpfs
>> remount. In this case, the user should have used 'virsh
>> snapshot-create --xmlfile' or 'virsh snapshot-create-as --diskspec'
>> to specify the name for the qcow2 file in a sane location, rather
>> than relying on libvirt generating a name that is most likely to
>> be wrong. We can help avoid naive mistakes by enforcing that
>> the user provide the external name for any backing file that is
>> not a regular file.
>>
>> * src/conf/domain_conf.c (virDomainSnapshotAlignDisks): Only
>> generate names if backing file exists as regular file.
>> Reported by MATSUDA Daiki.
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
8:15 a.m.
On 2011-11-21 9:51, MATSUDA, Daiki wrote:
Sorry, I confirmed the snapshot file is not wrong.
I moved the snapshot file created under /dev as /dev/VG1/abc.xxxxxx to
other place and fixed the config file re-writed by libvirt to use
moved snapshot file abc.xxxxxx. So, the domain starts with no problem.
Regards
MATSUDA Daiki
So the abc.xxxxxx is a plain image file instead of a LVM block device.
I was wondering why the plain file was allowed to be created on a devfs
file system.
> On 11/15/2011 08:20 PM, Dave Allan wrote:
>>> After working on this some more, I think that identifying problematic
>>> file systems, like devtmpfs, is too tricky to be portable. But I
>>> think
>>> we can meet halfway - right now, the libvirt code blindly generates a
>>> filename if one was not provided, regardless of the file type of the
>>> backing file it will be wrapping. But maybe it would be sufficient to
>>> make the command error out if no qcow2 filename is provided and the
>>> backing file is not a regular file. As in this patch:
>>
>> Ok, now I understand the situation; thanks to everyone for the
>> explanations. I'm somewhat uncomfortable using file type as a proxy
>> for "troublesome filesystem" since although they are linked in this
>> case, I'm not sure they're linked in all cases. Maybe I'm wrong
about
>> that. If there is no portable way to determine whether a particular
>> filesystem is going to be a problem, I would just clearly document the
>> limitations of letting libvirt choose the filename and the importance
>> of not using that functionality on filesystems that cannot hold a
>> useful snapshot.
>
> My patch would not be preventing snapshots of block devices, but merely
> requiring that the user supply the qcow2 filename that will wrap the
> block device. The problem with just documenting the issue is that
> someone will fail to read the documentation, perform a default snapshot
> that creates a problematic qcow2 file, then be upset that their domain
> fails to boot and that they lost all the changes made since the
> snapshot. I'm still in favor of this patch if anyone else thinks it is
> worthwhile.
>
>>> Subject: [PATCH] snapshot: refuse to generate names for non-regular
>>> backing
>>> files
>>>
>>> For whatever reason, the kernel allows you to create a regular
>>> file named /dev/sdc.12345; although this file will disappear the
>>> next time devtmpfs is remounted. If you let libvirt generate
>>> the name of the external snapshot for a disk image originally
>>> using the block device /dev/sdc, then the domain will be rendered
>>> unbootable once the qcow2 file is lost on the next devtmpfs
>>> remount. In this case, the user should have used 'virsh
>>> snapshot-create --xmlfile' or 'virsh snapshot-create-as
--diskspec'
>>> to specify the name for the qcow2 file in a sane location, rather
>>> than relying on libvirt generating a name that is most likely to
>>> be wrong. We can help avoid naive mistakes by enforcing that
>>> the user provide the external name for any backing file that is
>>> not a regular file.
>>>
>>> * src/conf/domain_conf.c (virDomainSnapshotAlignDisks): Only
>>> generate names if backing file exists as regular file.
>>> Reported by MATSUDA Daiki.
>
>
>
> --
> libvir-list mailing list
> libvir-list(a)redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
4563
days inactive
4615
days old
20 comments
5 participants
participants (5)
-
Daniel P. Berrange -
Dave Allan -
Eric Blake -
MATSUDA, Daiki -
shu ming