[Qemu-devel] [PULL for-2.4 0/7] Block patches

[Qemu-devel] [PULL for-2.4 0/7] Block patches

[Stefan Hajnoczi]

The following changes since commit f6e3035f75e5c6a73485335765ae070304c7a110:

  Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream-smm' into staging (2015-07-06 23:37:53 +0100)

are available in the git repository at:

  git://github.com/stefanha/qemu.git tags/block-pull-request

for you to fetch changes up to e065a2d586f4f66fe0aa6ef9fc5af3acdfe76fab:

  blockjob: add block_job_release function (2015-07-07 12:53:12 +0100)

----------------------------------------------------------------

----------------------------------------------------------------

Alberto Garcia (1):
  qcow2: remove unnecessary check

Fam Zheng (2):
  block: Initialize local_err in bdrv_append_temp_snapshot
  block: Use bdrv_drain to replace uncessary bdrv_drain_all

Peter Lieven (1):
  block/nfs: add support for setting debug level

Richard W.M. Jones (1):
  block/raw-posix: Don't think /dev/fd/<NN> is a floppy drive.

Stefan Hajnoczi (1):
  block: update bdrv_drain_all()/bdrv_drain() comments

Ting Wang (1):
  blockjob: add block_job_release function

 block.c                  |  8 ++++----
 block/io.c               | 20 ++++++++++----------
 block/mirror.c           |  2 ++
 block/nfs.c              | 28 ++++++++++++++++++++++++----
 block/qcow2-cache.c      |  3 ---
 block/raw-posix.c        |  3 ++-
 block/snapshot.c         |  2 +-
 blockjob.c               | 20 ++++++++++++--------
 include/block/blockjob.h |  8 ++++++++
 migration/block.c        |  2 +-
 qapi/block-core.json     | 20 ++++++++++++++++----
 11 files changed, 80 insertions(+), 36 deletions(-)

-- 
2.4.3

[Qemu-devel] [PULL for-2.4 1/7] qcow2: remove unnecessary check

[Stefan Hajnoczi]

From: Alberto Garcia <berto@igalia.com>

The value of 'i' is guaranteed to be >= 0

Signed-off-by: Alberto Garcia <berto@igalia.com>
Message-id: 1435824371-2660-1-git-send-email-berto@igalia.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/qcow2-cache.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c
index ed92a09..53b8afc 100644
--- a/block/qcow2-cache.c
+++ b/block/qcow2-cache.c
@@ -281,9 +281,6 @@ static int qcow2_cache_do_get(BlockDriverState *bs, Qcow2Cache *c,
     i = min_lru_index;
     trace_qcow2_cache_get_replace_entry(qemu_coroutine_self(),
                                         c == s->l2_table_cache, i);
-    if (i < 0) {
-        return i;
-    }
 
     ret = qcow2_cache_entry_flush(bs, c, i);
     if (ret < 0) {
-- 
2.4.3

[Qemu-devel] [PULL for-2.4 2/7] block: update bdrv_drain_all()/bdrv_drain() comments

[Stefan Hajnoczi]

The doc comments for bdrv_drain_all() and bdrv_drain() are outdated:

 * The bdrv_drain() comment is a poor man's bdrv_lock()/bdrv_unlock()
   which Fam Zheng is currently developing.  Unfortunately this warning
   was never really enough because devices keep submitting I/O and op
   blockers don't prevent that.

 * The bdrv_drain_all() comment is still partially correct but reflects
   the nature of the implementation rather than API documentation.

Do make it clear that bdrv_drain() is only appropriate within an
AioContext.  For anything spanning AioContexts you need
bdrv_drain_all().

Cc: Markus Armbruster <armbru@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Message-id: 1435854281-6078-1-git-send-email-stefanha@redhat.com
---
 block/io.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/block/io.c b/block/io.c
index 305e0d9..d4bc83b 100644
--- a/block/io.c
+++ b/block/io.c
@@ -236,12 +236,12 @@ static bool bdrv_requests_pending(BlockDriverState *bs)
 /*
  * Wait for pending requests to complete on a single BlockDriverState subtree
  *
- * See the warning in bdrv_drain_all().  This function can only be called if
- * you are sure nothing can generate I/O because you have op blockers
- * installed.
- *
  * Note that unlike bdrv_drain_all(), the caller must hold the BlockDriverState
  * AioContext.
+ *
+ * Only this BlockDriverState's AioContext is run, so in-flight requests must
+ * not depend on events in other AioContexts.  In that case, use
+ * bdrv_drain_all() instead.
  */
 void bdrv_drain(BlockDriverState *bs)
 {
@@ -260,12 +260,6 @@ void bdrv_drain(BlockDriverState *bs)
  *
  * This function does not flush data to disk, use bdrv_flush_all() for that
  * after calling this function.
- *
- * Note that completion of an asynchronous I/O operation can trigger any
- * number of other I/O operations on other devices---for example a coroutine
- * can be arbitrarily complex and a constant flow of I/O can come until the
- * coroutine is complete.  Because of this, it is not possible to have a
- * function to drain a single device's I/O queue.
  */
 void bdrv_drain_all(void)
 {
@@ -288,6 +282,12 @@ void bdrv_drain_all(void)
         }
     }
 
+    /* Note that completion of an asynchronous I/O operation can trigger any
+     * number of other I/O operations on other devices---for example a
+     * coroutine can submit an I/O request to another device in response to
+     * request completion.  Therefore we must keep looping until there was no
+     * more activity rather than simply draining each device independently.
+     */
     while (busy) {
         busy = false;
 
-- 
2.4.3

[Qemu-devel] [PULL for-2.4 3/7] block/nfs: add support for setting debug level

[Stefan Hajnoczi]

From: Peter Lieven <pl@kamp.de>

upcoming libnfs versions will support logging debug messages. Add
support for it in qemu through a per-drive option.

Examples:
 qemu -drive if=virtio,file=nfs://...,file.debug=2
 qemu-img create -o debug=2 nfs://... 10G

Signed-off-by: Peter Lieven <pl@kamp.de>
Reviewed-by: Fam Zheng <famz@redhat.com>
Message-id: 1436251847-16875-1-git-send-email-pl@kamp.de
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/nfs.c          | 28 ++++++++++++++++++++++++----
 qapi/block-core.json | 20 ++++++++++++++++----
 2 files changed, 40 insertions(+), 8 deletions(-)

diff --git a/block/nfs.c b/block/nfs.c
index c026ff6..72a4247 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -233,6 +233,11 @@ static QemuOptsList runtime_opts = {
             .type = QEMU_OPT_STRING,
             .help = "URL to the NFS file",
         },
+        {
+            .name = "debug",
+            .type = QEMU_OPT_NUMBER,
+            .help = "Set libnfs debug level (default 0 = no debug)",
+        },
         { /* end of list */ }
     },
 };
@@ -277,9 +282,9 @@ static void nfs_file_close(BlockDriverState *bs)
 }
 
 static int64_t nfs_client_open(NFSClient *client, const char *filename,
-                               int flags, Error **errp)
+                               int flags, QemuOpts *opts, Error **errp)
 {
-    int ret = -EINVAL, i;
+    int ret = -EINVAL, i, debug;
     struct stat st;
     URI *uri;
     QueryParams *qp = NULL;
@@ -343,6 +348,16 @@ static int64_t nfs_client_open(NFSClient *client, const char *filename,
         }
     }
 
+    debug = qemu_opt_get_number(opts, "debug", 0);
+    if (debug) {
+#ifdef LIBNFS_FEATURE_DEBUG
+        nfs_set_debug(client->context, debug);
+#else
+        error_report("NFS Warning: The linked version of libnfs does"
+                     " not support setting debug levels");
+#endif
+    }
+
     ret = nfs_mount(client->context, uri->server, uri->path);
     if (ret < 0) {
         error_setg(errp, "Failed to mount nfs share: %s",
@@ -405,7 +420,7 @@ static int nfs_file_open(BlockDriverState *bs, QDict *options, int flags,
     }
     ret = nfs_client_open(client, qemu_opt_get(opts, "filename"),
                           (flags & BDRV_O_RDWR) ? O_RDWR : O_RDONLY,
-                          errp);
+                          opts, errp);
     if (ret < 0) {
         goto out;
     }
@@ -425,6 +440,11 @@ static QemuOptsList nfs_create_opts = {
             .type = QEMU_OPT_SIZE,
             .help = "Virtual disk size"
         },
+        {
+            .name = "debug",
+            .type = QEMU_OPT_NUMBER,
+            .help = "Set libnfs debug level (default 0 = no debug)",
+        },
         { /* end of list */ }
     }
 };
@@ -441,7 +461,7 @@ static int nfs_file_create(const char *url, QemuOpts *opts, Error **errp)
     total_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
                           BDRV_SECTOR_SIZE);
 
-    ret = nfs_client_open(client, url, O_CREAT, errp);
+    ret = nfs_client_open(client, url, O_CREAT, opts, errp);
     if (ret < 0) {
         goto out;
     }
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 7b2efb8..f43a1b1 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1381,9 +1381,9 @@
 { 'enum': 'BlockdevDriver',
   'data': [ 'archipelago', 'blkdebug', 'blkverify', 'bochs', 'cloop',
             'dmg', 'file', 'ftp', 'ftps', 'host_cdrom', 'host_device',
-            'host_floppy', 'http', 'https', 'null-aio', 'null-co', 'parallels',
-            'qcow', 'qcow2', 'qed', 'quorum', 'raw', 'tftp', 'vdi', 'vhdx',
-            'vmdk', 'vpc', 'vvfat' ] }
+            'host_floppy', 'http', 'https', 'nfs', 'null-aio', 'null-co',
+            'parallels', 'qcow', 'qcow2', 'qed', 'quorum', 'raw', 'tftp',
+            'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat' ] }
 
 ##
 # @BlockdevOptionsBase
@@ -1635,6 +1635,18 @@
             '*vport': 'int',
             '*segment': 'str' } }
 
+##
+# @BlockdevOptionsNFS
+#
+# Driver specific block device options for NFS.
+#
+# @debug:       #optional set libnfs debug level (default: 0 = disabled)
+#
+# Since: 2.4
+##
+{ 'struct': 'BlockdevOptionsNFS',
+  'base': 'BlockdevOptionsFile',
+  'data': { '*debug': 'int' } }
 
 ##
 # @BlkdebugEvent
@@ -1816,7 +1828,7 @@
       'https':      'BlockdevOptionsFile',
 # TODO iscsi: Wait for structured options
 # TODO nbd: Should take InetSocketAddress for 'host'?
-# TODO nfs: Wait for structured options
+      'nfs':        'BlockdevOptionsNFS',
       'null-aio':   'BlockdevOptionsNull',
       'null-co':    'BlockdevOptionsNull',
       'parallels':  'BlockdevOptionsGenericFormat',
-- 
2.4.3

[Qemu-devel] [PULL for-2.4 4/7] block: Initialize local_err in bdrv_append_temp_snapshot

[Stefan Hajnoczi]

From: Fam Zheng <famz@redhat.com>

Cc: qemu-stable@nongnu.org
Signed-off-by: Fam Zheng <famz@redhat.com>
Message-id: 1436156684-16526-1-git-send-email-famz@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block.c b/block.c
index 7e130cc..42eb8e3 100644
--- a/block.c
+++ b/block.c
@@ -1271,7 +1271,7 @@ int bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error **errp)
     QemuOpts *opts = NULL;
     QDict *snapshot_options;
     BlockDriverState *bs_snapshot;
-    Error *local_err;
+    Error *local_err = NULL;
     int ret;
 
     /* if snapshot, we create a temporary backing file and open it
-- 
2.4.3

[Qemu-devel] [PULL for-2.4 5/7] block: Use bdrv_drain to replace uncessary bdrv_drain_all

[Stefan Hajnoczi]

From: Fam Zheng <famz@redhat.com>

There callers work on a single BlockDriverState subtree, where using
bdrv_drain() is more accurate.

Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block.c           | 6 +++---
 block/snapshot.c  | 2 +-
 migration/block.c | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/block.c b/block.c
index 42eb8e3..5e80336 100644
--- a/block.c
+++ b/block.c
@@ -1841,9 +1841,9 @@ void bdrv_close(BlockDriverState *bs)
     if (bs->job) {
         block_job_cancel_sync(bs->job);
     }
-    bdrv_drain_all(); /* complete I/O */
+    bdrv_drain(bs); /* complete I/O */
     bdrv_flush(bs);
-    bdrv_drain_all(); /* in case flush left pending I/O */
+    bdrv_drain(bs); /* in case flush left pending I/O */
     notifier_list_notify(&bs->close_notifiers, bs);
 
     if (bs->drv) {
@@ -3906,7 +3906,7 @@ void bdrv_attach_aio_context(BlockDriverState *bs,
 
 void bdrv_set_aio_context(BlockDriverState *bs, AioContext *new_context)
 {
-    bdrv_drain_all(); /* ensure there are no in-flight requests */
+    bdrv_drain(bs); /* ensure there are no in-flight requests */
 
     bdrv_detach_aio_context(bs);
 
diff --git a/block/snapshot.c b/block/snapshot.c
index 19395ae..49e143e 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -239,7 +239,7 @@ int bdrv_snapshot_delete(BlockDriverState *bs,
     }
 
     /* drain all pending i/o before deleting snapshot */
-    bdrv_drain_all();
+    bdrv_drain(bs);
 
     if (drv->bdrv_snapshot_delete) {
         return drv->bdrv_snapshot_delete(bs, snapshot_id, name, errp);
diff --git a/migration/block.c b/migration/block.c
index ddb59cc..ed865ed 100644
--- a/migration/block.c
+++ b/migration/block.c
@@ -457,7 +457,7 @@ static int mig_save_device_dirty(QEMUFile *f, BlkMigDevState *bmds,
         blk_mig_lock();
         if (bmds_aio_inflight(bmds, sector)) {
             blk_mig_unlock();
-            bdrv_drain_all();
+            bdrv_drain(bmds->bs);
         } else {
             blk_mig_unlock();
         }
-- 
2.4.3

[Qemu-devel] [PULL for-2.4 6/7] block/raw-posix: Don't think /dev/fd/<NN> is a floppy drive.

[Stefan Hajnoczi]

From: "Richard W.M. Jones" <rjones@redhat.com>

In libguestfs we use /dev/fd/<NN> to pass pre-opened file descriptors
to qemu-img.  Lately I've discovered that although this works, qemu
believes that these are floppy disk images.  That in itself isn't much
of a problem, but now qemu prints a warning about host floppy
pass-thru being deprecated.

Extend the existing test so that it ignores /dev/fd/ as well as
/dev/fdset/

A simple test of this, if you are using the bash shell, is:

  qemu-img info <( cat /dev/null )

without this patch:

  $ qemu-img info <( cat /dev/null )
  qemu-img: Host floppy pass-through is deprecated
  Support for it will be removed in a future release.
  qemu-img: Could not open '/dev/fd/63': Could not refresh total sector count: Illegal seek

with this patch:

  $ qemu-img info <( cat /dev/null )
  qemu-img: Could not open '/dev/fd/63': Could not refresh total sector count: Illegal seek

Signed-off-by: Richard W.M. Jones <rjones@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-id: 1435761614-31358-1-git-send-email-rjones@redhat.com
Fixes: https://bugs.launchpad.net/qemu/+bug/1470536
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/raw-posix.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index cbe6574..855febe 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -2430,7 +2430,8 @@ static int floppy_probe_device(const char *filename)
     struct stat st;
 
     if (strstart(filename, "/dev/fd", NULL) &&
-        !strstart(filename, "/dev/fdset/", NULL)) {
+        !strstart(filename, "/dev/fdset/", NULL) &&
+        !strstart(filename, "/dev/fd/", NULL)) {
         prio = 50;
     }
 
-- 
2.4.3

[Qemu-devel] [PULL for-2.4 7/7] blockjob: add block_job_release function

[Stefan Hajnoczi]

From: Ting Wang <kathy.wangting@huawei.com>

There is job resource leak in function mirror_start_job,
although bdrv_create_dirty_bitmap is unlikely failed.
Add block_job_release for each release when needed.

Signed-off-by: Ting Wang <kathy.wangting@huawei.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 1435311455-56048-1-git-send-email-kathy.wangting@huawei.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/mirror.c           |  2 ++
 blockjob.c               | 20 ++++++++++++--------
 include/block/blockjob.h |  8 ++++++++
 3 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index 8888cea..d409337 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -708,6 +708,8 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
 
     s->dirty_bitmap = bdrv_create_dirty_bitmap(bs, granularity, NULL, errp);
     if (!s->dirty_bitmap) {
+        g_free(s->replaces);
+        block_job_release(bs);
         return;
     }
     bdrv_set_enable_write_cache(s->target, true);
diff --git a/blockjob.c b/blockjob.c
index ec46fad..62bb906 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -66,10 +66,7 @@ void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,
 
         block_job_set_speed(job, speed, &local_err);
         if (local_err) {
-            bs->job = NULL;
-            bdrv_op_unblock_all(bs, job->blocker);
-            error_free(job->blocker);
-            g_free(job);
+            block_job_release(bs);
             error_propagate(errp, local_err);
             return NULL;
         }
@@ -77,18 +74,25 @@ void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,
     return job;
 }
 
-void block_job_completed(BlockJob *job, int ret)
+void block_job_release(BlockDriverState *bs)
 {
-    BlockDriverState *bs = job->bs;
+    BlockJob *job = bs->job;
 
-    assert(bs->job == job);
-    job->cb(job->opaque, ret);
     bs->job = NULL;
     bdrv_op_unblock_all(bs, job->blocker);
     error_free(job->blocker);
     g_free(job);
 }
 
+void block_job_completed(BlockJob *job, int ret)
+{
+    BlockDriverState *bs = job->bs;
+
+    assert(bs->job == job);
+    job->cb(job->opaque, ret);
+    block_job_release(bs);
+}
+
 void block_job_set_speed(BlockJob *job, int64_t speed, Error **errp)
 {
     Error *local_err = NULL;
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index 57d8ef1..dd9d5e6 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -166,6 +166,14 @@ void block_job_sleep_ns(BlockJob *job, QEMUClockType type, int64_t ns);
 void block_job_yield(BlockJob *job);
 
 /**
+ * block_job_release:
+ * @bs: The block device.
+ *
+ * Release job resources when an error occurred or job completed.
+ */
+void block_job_release(BlockDriverState *bs);
+
+/**
  * block_job_completed:
  * @job: The job being completed.
  * @ret: The status code.
-- 
2.4.3

Re: [Qemu-devel] [PULL for-2.4 3/7] block/nfs: add support for setting debug level

[Peter Lieven]

Am 07.07.2015 um 14:08 schrieb Stefan Hajnoczi:
> From: Peter Lieven <pl@kamp.de>
>
> upcoming libnfs versions will support logging debug messages. Add
> support for it in qemu through a per-drive option.
>
> Examples:
>   qemu -drive if=virtio,file=nfs://...,file.debug=2
>   qemu-img create -o debug=2 nfs://... 10G
>
> Signed-off-by: Peter Lieven <pl@kamp.de>
> Reviewed-by: Fam Zheng <famz@redhat.com>
> Message-id: 1436251847-16875-1-git-send-email-pl@kamp.de
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

Thanks for pulling this.

Question: Is it possible to insert a CDROM other than with the 'change' command?
The change command obviously does not support file.debug parameters...

Peter

Re: [Qemu-devel] [PULL for-2.4 3/7] block/nfs: add support for setting debug level

[Stefan Hajnoczi]

On Tue, Jul 7, 2015 at 1:50 PM, Peter Lieven <pl@kamp.de> wrote:
> Am 07.07.2015 um 14:08 schrieb Stefan Hajnoczi:
>>
>> From: Peter Lieven <pl@kamp.de>
>>
>> upcoming libnfs versions will support logging debug messages. Add
>> support for it in qemu through a per-drive option.
>>
>> Examples:
>>   qemu -drive if=virtio,file=nfs://...,file.debug=2
>>   qemu-img create -o debug=2 nfs://... 10G
>>
>> Signed-off-by: Peter Lieven <pl@kamp.de>
>> Reviewed-by: Fam Zheng <famz@redhat.com>
>> Message-id: 1436251847-16875-1-git-send-email-pl@kamp.de
>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>
>
> Thanks for pulling this.
>
> Question: Is it possible to insert a CDROM other than with the 'change'
> command?
> The change command obviously does not support file.debug parameters...

Not that I'm aware of.

You may be able to hotplug SCSI CD-ROM drives, but that's not the same
as ejecting/inserting CD-ROM media.

Sounds like an item for the block layer todo list:
http://qemu-project.org/Features/Block/Todo

Stefan

Re: [Qemu-devel] [PULL for-2.4 3/7] block/nfs: add support for setting debug level

[Stefan Hajnoczi]

On Tue, Jul 7, 2015 at 1:08 PM, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> From: Peter Lieven <pl@kamp.de>
>
> upcoming libnfs versions will support logging debug messages. Add
> support for it in qemu through a per-drive option.
>
> Examples:
>  qemu -drive if=virtio,file=nfs://...,file.debug=2
>  qemu-img create -o debug=2 nfs://... 10G
>
> Signed-off-by: Peter Lieven <pl@kamp.de>
> Reviewed-by: Fam Zheng <famz@redhat.com>
> Message-id: 1436251847-16875-1-git-send-email-pl@kamp.de
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  block/nfs.c          | 28 ++++++++++++++++++++++++----
>  qapi/block-core.json | 20 ++++++++++++++++----
>  2 files changed, 40 insertions(+), 8 deletions(-)

Kevin has pointed out at the QAPI portion of this patch isn't ready
for prime time yet, so I will revert the patch and resend the pull
request.

Re: [Qemu-devel] [PULL for-2.4 3/7] block/nfs: add support for setting debug level

[Kevin Wolf]

Am 07.07.2015 um 15:13 hat Stefan Hajnoczi geschrieben:
> On Tue, Jul 7, 2015 at 1:50 PM, Peter Lieven <pl@kamp.de> wrote:
> > Am 07.07.2015 um 14:08 schrieb Stefan Hajnoczi:
> >>
> >> From: Peter Lieven <pl@kamp.de>
> >>
> >> upcoming libnfs versions will support logging debug messages. Add
> >> support for it in qemu through a per-drive option.
> >>
> >> Examples:
> >>   qemu -drive if=virtio,file=nfs://...,file.debug=2
> >>   qemu-img create -o debug=2 nfs://... 10G
> >>
> >> Signed-off-by: Peter Lieven <pl@kamp.de>
> >> Reviewed-by: Fam Zheng <famz@redhat.com>
> >> Message-id: 1436251847-16875-1-git-send-email-pl@kamp.de
> >> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> >
> >
> > Thanks for pulling this.
> >
> > Question: Is it possible to insert a CDROM other than with the 'change'
> > command?
> > The change command obviously does not support file.debug parameters...
> 
> Not that I'm aware of.
> 
> You may be able to hotplug SCSI CD-ROM drives, but that's not the same
> as ejecting/inserting CD-ROM media.
> 
> Sounds like an item for the block layer todo list:
> http://qemu-project.org/Features/Block/Todo

I think Max' media handling series might implement this or at least go a
step in that direction. CCing Max.

Kevin

Re: [Qemu-devel] [PULL for-2.4 3/7] block/nfs: add support for setting debug level

[Peter Lieven]

Am 07.07.2015 um 15:24 schrieb Stefan Hajnoczi:
> On Tue, Jul 7, 2015 at 1:08 PM, Stefan Hajnoczi <stefanha@redhat.com> wrote:
>> From: Peter Lieven <pl@kamp.de>
>>
>> upcoming libnfs versions will support logging debug messages. Add
>> support for it in qemu through a per-drive option.
>>
>> Examples:
>>   qemu -drive if=virtio,file=nfs://...,file.debug=2
>>   qemu-img create -o debug=2 nfs://... 10G
>>
>> Signed-off-by: Peter Lieven <pl@kamp.de>
>> Reviewed-by: Fam Zheng <famz@redhat.com>
>> Message-id: 1436251847-16875-1-git-send-email-pl@kamp.de
>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>> ---
>>   block/nfs.c          | 28 ++++++++++++++++++++++++----
>>   qapi/block-core.json | 20 ++++++++++++++++----
>>   2 files changed, 40 insertions(+), 8 deletions(-)
> Kevin has pointed out at the QAPI portion of this patch isn't ready
> for prime time yet, so I will revert the patch and resend the pull
> request.

Yes please, I was not aware that adding the QAPI part
has such an impact. I would appreciate if someone who has
more experience with the QAPI stuff would look at this and
make a proposal how to handle the NFS (and also the iSCSI)
case. I'm happy to look at the implementation then.

Peter

Re: [Qemu-devel] [PULL for-2.4 3/7] block/nfs: add support for setting debug level

[Kevin Wolf]

Am 07.07.2015 um 15:41 hat Peter Lieven geschrieben:
> Am 07.07.2015 um 15:24 schrieb Stefan Hajnoczi:
> >On Tue, Jul 7, 2015 at 1:08 PM, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> >>From: Peter Lieven <pl@kamp.de>
> >>
> >>upcoming libnfs versions will support logging debug messages. Add
> >>support for it in qemu through a per-drive option.
> >>
> >>Examples:
> >>  qemu -drive if=virtio,file=nfs://...,file.debug=2
> >>  qemu-img create -o debug=2 nfs://... 10G
> >>
> >>Signed-off-by: Peter Lieven <pl@kamp.de>
> >>Reviewed-by: Fam Zheng <famz@redhat.com>
> >>Message-id: 1436251847-16875-1-git-send-email-pl@kamp.de
> >>Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> >>---
> >>  block/nfs.c          | 28 ++++++++++++++++++++++++----
> >>  qapi/block-core.json | 20 ++++++++++++++++----
> >>  2 files changed, 40 insertions(+), 8 deletions(-)
> >Kevin has pointed out at the QAPI portion of this patch isn't ready
> >for prime time yet, so I will revert the patch and resend the pull
> >request.
> 
> Yes please, I was not aware that adding the QAPI part
> has such an impact. I would appreciate if someone who has
> more experience with the QAPI stuff would look at this and
> make a proposal how to handle the NFS (and also the iSCSI)
> case. I'm happy to look at the implementation then.

We should definitely come to a conclusion on this for 2.5. The same
problem doesn't only apply for NFS, but for all network protocols, and
this is one of the major reasons why blockdev-add isn't ready yet.

We'll need Eric for this discussion, though, and he'll only return from
vacation next Monday.

Kevin

Re: [Qemu-devel] [PULL for-2.4 3/7] block/nfs: add support for setting debug level

[Max Reitz]

On 07.07.2015 15:40, Kevin Wolf wrote:
> Am 07.07.2015 um 15:13 hat Stefan Hajnoczi geschrieben:
>> On Tue, Jul 7, 2015 at 1:50 PM, Peter Lieven <pl@kamp.de> wrote:
>>> Am 07.07.2015 um 14:08 schrieb Stefan Hajnoczi:
>>>>
>>>> From: Peter Lieven <pl@kamp.de>
>>>>
>>>> upcoming libnfs versions will support logging debug messages. Add
>>>> support for it in qemu through a per-drive option.
>>>>
>>>> Examples:
>>>>    qemu -drive if=virtio,file=nfs://...,file.debug=2
>>>>    qemu-img create -o debug=2 nfs://... 10G
>>>>
>>>> Signed-off-by: Peter Lieven <pl@kamp.de>
>>>> Reviewed-by: Fam Zheng <famz@redhat.com>
>>>> Message-id: 1436251847-16875-1-git-send-email-pl@kamp.de
>>>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>>>
>>>
>>> Thanks for pulling this.
>>>
>>> Question: Is it possible to insert a CDROM other than with the 'change'
>>> command?
>>> The change command obviously does not support file.debug parameters...
>>
>> Not that I'm aware of.
>>
>> You may be able to hotplug SCSI CD-ROM drives, but that's not the same
>> as ejecting/inserting CD-ROM media.
>>
>> Sounds like an item for the block layer todo list:
>> http://qemu-project.org/Features/Block/Todo
>
> I think Max' media handling series might implement this or at least go a
> step in that direction. CCing Max.

Since with that series, you can insert any BDS as a medium, yes, it 
naturally does.

But for now, wouldn't JSON file names work?

Max

Re: [Qemu-devel] [PULL for-2.4 3/7] block/nfs: add support for setting debug level

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1739 bytes --]

On 07/07/2015 06:08 AM, Stefan Hajnoczi wrote:
> From: Peter Lieven <pl@kamp.de>
> 
> upcoming libnfs versions will support logging debug messages. Add
> support for it in qemu through a per-drive option.
> 
> Examples:
>  qemu -drive if=virtio,file=nfs://...,file.debug=2
>  qemu-img create -o debug=2 nfs://... 10G
> 
> Signed-off-by: Peter Lieven <pl@kamp.de>
> Reviewed-by: Fam Zheng <famz@redhat.com>
> Message-id: 1436251847-16875-1-git-send-email-pl@kamp.de
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  block/nfs.c          | 28 ++++++++++++++++++++++++----
>  qapi/block-core.json | 20 ++++++++++++++++----
>  2 files changed, 40 insertions(+), 8 deletions(-)
> 

> +++ b/qapi/block-core.json
> @@ -1381,9 +1381,9 @@
>  { 'enum': 'BlockdevDriver',
>    'data': [ 'archipelago', 'blkdebug', 'blkverify', 'bochs', 'cloop',
>              'dmg', 'file', 'ftp', 'ftps', 'host_cdrom', 'host_device',
> -            'host_floppy', 'http', 'https', 'null-aio', 'null-co', 'parallels',
> -            'qcow', 'qcow2', 'qed', 'quorum', 'raw', 'tftp', 'vdi', 'vhdx',
> -            'vmdk', 'vpc', 'vvfat' ] }
> +            'host_floppy', 'http', 'https', 'nfs', 'null-aio', 'null-co',
> +            'parallels', 'qcow', 'qcow2', 'qed', 'quorum', 'raw', 'tftp',
> +            'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat' ] }

Probably also ought to document the docs for BlockDeviceInfo.drv, to
mention that 'nfs' was added in 2.4.

[may be moot for now, since this patch was dropped when the pull request
was resent; or even mean you want "2.5" when you DO add and document it]

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PULL for-2.4 3/7] block/nfs: add support for setting debug level

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1072 bytes --]

On 07/07/2015 07:45 AM, Kevin Wolf wrote:

>>> Kevin has pointed out at the QAPI portion of this patch isn't ready
>>> for prime time yet, so I will revert the patch and resend the pull
>>> request.
>>
>> Yes please, I was not aware that adding the QAPI part
>> has such an impact. I would appreciate if someone who has
>> more experience with the QAPI stuff would look at this and
>> make a proposal how to handle the NFS (and also the iSCSI)
>> case. I'm happy to look at the implementation then.
> 
> We should definitely come to a conclusion on this for 2.5. The same
> problem doesn't only apply for NFS, but for all network protocols, and
> this is one of the major reasons why blockdev-add isn't ready yet.
> 
> We'll need Eric for this discussion, though, and he'll only return from
> vacation next Monday.

I'm back now (obviously), but we should probably discuss in a separate
thread rather than dangling off this pull request ;)

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]