Re: [Qemu-devel] [PATCH 13/13] blockdev: New -blockdev to define a host block device

2010-06-04 Thread Markus Armbruster
Christoph Hellwig h...@lst.de writes:

 On Wed, Jun 02, 2010 at 06:55:29PM +0200, Markus Armbruster wrote:
 Existing -drive defines both host and guest part.  To make it work
 with -device, we created if=none.  But all this does is peel off guest
 device selection.  The other guest properties such as geometry,
 removable vs. fixed media, and serial number are still in the wrong
 place.
 
 Instead of overloading -drive even further, create a new, clean option
 to define a host block device.  -drive stays around unchanged for
 command line convenience and backwards compatibility.
 
 This is just a first step.  Future work includes:

 One thing we really needs is a protocol option.  The current colon
 syntax means we can't support filenames with colons in them which
 users keep requesting.  By making the protocol a separate option
 we can sort this out.

You're absolutely right.  I'll look into it.



Re: [Qemu-devel] [PATCH 13/13] blockdev: New -blockdev to define a host block device

2010-06-03 Thread Christoph Hellwig
On Wed, Jun 02, 2010 at 06:55:29PM +0200, Markus Armbruster wrote:
 Existing -drive defines both host and guest part.  To make it work
 with -device, we created if=none.  But all this does is peel off guest
 device selection.  The other guest properties such as geometry,
 removable vs. fixed media, and serial number are still in the wrong
 place.
 
 Instead of overloading -drive even further, create a new, clean option
 to define a host block device.  -drive stays around unchanged for
 command line convenience and backwards compatibility.
 
 This is just a first step.  Future work includes:

One thing we really needs is a protocol option.  The current colon
syntax means we can't support filenames with colons in them which
users keep requesting.  By making the protocol a separate option
we can sort this out.




[Qemu-devel] [PATCH 13/13] blockdev: New -blockdev to define a host block device

2010-06-02 Thread Markus Armbruster
Existing -drive defines both host and guest part.  To make it work
with -device, we created if=none.  But all this does is peel off guest
device selection.  The other guest properties such as geometry,
removable vs. fixed media, and serial number are still in the wrong
place.

Instead of overloading -drive even further, create a new, clean option
to define a host block device.  -drive stays around unchanged for
command line convenience and backwards compatibility.

This is just a first step.  Future work includes:

* A set of monitor commands to go with it.

* Let device model declare media removable.  -drive has that in the
  host part, as media=(cdrom|floppy), but it really belongs to the
  guest part.

* Turn geometry into device properties.  This will also ensure proper
  range checking.  The existing range checking for -drive can't work
  with if=none.

* Make device models reject error actions they don't support.  The
  existing check for -drive can't work with if=none.

* Like -drive, -blockdev ignores cache= silently when snapshot=on.  Do
  we really want that?

Signed-off-by: Markus Armbruster arm...@redhat.com
---
 blockdev.c  |  141 ++
 blockdev.h  |2 +
 qemu-config.c   |   38 +++
 qemu-config.h   |1 +
 qemu-options.hx |   49 +++
 vl.c|   29 ++-
 6 files changed, 246 insertions(+), 14 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index a1e6394..e03ecfc 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -98,6 +98,132 @@ static int parse_block_aio(const char *val)
 }
 }
 
+static int blockdev_insert(BlockDriverState *bs, QemuOpts *opts)
+{
+int snapshot = qemu_opt_get_bool(opts, snapshot, 0);
+const char *file = qemu_opt_get(opts, file);
+const char *cache = qemu_opt_get(opts, cache);
+const char *aio = qemu_opt_get(opts, aio);
+const char *format = qemu_opt_get(opts, format);
+const char *rerror = qemu_opt_get(opts, rerror);
+const char *werror = qemu_opt_get(opts, werror);
+int readonly = qemu_opt_get_bool(opts, readonly, 0);
+BlockDriver *drv;
+int res, flags;
+BlockErrorAction on_read_error, on_write_error;
+
+if (!file) {
+qerror_report(QERR_MISSING_PARAMETER, file);
+return -1;
+}
+
+drv = NULL;
+if (format) {
+drv = parse_block_format(format);
+if (!drv) {
+return -1;
+}
+}
+
+res = parse_block_error_action(rerror, 1);
+if (on_read_error  0) {
+return -1;
+}
+on_read_error = res;
+res = parse_block_error_action(werror, 0);
+if (res  0) {
+return -1;
+}
+on_write_error = res;
+
+flags = 0;
+res = parse_block_cache(cache);
+if (res  0) {
+return -1;
+}
+flags |= res;
+if (snapshot) {
+/* always use write-back with snapshot */
+/* FIXME ignores explicit cache= *silently*; really want that? */
+flags = ~BDRV_O_CACHE_MASK;
+flags |= (BDRV_O_SNAPSHOT | BDRV_O_CACHE_WB);
+flags |= BDRV_O_SNAPSHOT;
+}
+res = parse_block_aio(aio);
+if (res  0) {
+return -1;
+}
+flags |= res;
+flags |= readonly ? 0 : BDRV_O_RDWR;
+
+bdrv_set_on_error(bs, on_read_error, on_write_error);
+res = bdrv_open(bs, file, flags, drv);
+if (res  0) {
+qerror_report(QERR_OPEN_FILE_FAILED, file);
+bdrv_close(bs);
+return -1;
+}
+return 0;
+}
+
+BlockDriverState *blockdev_open(QemuOpts *opts)
+{
+const char *id = qemu_opts_id(opts);
+const char *file = qemu_opt_get(opts, file);
+BlockDriverState *bs;
+QemuOpt *opt;
+const char *name;
+
+if (!id) {
+qerror_report(QERR_MISSING_PARAMETER, id);
+return NULL;
+}
+
+bs = bdrv_find(id);
+if (bs) {
+qerror_report(QERR_DUPLICATE_ID, id, blockdev);
+return NULL;
+}
+bs = bdrv_new(id);
+
+if (!file) {
+/* file is optional only if no other options are present; check */
+opt = NULL;
+while ((opt = qemu_opt_next(opts, opt))) {
+name = qemu_opt_name(opt);
+if (strcmp(name, file)) {
+qerror_report(QERR_MISSING_PARAMETER, file);
+return NULL;
+}
+}
+/* block device without media wanted */
+return bs;
+}
+
+if (blockdev_insert(bs, opts)  0) {
+return NULL;
+}
+return bs;
+}
+
+static void blockdev_format_help_iter(void *opaque, const char *name)
+{
+error_printf( %s, name);
+}
+
+int blockdev_format_help(QemuOpts *opts)
+{
+const char *format = qemu_opt_get(opts, format);
+
+if (format  !strcmp(format, ?)) {
+error_printf(Supported block device formats:);
+bdrv_iterate_format(blockdev_format_help_iter, NULL);
+error_printf(\n);
+return 1;
+}
+return 0;
+}
+
 static int blockdev_del_dinfo(BlockDriverState *bs)