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

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