[libvirt] bug: try to take disk snapshot for LVM2 Volume
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
(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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
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
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
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 :|
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@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
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
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@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
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@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
(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@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
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@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@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
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@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@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
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:
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
On 11/15/2011 08:20 PM, Dave Allan wrote: 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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
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@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
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@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (5)
-
Daniel P. Berrange -
Dave Allan -
Eric Blake -
MATSUDA, Daiki -
shu ming