On 07/27/2011 04:43 AM, Daniel P. Berrange wrote:
On Wed, Jul 27, 2011 at 10:36:25AM +0200, Kevin Wolf wrote:
Am 27.07.2011 10:22, schrieb Daniel P. Berrange:
On Wed, Jul 27, 2011 at 10:11:06AM +0200, Kevin Wolf wrote:
Am 26.07.2011 18:57, schrieb Corey Bryant:
  diff --git a/block/cow.c b/block/cow.c
  index 4cf543c..e17f8e7 100644
  --- a/block/cow.c
  +++ b/block/cow.c
  @@ -82,6 +82,11 @@ static int cow_open(BlockDriverState *bs, int flags)
        pstrcpy(bs->backing_file, sizeof(bs->backing_file),
                cow_header.backing_file);

  +    if (bs->backing_file[0] != '\0'&&   bdrv_is_fd_protocol(bs)) {
  +        /* backing file currently not supported by fd: protocol */
  +        goto fail;
  +    }
I don't think these checks are strictly needed. Without the check you
can open the image itself using an fd, and the backing file using good
old raw-posix. We shouldn't decide for our users that this isn't useful.

In any case, it would have to move into block.c, you can't modify
independent drivers for this.


I understand the point on not modifying independent drivers.

But if the backing file resides on NFS, wouldn't the the proposed
SELinux changes prevent the open?

Probably. But what about cases where the backing file is local? Or even
a non-libvirt, non-SELinux use case?

Or are you talking about support where libvirt opens the backing file
and passes the fd to Qemu?  If so there was some discussion about future
support for this here:
http://lists.gnu.org/archive/html/qemu-devel/2011-06/msg01496.html

Not really, but implementing this will be a bit easier if you don't
forbid using backing files with fd.

In any case, for 'fd:' to be actually used by libvirt, we need to have
backing files supported. There are major users of libvirt who rely on
NFS and also use backing files, so an 'fd:' impl which can't deal with
the backing file problem isn't much use to libvirt.

I'm perfectly aware of that. But seriously, repeating it over and over
again isn't going to make it happen any sooner. It requires -blockdev
which we may or may not have by 1.0.

Yes, I understand we need also -blockdev for this. Other messages in this
mail thread have impied that this proposed patch on its own is useful for
libvirt's requirements, so I just wanted to remind people in general that
we can't use this patch on its own until we have something like -blockdev.

Regards,
Daniel

Kevin/Daniel, thanks a lot for your input.

In terms of the support that libvirt requires, I just want to make sure all bases are covered.

In order for this support to be useful to libvirt, the following are required (sorry if this is repetitive):

1) -blockdev (backing file support)
2) savevm (snapshot support)
3) snapshot_blkdev (snapshot support)
4) 'change' monitor command
5) -cdrom

and as far as I know, the status of the above is:

1) Someone is slated to work on this (not me)
2) I need to figure out how to "re-open" file without an open (me)
3) This will be covered by live snapshots feature (not me)
4) Should just work as is (me)
5) May also just work as is (me)

In other words, 1 and 3 will not be implemented by me (except perhaps some re-usable infrastructure). Could you confirm my understanding?

Regards,
Corey

Reply via email to