Re: [Qemu-devel] [PATCH v3] Add support for fd: protocol

[Corey Bryant]

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