Re: [libvirt] [Qemu-devel] [PATCH v3] Add support for fd: protocol
Am 11.08.2011 18:28, schrieb Corey Bryant:
On 07/26/2011 08:51 AM, Corey Bryant wrote:
+static int raw_open_fd(BlockDriverState *bs, const char *filename,
int flags)
+{
+BDRVRawState *s = bs-opaque;
+const char *fd_str;
+int fd;
+
+/* extract the file descriptor - fail if it's not fd: */
+if (!strstart(filename, fd:,fd_str)) {
+return -EINVAL;
+}
+
+if (!qemu_isdigit(fd_str[0])) {
+/* get fd from monitor */
+fd = qemu_get_fd(fd_str);
+if (fd == -1) {
+return -EBADF;
+}
+} else {
+char *endptr = NULL;
+
+fd = strtol(fd_str,endptr, 10);
+if (*endptr || (fd == 0 fd_str == endptr)) {
+return -EBADF;
+}
+}
+
+s-fd = fd;
+s-type = FTYPE_FILE;
+
+return raw_open_common(bs, filename, flags, 0);
+}
+
+static BlockDriver bdrv_file_fd = {
+.format_name = file,
+.protocol_name = fd,
+.instance_size = sizeof(BDRVRawState),
+.bdrv_probe = NULL, /* no probe for protocols */
+.bdrv_file_open = raw_open_fd,
+.bdrv_read = raw_read,
+.bdrv_write = raw_write,
+.bdrv_close = raw_close,
+.bdrv_flush = raw_flush,
+.bdrv_discard = raw_discard,
+
+.bdrv_aio_readv = raw_aio_readv,
+.bdrv_aio_writev = raw_aio_writev,
+.bdrv_aio_flush = raw_aio_flush,
+
+.bdrv_truncate = raw_truncate,
+.bdrv_getlength = raw_getlength,
+
+.create_options = raw_create_options,
+};
+
I'm a bit unsure of how to support CD-ROM with the fd: protocol.
I don't think use of bdrv_file_fd is an option. For example, while
raw_open_fd may work, there is no eject support.
Another approach is to have 2 BlockDriver structs that support the fd
protocol. For example, creating a new BlockDriver struct that mirrors
bdrv_host_cdrom. So you would have bdrv_host_cdrom_fd with a
corresponding cdrom_open_fd function. But that doesn't appear possible
as it appears that the protocol must be unique among all BlockDriver
structs.
Do we need to introduce a similar protocol (maybe cdfd) to support
passing of CD-ROM file descriptors?
I would try to avoid that. I think you can determine the kind of device
during fd_open and then implement an fd_eject etc. that checks this type
and calls the appropriate ioctl. Maybe the same should be done for
file/host_device/host_cdrom instead of having separate protocols.
Anyway, I don't think this is important to get a first version merged.
It will still take a while anyway until we have full support that
libvirt would want to use.
Kevin
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v3] Add support for fd: protocol
Am 26.07.2011 18:57, schrieb Corey Bryant:
Kevin, thanks for the input.
On 07/26/2011 11:18 AM, Kevin Wolf wrote:
Am 26.07.2011 14:51, schrieb Corey Bryant:
sVirt provides SELinux MAC isolation for Qemu guest processes and their
corresponding resources (image files). sVirt provides this support
by labeling guests and resources with security labels that are stored
in file system extended attributes. Some file systems, such as NFS, do
not support the extended attribute security namespace, which is needed
for image file isolation when using the sVirt SELinux security driver
in libvirt.
The proposed solution entails a combination of Qemu, libvirt, and
SELinux patches that work together to isolate multiple guests' images
when they're stored in the same NFS mount. This results in an
environment where sVirt isolation and NFS image file isolation can both
be provided.
This patch contains the Qemu code to support this solution. I would
like to solicit input from the libvirt community prior to starting
the libvirt patch.
Currently, Qemu opens an image file in addition to performing the
necessary read and write operations. The proposed solution will move
the open out of Qemu and into libvirt. Once libvirt opens an image
file for the guest, it will pass the file descriptor to Qemu via a
new fd: protocol.
If the image file resides in an NFS mount, the following SELinux policy
changes will provide image isolation:
- A new SELinux boolean is created (e.g. virt_read_write_nfs) to
allow Qemu (svirt_t) to only have SELinux read and write
permissions on nfs_t files
- Qemu (svirt_t) also gets SELinux use permissions on libvirt
(virtd_t) file descriptors
Following is a sample invocation of Qemu using the fd: protocol on
the command line:
qemu -drive file=fd:4,format=qcow2
The fd: protocol is also supported by the drive_add monitor command.
This requires that the specified file descriptor is passed to the
monitor alongside a prior getfd monitor command.
There are some additional features provided by certain image types
where Qemu reopens the image file. All of these scenarios will be
unsupported for the fd: protocol, at least for this patch:
- The -snapshot command line option
- The savevm monitor command
- The snapshot_blkdev monitor command
- Use of copy-on-write image files
- The -cdrom command line option
- The -drive command line option with media=cdrom
- The change monitor command
The thought is that this support can be added in the future, but is
not required for the initial fd: support.
This patch was tested with the following formats: raw, cow, qcow,
qcow2, qed, and vmdk, using the fd: protocol from the command line
and the monitor. Tests were also run to verify existing file name
support and qemu-img were not regressed. Non-valid file descriptors,
fd: without format, snapshot and backing files, and cdrom were also
tested.
v2:
- Add drive_add monitor command support
- Fence off unsupported features that re-open image file
v3:
- Fence off cdrom and change monitor command support
Signed-off-by: Corey Bryantcor...@linux.vnet.ibm.com
---
block.c | 16 ++
block.h |1 +
block/cow.c |5 +++
block/qcow.c |5 +++
block/qcow2.c |5 +++
block/qed.c |4 ++
block/raw-posix.c | 81
+++--
block/vmdk.c |5 +++
block_int.h |1 +
blockdev.c| 19
monitor.c |5 +++
monitor.h |1 +
qemu-options.hx |8 +++--
qemu-tool.c |5 +++
14 files changed, 149 insertions(+), 12 deletions(-)
diff --git a/block.c b/block.c
index 24a25d5..500db84 100644
--- a/block.c
+++ b/block.c
@@ -536,6 +536,10 @@ int bdrv_open(BlockDriverState *bs, const char
*filename, int flags,
char tmp_filename[PATH_MAX];
char backing_filename[PATH_MAX];
+if (bdrv_is_fd_protocol(bs)) {
+return -ENOTSUP;
+}
Hm, shouldn't that just work even with fd?
My thought was that the proposed SELinux changes would prevent the open
of the temporary backing file if the file corresponding to fd resides on
NFS. But perhaps the backing file is not opened on NFS?
Depends on how broken your SELinux restrictions are. ;-)
I would argue that allowing qemu to create temporary files is a
reasonable thing to do. Maybe libvirt comes to a different conclusion,
but that doesn't mean that every other management tool comes to the same.
Also, as Alex already said, you shouldn't think of your use case as the
only valid use case. What a fd driver implementation is about is just
working with an fd for images. If libvirts puts more restrictions on it,
that's fine, but don't force other users
Re: [libvirt] [Qemu-devel] [PATCH v3] Add support for fd: protocol
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.
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] [Qemu-devel] [PATCH v3] Add support for fd: protocol
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.
Kevin
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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
--
|: 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] [Qemu-devel] [PATCH v3] Add support for fd: protocol
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v3] Add support for fd: protocol
Am 27.07.2011 15:09, schrieb Corey Bryant: 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) savevm is harmless and doesn't need any special treatment. 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? Yes, at least not at this point. Kevin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v3] Add support for fd: protocol
On Tue, Jul 26, 2011 at 3:51 PM, Corey Bryant cor...@linux.vnet.ibm.com wrote:
sVirt provides SELinux MAC isolation for Qemu guest processes and their
corresponding resources (image files). sVirt provides this support
by labeling guests and resources with security labels that are stored
in file system extended attributes. Some file systems, such as NFS, do
not support the extended attribute security namespace, which is needed
for image file isolation when using the sVirt SELinux security driver
in libvirt.
The proposed solution entails a combination of Qemu, libvirt, and
SELinux patches that work together to isolate multiple guests' images
when they're stored in the same NFS mount. This results in an
environment where sVirt isolation and NFS image file isolation can both
be provided.
This patch contains the Qemu code to support this solution. I would
like to solicit input from the libvirt community prior to starting
the libvirt patch.
Currently, Qemu opens an image file in addition to performing the
necessary read and write operations. The proposed solution will move
the open out of Qemu and into libvirt. Once libvirt opens an image
file for the guest, it will pass the file descriptor to Qemu via a
new fd: protocol.
If the image file resides in an NFS mount, the following SELinux policy
changes will provide image isolation:
- A new SELinux boolean is created (e.g. virt_read_write_nfs) to
allow Qemu (svirt_t) to only have SELinux read and write
permissions on nfs_t files
- Qemu (svirt_t) also gets SELinux use permissions on libvirt
(virtd_t) file descriptors
Following is a sample invocation of Qemu using the fd: protocol on
the command line:
qemu -drive file=fd:4,format=qcow2
The fd: protocol is also supported by the drive_add monitor command.
This requires that the specified file descriptor is passed to the
monitor alongside a prior getfd monitor command.
There are some additional features provided by certain image types
where Qemu reopens the image file. All of these scenarios will be
unsupported for the fd: protocol, at least for this patch:
- The -snapshot command line option
- The savevm monitor command
- The snapshot_blkdev monitor command
- Use of copy-on-write image files
- The -cdrom command line option
- The -drive command line option with media=cdrom
- The change monitor command
In the earlier discussion, virtio-blk and iSCSI were identified as
interesting protocols to be used with file descriptors in the future.
This patch would bind fd: protocol only to raw file interface to be
used by qcow2 etc. higher levels. I guess with small changes the
protocol could be made selectable. Maybe it's just changing command
line to format=virtio-blk or format=iscsi for those uses, though I
fear there may be a layering violation.
Considering the privilege separation side, this patch seems to be
somewhat orthogonal to that (I don't expect any command line passing
and parsing to happen between QEMU processes) but the low level fd
handling seems useful modulo the comments made by others.
The thought is that this support can be added in the future, but is
not required for the initial fd: support.
This patch was tested with the following formats: raw, cow, qcow,
qcow2, qed, and vmdk, using the fd: protocol from the command line
and the monitor. Tests were also run to verify existing file name
support and qemu-img were not regressed. Non-valid file descriptors,
fd: without format, snapshot and backing files, and cdrom were also
tested.
v2:
- Add drive_add monitor command support
- Fence off unsupported features that re-open image file
v3:
- Fence off cdrom and change monitor command support
Signed-off-by: Corey Bryant cor...@linux.vnet.ibm.com
---
block.c | 16 ++
block.h | 1 +
block/cow.c | 5 +++
block/qcow.c | 5 +++
block/qcow2.c | 5 +++
block/qed.c | 4 ++
block/raw-posix.c | 81
+++--
block/vmdk.c | 5 +++
block_int.h | 1 +
blockdev.c | 19
monitor.c | 5 +++
monitor.h | 1 +
qemu-options.hx | 8 +++--
qemu-tool.c | 5 +++
14 files changed, 149 insertions(+), 12 deletions(-)
diff --git a/block.c b/block.c
index 24a25d5..500db84 100644
--- a/block.c
+++ b/block.c
@@ -536,6 +536,10 @@ int bdrv_open(BlockDriverState *bs, const char
*filename, int flags,
char tmp_filename[PATH_MAX];
char backing_filename[PATH_MAX];
+ if (bdrv_is_fd_protocol(bs)) {
+ return -ENOTSUP;
+ }
+
/* if snapshot, we create a temporary backing file and open it
instead of opening 'filename' directly */
@@ -585,6 +589,10 @@ int bdrv_open(BlockDriverState *bs, const char
*filename, int flags,
/* Find the right image format driver */
if (!drv) {
Re: [libvirt] [Qemu-devel] [PATCH v3] Add support for fd: protocol
I have to say I really hate it. We've been working hard on getting rid of special cases in the qemu block layer, and this sprinkles them all over. I'd recommend to fix your security model instead. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v3] Add support for fd: protocol
On 07/26/2011 07:02 AM, Christoph Hellwig wrote: I have to say I really hate it. We've been working hard on getting rid of special cases in the qemu block layer, and this sprinkles them all over. I'd recommend to fix your security model instead. What is it that you hate - the particular qemu implementation expressed by this patch (in which case, we're all ears for a better implementation), or the need for fd passing (which libvirt is strongly in favor of, and which there really is no other way to fix the security model to work well with file systems like NFS that lack per-file labels)? In my mind, increased security trumps poor aesthetics - if the patch really does make it possible to enhance security, then the special casing to support the patch is worth it. -- 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] [Qemu-devel] [PATCH v3] Add support for fd: protocol
Am 26.07.2011 15:02, schrieb Christoph Hellwig: I have to say I really hate it. We've been working hard on getting rid of special cases in the qemu block layer, and this sprinkles them all over. I'd recommend to fix your security model instead. I think the problem here is more with the implementation that with the intention. I agree that you just can't do this. A patch adding support for a fd: protocol should touch block/fd.c and nothing else. You can add some supporting patches that extend the generic block layer to support e.g. formats that can't reopen. However, if you touch the code of other block drivers, you're doing it wrong. Kevin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v3] Add support for fd: protocol
On 07/26/2011 10:05 AM, Kevin Wolf wrote: Am 26.07.2011 15:02, schrieb Christoph Hellwig: I have to say I really hate it. We've been working hard on getting rid of special cases in the qemu block layer, and this sprinkles them all over. I'd recommend to fix your security model instead. I think the problem here is more with the implementation that with the intention. I agree that you just can't do this. A patch adding support for a fd: protocol should touch block/fd.c and nothing else. You can add some supporting patches that extend the generic block layer to support e.g. formats that can't reopen. However, if you touch the code of other block drivers, you're doing it wrong. Kevin I'll look into this approach, but on the surface it seems like this could prevent a lot of code reuse in block/raw-posix.c. Regards, Corey -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v3] Add support for fd: protocol
Am 26.07.2011 16:46, schrieb Corey Bryant: On 07/26/2011 10:05 AM, Kevin Wolf wrote: Am 26.07.2011 15:02, schrieb Christoph Hellwig: I have to say I really hate it. We've been working hard on getting rid of special cases in the qemu block layer, and this sprinkles them all over. I'd recommend to fix your security model instead. I think the problem here is more with the implementation that with the intention. I agree that you just can't do this. A patch adding support for a fd: protocol should touch block/fd.c and nothing else. You can add some supporting patches that extend the generic block layer to support e.g. formats that can't reopen. However, if you touch the code of other block drivers, you're doing it wrong. I'll look into this approach, but on the surface it seems like this could prevent a lot of code reuse in block/raw-posix.c. What I meant here is more about modifying qcow2, VMDK etc. I still need to look into the details of what you share with raw-posix.c. Kevin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v3] Add support for fd: protocol
Kevin, thanks for the input.
On 07/26/2011 11:18 AM, Kevin Wolf wrote:
Am 26.07.2011 14:51, schrieb Corey Bryant:
sVirt provides SELinux MAC isolation for Qemu guest processes and their
corresponding resources (image files). sVirt provides this support
by labeling guests and resources with security labels that are stored
in file system extended attributes. Some file systems, such as NFS, do
not support the extended attribute security namespace, which is needed
for image file isolation when using the sVirt SELinux security driver
in libvirt.
The proposed solution entails a combination of Qemu, libvirt, and
SELinux patches that work together to isolate multiple guests' images
when they're stored in the same NFS mount. This results in an
environment where sVirt isolation and NFS image file isolation can both
be provided.
This patch contains the Qemu code to support this solution. I would
like to solicit input from the libvirt community prior to starting
the libvirt patch.
Currently, Qemu opens an image file in addition to performing the
necessary read and write operations. The proposed solution will move
the open out of Qemu and into libvirt. Once libvirt opens an image
file for the guest, it will pass the file descriptor to Qemu via a
new fd: protocol.
If the image file resides in an NFS mount, the following SELinux policy
changes will provide image isolation:
- A new SELinux boolean is created (e.g. virt_read_write_nfs) to
allow Qemu (svirt_t) to only have SELinux read and write
permissions on nfs_t files
- Qemu (svirt_t) also gets SELinux use permissions on libvirt
(virtd_t) file descriptors
Following is a sample invocation of Qemu using the fd: protocol on
the command line:
qemu -drive file=fd:4,format=qcow2
The fd: protocol is also supported by the drive_add monitor command.
This requires that the specified file descriptor is passed to the
monitor alongside a prior getfd monitor command.
There are some additional features provided by certain image types
where Qemu reopens the image file. All of these scenarios will be
unsupported for the fd: protocol, at least for this patch:
- The -snapshot command line option
- The savevm monitor command
- The snapshot_blkdev monitor command
- Use of copy-on-write image files
- The -cdrom command line option
- The -drive command line option with media=cdrom
- The change monitor command
The thought is that this support can be added in the future, but is
not required for the initial fd: support.
This patch was tested with the following formats: raw, cow, qcow,
qcow2, qed, and vmdk, using the fd: protocol from the command line
and the monitor. Tests were also run to verify existing file name
support and qemu-img were not regressed. Non-valid file descriptors,
fd: without format, snapshot and backing files, and cdrom were also
tested.
v2:
- Add drive_add monitor command support
- Fence off unsupported features that re-open image file
v3:
- Fence off cdrom and change monitor command support
Signed-off-by: Corey Bryantcor...@linux.vnet.ibm.com
---
block.c | 16 ++
block.h |1 +
block/cow.c |5 +++
block/qcow.c |5 +++
block/qcow2.c |5 +++
block/qed.c |4 ++
block/raw-posix.c | 81
+++--
block/vmdk.c |5 +++
block_int.h |1 +
blockdev.c| 19
monitor.c |5 +++
monitor.h |1 +
qemu-options.hx |8 +++--
qemu-tool.c |5 +++
14 files changed, 149 insertions(+), 12 deletions(-)
diff --git a/block.c b/block.c
index 24a25d5..500db84 100644
--- a/block.c
+++ b/block.c
@@ -536,6 +536,10 @@ int bdrv_open(BlockDriverState *bs, const char
*filename, int flags,
char tmp_filename[PATH_MAX];
char backing_filename[PATH_MAX];
+if (bdrv_is_fd_protocol(bs)) {
+return -ENOTSUP;
+}
Hm, shouldn't that just work even with fd?
My thought was that the proposed SELinux changes would prevent the open
of the temporary backing file if the file corresponding to fd resides on
NFS. But perhaps the backing file is not opened on NFS?
+
/* if snapshot, we create a temporary backing file and open it
instead of opening 'filename' directly */
@@ -585,6 +589,10 @@ int bdrv_open(BlockDriverState *bs, const char
*filename, int flags,
/* Find the right image format driver */
if (!drv) {
+/* format must be specified for fd: protocol */
+if (bdrv_is_fd_protocol(bs)) {
+return -ENOTSUP;
+}
This isn't really required to make fd work.
If format is not specified, the file needs to be opened and probed to
Re: [libvirt] [Qemu-devel] [PATCH v3] Add support for fd: protocol
On 26.07.2011, at 18:57, Corey Bryant wrote:
Kevin, thanks for the input.
On 07/26/2011 11:18 AM, Kevin Wolf wrote:
Am 26.07.2011 14:51, schrieb Corey Bryant:
sVirt provides SELinux MAC isolation for Qemu guest processes and their
corresponding resources (image files). sVirt provides this support
by labeling guests and resources with security labels that are stored
in file system extended attributes. Some file systems, such as NFS, do
not support the extended attribute security namespace, which is needed
for image file isolation when using the sVirt SELinux security driver
in libvirt.
The proposed solution entails a combination of Qemu, libvirt, and
SELinux patches that work together to isolate multiple guests' images
when they're stored in the same NFS mount. This results in an
environment where sVirt isolation and NFS image file isolation can both
be provided.
This patch contains the Qemu code to support this solution. I would
like to solicit input from the libvirt community prior to starting
the libvirt patch.
Currently, Qemu opens an image file in addition to performing the
necessary read and write operations. The proposed solution will move
the open out of Qemu and into libvirt. Once libvirt opens an image
file for the guest, it will pass the file descriptor to Qemu via a
new fd: protocol.
If the image file resides in an NFS mount, the following SELinux policy
changes will provide image isolation:
- A new SELinux boolean is created (e.g. virt_read_write_nfs) to
allow Qemu (svirt_t) to only have SELinux read and write
permissions on nfs_t files
- Qemu (svirt_t) also gets SELinux use permissions on libvirt
(virtd_t) file descriptors
Following is a sample invocation of Qemu using the fd: protocol on
the command line:
qemu -drive file=fd:4,format=qcow2
The fd: protocol is also supported by the drive_add monitor command.
This requires that the specified file descriptor is passed to the
monitor alongside a prior getfd monitor command.
There are some additional features provided by certain image types
where Qemu reopens the image file. All of these scenarios will be
unsupported for the fd: protocol, at least for this patch:
- The -snapshot command line option
- The savevm monitor command
- The snapshot_blkdev monitor command
- Use of copy-on-write image files
- The -cdrom command line option
- The -drive command line option with media=cdrom
- The change monitor command
The thought is that this support can be added in the future, but is
not required for the initial fd: support.
This patch was tested with the following formats: raw, cow, qcow,
qcow2, qed, and vmdk, using the fd: protocol from the command line
and the monitor. Tests were also run to verify existing file name
support and qemu-img were not regressed. Non-valid file descriptors,
fd: without format, snapshot and backing files, and cdrom were also
tested.
v2:
- Add drive_add monitor command support
- Fence off unsupported features that re-open image file
v3:
- Fence off cdrom and change monitor command support
Signed-off-by: Corey Bryantcor...@linux.vnet.ibm.com
---
block.c | 16 ++
block.h |1 +
block/cow.c |5 +++
block/qcow.c |5 +++
block/qcow2.c |5 +++
block/qed.c |4 ++
block/raw-posix.c | 81
+++--
block/vmdk.c |5 +++
block_int.h |1 +
blockdev.c| 19
monitor.c |5 +++
monitor.h |1 +
qemu-options.hx |8 +++--
qemu-tool.c |5 +++
14 files changed, 149 insertions(+), 12 deletions(-)
diff --git a/block.c b/block.c
index 24a25d5..500db84 100644
--- a/block.c
+++ b/block.c
@@ -536,6 +536,10 @@ int bdrv_open(BlockDriverState *bs, const char
*filename, int flags,
char tmp_filename[PATH_MAX];
char backing_filename[PATH_MAX];
+if (bdrv_is_fd_protocol(bs)) {
+return -ENOTSUP;
+}
Hm, shouldn't that just work even with fd?
My thought was that the proposed SELinux changes would prevent the open of
the temporary backing file if the file corresponding to fd resides on NFS.
But perhaps the backing file is not opened on NFS?
Then make a new flag that allows you to prohibit backing files.
+
/* if snapshot, we create a temporary backing file and open it
instead of opening 'filename' directly */
@@ -585,6 +589,10 @@ int bdrv_open(BlockDriverState *bs, const char
*filename, int flags,
/* Find the right image format driver */
if (!drv) {
+/* format must be
Re: [libvirt] [Qemu-devel] [PATCH v3] Add support for fd: protocol
On 07/26/2011 01:10 PM, Alexander Graf wrote:
On 26.07.2011, at 18:57, Corey Bryant wrote:
Kevin, thanks for the input.
On 07/26/2011 11:18 AM, Kevin Wolf wrote:
Am 26.07.2011 14:51, schrieb Corey Bryant:
sVirt provides SELinux MAC isolation for Qemu guest processes and
their
corresponding resources (image files). sVirt provides this support
by labeling guests and resources with security labels that are stored
in file system extended attributes. Some file systems, such as NFS, do
not support the extended attribute security namespace, which is needed
for image file isolation when using the sVirt SELinux security driver
in libvirt.
The proposed solution entails a combination of Qemu, libvirt, and
SELinux patches that work together to isolate multiple guests' images
when they're stored in the same NFS mount. This results in an
environment where sVirt isolation and NFS image file isolation can
both
be provided.
This patch contains the Qemu code to support this solution. I would
like to solicit input from the libvirt community prior to starting
the libvirt patch.
Currently, Qemu opens an image file in addition to performing the
necessary read and write operations. The proposed solution will move
the open out of Qemu and into libvirt. Once libvirt opens an image
file for the guest, it will pass the file descriptor to Qemu via a
new fd: protocol.
If the image file resides in an NFS mount, the following SELinux
policy
changes will provide image isolation:
- A new SELinux boolean is created (e.g. virt_read_write_nfs) to
allow Qemu (svirt_t) to only have SELinux read and write
permissions on nfs_t files
- Qemu (svirt_t) also gets SELinux use permissions on libvirt
(virtd_t) file descriptors
Following is a sample invocation of Qemu using the fd: protocol on
the command line:
qemu -drive file=fd:4,format=qcow2
The fd: protocol is also supported by the drive_add monitor command.
This requires that the specified file descriptor is passed to the
monitor alongside a prior getfd monitor command.
There are some additional features provided by certain image types
where Qemu reopens the image file. All of these scenarios will be
unsupported for the fd: protocol, at least for this patch:
- The -snapshot command line option
- The savevm monitor command
- The snapshot_blkdev monitor command
- Use of copy-on-write image files
- The -cdrom command line option
- The -drive command line option with media=cdrom
- The change monitor command
The thought is that this support can be added in the future, but is
not required for the initial fd: support.
This patch was tested with the following formats: raw, cow, qcow,
qcow2, qed, and vmdk, using the fd: protocol from the command line
and the monitor. Tests were also run to verify existing file name
support and qemu-img were not regressed. Non-valid file descriptors,
fd: without format, snapshot and backing files, and cdrom were also
tested.
v2:
- Add drive_add monitor command support
- Fence off unsupported features that re-open image file
v3:
- Fence off cdrom and change monitor command support
Signed-off-by: Corey Bryantcor...@linux.vnet.ibm.com
---
block.c | 16 ++
block.h |1 +
block/cow.c |5 +++
block/qcow.c |5 +++
block/qcow2.c |5 +++
block/qed.c |4 ++
block/raw-posix.c | 81
+++--
block/vmdk.c |5 +++
block_int.h |1 +
blockdev.c| 19
monitor.c |5 +++
monitor.h |1 +
qemu-options.hx |8 +++--
qemu-tool.c |5 +++
14 files changed, 149 insertions(+), 12 deletions(-)
diff --git a/block.c b/block.c
index 24a25d5..500db84 100644
--- a/block.c
+++ b/block.c
@@ -536,6 +536,10 @@ int bdrv_open(BlockDriverState *bs, const char
*filename, int flags,
char tmp_filename[PATH_MAX];
char backing_filename[PATH_MAX];
+if (bdrv_is_fd_protocol(bs)) {
+return -ENOTSUP;
+}
Hm, shouldn't that just work even with fd?
My thought was that the proposed SELinux changes would prevent the open of
the temporary backing file if the file corresponding to fd resides on NFS. But
perhaps the backing file is not opened on NFS?
Then make a new flag that allows you to prohibit backing files.
+
/*
