Re: [libvirt] bug: try to take disk snapshot for LVM2 Volume

2011-11-21 Thread shu ming

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.xx to 
other place and fixed the config file re-writed by libvirt to use 
moved snapshot file abc.xx. So, the domain starts with no problem.


Regards
MATSUDA Daiki

So the abc.xx 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




--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] bug: try to take disk snapshot for LVM2 Volume

2011-11-20 Thread MATSUDA, Daiki

Sorry, I confirmed the snapshot file is not wrong.

I moved the snapshot file created under /dev as /dev/VG1/abc.xx to 
other place and fixed the config file re-writed by libvirt to use moved 
snapshot file abc.xx. 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


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] bug: try to take disk snapshot for LVM2 Volume

2011-11-17 Thread shu ming
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@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

Re: [libvirt] bug: try to take disk snapshot for LVM2 Volume

2011-11-17 Thread Eric Blake
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   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] bug: try to take disk snapshot for LVM2 Volume

2011-11-17 Thread Dave Allan
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   ebl...@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


Re: [libvirt] bug: try to take disk snapshot for LVM2 Volume

2011-11-17 Thread Eric Blake
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   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] bug: try to take disk snapshot for LVM2 Volume

2011-11-16 Thread Eric Blake
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   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] bug: try to take disk snapshot for LVM2 Volume

2011-11-15 Thread Dave Allan
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   ebl...@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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] bug: try to take disk snapshot for LVM2 Volume

2011-11-15 Thread MATSUDA, Daiki

(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'
  nameabc/name
  uuid791ede5a-e617-ca6b-125d-4e9704a2ddc2/uuid
  memory524288/memory
  currentMemory524288/currentMemory
  vcpu1/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_poweroffdestroy/on_poweroff
  on_rebootrestart/on_reboot
  on_crashrestart/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='0x' 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='0x' 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='0x' 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='0x' bus='0x00' slot='0x04' 
function='0x0'/

/sound
video
  model type='cirrus' vram='9216' heads='1'/
  address type='pci' domain='0x' bus='0x00' slot='0x02' 
function='0x0'/

/video
memballoon model='virtio'
  address type='pci' domain='0x' 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='0x' 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 

Re: [libvirt] bug: try to take disk snapshot for LVM2 Volume

2011-11-15 Thread Eric Blake
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='0x' 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='0x' 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   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] bug: try to take disk snapshot for LVM2 Volume

2011-11-15 Thread Eric Blake
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 ebl...@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   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: 

Re: [libvirt] bug: try to take disk snapshot for LVM2 Volume

2011-11-15 Thread Dave Allan
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 ebl...@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 

Re: [libvirt] bug: try to take disk snapshot for LVM2 Volume

2011-11-14 Thread Eric Blake
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   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] bug: try to take disk snapshot for LVM2 Volume

2011-11-13 Thread MATSUDA, Daiki
 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
--- libvirt-0.9.7.orig/src/qemu/qemu_driver.c   2011-11-03 23:48:23.0 
+0900
+++ libvirt-0.9.7/src/qemu/qemu_driver.c2011-11-14 12:56:51.870453885 
+0900
@@ -9025,6 +9025,7 @@ qemuDomainSnapshotCreateSingleDiskActive
 char *origsrc = NULL;
 char *origdriver = NULL;
 bool need_unlink = false;
+struct stat sb;
 
 if (snap-snapshot != VIR_DOMAIN_DISK_SNAPSHOT_EXTERNAL) {
 qemuReportError(VIR_ERR_INTERNAL_ERROR, %s,
@@ -9032,6 +9033,17 @@ qemuDomainSnapshotCreateSingleDiskActive
 return -1;
 }
 
+if (stat(disk-src, sb)  0) {
+virReportSystemError(errno,
+ _(unable to stat for disk %s),
+ disk-src);
+}
+if (S_ISBLK(sb.st_mode)) {
+qemuReportError(VIR_ERR_INTERNAL_ERROR,
+%s, _(src image is Block Device));
+return -1;
+}
+
 if (virAsprintf(device, drive-%s, disk-info.alias)  0 ||
 !(source = strdup(snap-file)) ||
 (STRNEQ_NULLABLE(disk-driverType, qcow2) 
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] bug: try to take disk snapshot for LVM2 Volume

2011-11-07 Thread Daniel P. Berrange
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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] bug: try to take disk snapshot for LVM2 Volume

2011-11-07 Thread Eric Blake
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   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] bug: try to take disk snapshot for LVM2 Volume

2011-11-06 Thread 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

diff -uNrp libvirt-0.9.7.orig/src/qemu/qemu_driver.c 
libvirt-0.9.7/src/qemu/qemu_driver.c
--- libvirt-0.9.7.orig/src/qemu/qemu_driver.c   2011-10-31 12:46:04.0 
+0900
+++ libvirt-0.9.7/src/qemu/qemu_driver.c2011-11-07 13:03:07.162639142 
+0900
@@ -8976,6 +8976,25 @@ cleanup:
 return ret;
 }
 
+static int ATTRIBUTE_NONNULL(1)
+qemuDomainSnapshotCheckSrcQcow2(const char *src)
+{
+const char *qemuimgarg[] = {NULL, check, -f, qcow2, NULL, NULL};
+int ret = -1;
+
+qemuimgarg[0] = virFindFileInPath(qemu-img);
+if (!qemuimgarg[0]) {
+qemuReportError(VIR_ERR_INTERNAL_ERROR,
+%s, _(unable to find qemu-img));
+return ret;
+}
+qemuimgarg[4] = src;
+
+ret = virRun(qemuimgarg, NULL);
+VIR_FREE(qemuimgarg[0]);
+return ret;
+}
+
 /* The domain is expected to hold monitor lock.  */
 static int
 qemuDomainSnapshotCreateSingleDiskActive(struct qemud_driver *driver,
@@ -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;
+}
+
 if (virAsprintf(device, drive-%s, disk-info.alias)  0 ||
 !(source = strdup(snap-file)) ||
 (STRNEQ_NULLABLE(disk-driverType, qcow2) 
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] bug: try to take disk snapshot for LVM2 Volume

2011-10-04 Thread MATSUDA, Daiki
 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 

Re: [libvirt] bug: try to take disk snapshot for LVM2 Volume

2011-09-30 Thread 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
 
 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] bug: try to take disk snapshot for LVM2 Volume

2011-09-30 Thread Eric Blake
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   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list