[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [PATCH v2 15/36] file-posix: Support .bdrv_co_create

From: Eric Blake
Subject: Re: [Qemu-block] [PATCH v2 15/36] file-posix: Support .bdrv_co_create
Date: Thu, 22 Feb 2018 17:34:28 -0600
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0

On 02/21/2018 07:53 AM, Kevin Wolf wrote:
This adds the .bdrv_co_create driver callback to file, which enables
image creation over QMP.

Signed-off-by: Kevin Wolf <address@hidden>
Reviewed-by: Max Reitz <address@hidden>
  qapi/block-core.json | 20 +++++++++++++-
  block/file-posix.c   | 77 +++++++++++++++++++++++++++++++++++++---------------
  2 files changed, 74 insertions(+), 23 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 359195a1a3..0040795603 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3359,6 +3359,24 @@
  { 'command': 'blockdev-del', 'data': { 'node-name': 'str' } }
+# @BlockdevCreateOptionsFile:
+# Driver specific image creation options for file.
+# @filename         Filename for the new image file

Does this allow /dev/fdset magic filenames, for when libvirt has to pre-create a file and assign correct SELinux permissions, then hand the fd over the monitor, but where qemu then takes over the rest of the creation task? What tasks remain in file-posix creation, you ask?...

+# @size             Size of the virtual disk in bytes
+# @preallocation    Preallocation mode for the new image (default: off)

...why, of course, a non-default preallocation. It would be nice to hand a 0-byte fd to qemu, then let qemu uniformly truncate it to its desired size, using whatever preallocation strategy is supported, rather than having to have libvirt worry about preallocation in addition to SELinux labeling. Especially once we get an async command variant working, where we can track progress, given that some forms of preallocation take a while. And we may someday still reach the policy decision where we can block qemu from directly calling open().

+# @nocow            Turn off copy-on-write (valid only on btrfs; default: off)
+# Since: 2.12
+{ 'struct': 'BlockdevCreateOptionsFile',
+  'data': { 'filename':         'str',
+            'size':             'size',
+            '*preallocation':   'PreallocMode',
+            '*nocow':           'bool' } }

I think I asked earlier why size is mandatory at this layer, but I'm still okay with that.

Hmm, since "creating" a file can be a destructive operation (if size requires a downwards truncation, or even if we intentionally wipe the first sector so that any prior bits that resemble a different format are no longer visible, or if preallocation explicitly wipes the entire image...), do we want to have any safeguards in place so that creation is attempted only on a newly-opened BDS, or with a force flag if size does not match the current size, or so on? That's more a question for the x-blockdev-create command though, so it doesn't stop review of this patch.

- fd = qemu_open(filename, O_RDWR | O_CREAT | O_TRUNC | O_BINARY,
+    /* Create file */
+    fd = qemu_open(file_opts->filename, O_RDWR | O_CREAT | O_TRUNC | O_BINARY,

At any rate, qemu_open() means that we be supporting /dev/fdset magic on a passed-in fd.

Reviewed-by: Eric Blake <address@hidden>

Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

reply via email to

[Prev in Thread] Current Thread [Next in Thread]