[Qemu-block] [PATCH 2/2] block: rename raw-{posix, win32} to file-*.c

2016-10-19 Thread Eric Blake 
These files deal with the file protocol, not the raw format (the
file protocol is often used with other formats, and the raw
protocol is not forced to use the file format).  Rename things
to make it a bit easier to follow.

Suggested-by: Daniel P. Berrange 
Signed-off-by: Eric Blake 
---
 include/block/block_int.h   | 2 +-
 block/{raw-posix.c => file-posix.c} | 0
 block/{raw-win32.c => file-win32.c} | 0
 block/gluster.c | 4 ++--
 MAINTAINERS | 4 ++--
 block/Makefile.objs | 4 ++--
 block/trace-events  | 4 ++--
 configure   | 2 +-
 8 files changed, 10 insertions(+), 10 deletions(-)
 rename block/{raw-posix.c => file-posix.c} (100%)
 rename block/{raw-win32.c => file-win32.c} (100%)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index 3e79228..36d26cc 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -186,7 +186,7 @@ struct BlockDriver {

 /*
  * Flushes all data that was already written to the OS all the way down to
- * the disk (for example raw-posix calls fsync()).
+ * the disk (for example file-posix.c calls fsync()).
  */
 int coroutine_fn (*bdrv_co_flush_to_disk)(BlockDriverState *bs);

diff --git a/block/raw-posix.c b/block/file-posix.c
similarity index 100%
rename from block/raw-posix.c
rename to block/file-posix.c
diff --git a/block/raw-win32.c b/block/file-win32.c
similarity index 100%
rename from block/raw-win32.c
rename to block/file-win32.c
diff --git a/block/gluster.c b/block/gluster.c
index af76d7d5..257ade5 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -1172,7 +1172,7 @@ static int qemu_gluster_has_zero_init(BlockDriverState 
*bs)
  * If @start is in a trailing hole or beyond EOF, return -ENXIO.
  * If we can't find out, return a negative errno other than -ENXIO.
  *
- * (Shamefully copied from raw-posix.c, only miniscule adaptions.)
+ * (Shamefully copied from file-posix.c, only miniscule adaptions.)
  */
 static int find_allocation(BlockDriverState *bs, off_t start,
off_t *data, off_t *hole)
@@ -1262,7 +1262,7 @@ static int find_allocation(BlockDriverState *bs, off_t 
start,
  * 'nb_sectors' is the max value 'pnum' should be set to.  If nb_sectors goes
  * beyond the end of the disk image it will be clamped.
  *
- * (Based on raw_co_get_block_status() from raw-posix.c.)
+ * (Based on raw_co_get_block_status() from file-posix.c.)
  */
 static int64_t coroutine_fn qemu_gluster_co_get_block_status(
 BlockDriverState *bs, int64_t sector_num, int nb_sectors, int *pnum,
diff --git a/MAINTAINERS b/MAINTAINERS
index 6f984c3..950ebea 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1675,8 +1675,8 @@ L: qemu-block@nongnu.org
 S: Supported
 F: block/linux-aio.c
 F: include/block/raw-aio.h
-F: block/raw-posix.c
-F: block/raw-win32.c
+F: block/file-posix.c
+F: block/file-win32.c
 F: block/raw.c
 F: block/win32-aio.c

diff --git a/block/Makefile.objs b/block/Makefile.objs
index c10941e..3fe8910 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -6,8 +6,8 @@ block-obj-y += vhdx.o vhdx-endian.o vhdx-log.o
 block-obj-y += quorum.o
 block-obj-y += parallels.o blkdebug.o blkverify.o blkreplay.o
 block-obj-y += block-backend.o snapshot.o qapi.o
-block-obj-$(CONFIG_WIN32) += raw-win32.o win32-aio.o
-block-obj-$(CONFIG_POSIX) += raw-posix.o
+block-obj-$(CONFIG_WIN32) += file-win32.o win32-aio.o
+block-obj-$(CONFIG_POSIX) += file-posix.o
 block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
 block-obj-y += null.o mirror.o commit.o io.o
 block-obj-y += throttle-groups.o
diff --git a/block/trace-events b/block/trace-events
index 05fa13c..627214b 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -55,8 +55,8 @@ qmp_block_job_complete(void *job) "job %p"
 block_job_cb(void *bs, void *job, int ret) "bs %p job %p ret %d"
 qmp_block_stream(void *bs, void *job) "bs %p job %p"

-# block/raw-win32.c
-# block/raw-posix.c
+# block/file-win32.c
+# block/file-posix.c
 paio_submit_co(int64_t offset, int count, int type) "offset %"PRId64" count %d 
type %d"
 paio_submit(void *acb, void *opaque, int64_t offset, int count, int type) "acb 
%p opaque %p offset %"PRId64" count %d type %d"

diff --git a/configure b/configure
index dd9e679..e2b169f 100755
--- a/configure
+++ b/configure
@@ -2727,7 +2727,7 @@ if compile_prog "" "" ; then
 fi

 ##
-# xfsctl() probe, used for raw-posix
+# xfsctl() probe, used for file-posix.c
 if test "$xfs" != "no" ; then
   cat > $TMPC << EOF
 #include   /* NULL */
-- 
2.7.4

[Qemu-block] [PATCH 0/2] less confusing block file names

2016-10-19 Thread Eric Blake 
Based on a suggestion here:
https://lists.gnu.org/archive/html/qemu-devel/2016-10/msg00350.html

Eric Blake (2):
  block: Rename raw_bsd to raw.c
  block: rename raw-{posix,win32} to file-*.c

 include/block/block_int.h   | 2 +-
 block/{raw-posix.c => file-posix.c} | 0
 block/{raw-win32.c => file-win32.c} | 0
 block/gluster.c | 4 ++--
 block/{raw_bsd.c => raw.c}  | 0
 MAINTAINERS | 6 +++---
 block/Makefile.objs | 6 +++---
 block/trace-events  | 4 ++--
 configure   | 2 +-
 9 files changed, 12 insertions(+), 12 deletions(-)
 rename block/{raw-posix.c => file-posix.c} (100%)
 rename block/{raw-win32.c => file-win32.c} (100%)
 rename block/{raw_bsd.c => raw.c} (100%)

-- 
2.7.4

Re: [Qemu-block] [PATCH v11 01/19] block: Add bdrv_drain_all_{begin, end}()

2016-10-19 Thread Kevin Wolf 
Am 14.10.2016 um 15:08 hat Alberto Garcia geschrieben:
> bdrv_drain_all() doesn't allow the caller to do anything after all
> pending requests have been completed but before block jobs are
> resumed.
> 
> This patch splits bdrv_drain_all() into _begin() and _end() for that
> purpose. It also adds aio_{disable,enable}_external() calls to disable
> external clients in the meantime.
> 
> Signed-off-by: Alberto Garcia 

This looks okay as a first step, possibly enough for this series (we'll
have to review this carefully), but it leaves us with a rather limited
version of bdrv_drain_all_begin/end that excludes many useful cases. One
of them is that John wants to use it around QMP transactions.

Specifically, you can't add a new BDS or a new block job in a drain_all
section because then bdrv_drain_all_end() would try to unpause the new
thing even though it has never been paused. Depending on what else we
did with it, this will either corrupt the pause counters or just
directly fail an assertion.

My first thoughts were about how to let an unpause succeed without a
previous pause for these objects, but actually I think this isn't what
we should do. We rather want to actually do the pause instead because
even new BDSes and block jobs should probably start in a quiesced state
when created inside a drain_all section.

This is somewhat similar to attaching a BlockBackend to a drained BDS.
We already take care to immediately quiesce the BB in this case (even
though this isn't very effective because the BB doesn't propagate it
correctly to its users yet...)

Thoughts?
(Paolo, I'm looking at you.)

Kevin

Re: [Qemu-block] [Qemu-devel] [PATCH v14 15/21] qom: support non-scalar properties with -object

2016-10-19 Thread Markus Armbruster 
"Daniel P. Berrange"  writes:

> The current -object command line syntax only allows for
> creation of objects with scalar properties, or a list
> with a fixed scalar element type. Objects which have
> properties that are represented as structs in the QAPI
> schema cannot be created using -object.
>
> This is a design limitation of the way the OptsVisitor
> is written. It simply iterates over the QemuOpts values
> as a flat list. The support for lists is enabled by
> allowing the same key to be repeated in the opts string.
>
> The QObjectInputVisitor now has functionality that allows
> it to replace OptsVisitor, maintaining full backwards
> compatibility for previous CLI syntax, while also allowing
> use of new syntax for structs.
>
> Thus -object can now support non-scalar properties,
> for example the QMP object:
>
>   {
> "execute": "object-add",
> "arguments": {
>   "qom-type": "demo",
>   "id": "demo0",
>   "parameters": {
> "foo": [
>   { "bar": "one", "wizz": "1" },
>   { "bar": "two", "wizz": "2" }
> ]
>   }
> }
>   }
>
> Would be creatable via the CLI now using
>
> $QEMU \
>   -object demo,id=demo0,\
>   foo.0.bar=one,foo.0.wizz=1,\
>   foo.1.bar=two,foo.1.wizz=2
>
> Notice that this syntax is intentionally compatible
> with that currently used by block drivers. NB the
> indentation seen here after line continuations should
> not be used in reality, it is just for clarity in this
> commit message.
>
> Signed-off-by: Daniel P. Berrange 
> ---
>  qapi/qobject-input-visitor.c |   2 +-
>  qom/object_interfaces.c  |  37 -
>  tests/check-qom-proplist.c   | 367 
> ++-
>  3 files changed, 397 insertions(+), 9 deletions(-)
>
> diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
> index 5a3872c..f1030d5 100644
> --- a/qapi/qobject-input-visitor.c
> +++ b/qapi/qobject-input-visitor.c
> @@ -204,7 +204,7 @@ static void qobject_input_start_struct(Visitor *v, const 
> char *name, void **obj,
>  *obj = NULL;
>  }
>  
> -if (!qobj && (qiv->struct_level < qiv->autocreate_struct_levels)) {
> +if (!qobj && (qiv->struct_level <= qiv->autocreate_struct_levels)) {
>  /* Create a new dict that contains all the currently
>   * unvisited items */
>  if (tos) {

Uh, can you explain this hunk?  The < comes from PATCH 10.

> diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
> index fdc406b..88a1e88 100644
> --- a/qom/object_interfaces.c
> +++ b/qom/object_interfaces.c
> @@ -4,7 +4,8 @@
>  #include "qemu/module.h"
>  #include "qapi-visit.h"
>  #include "qapi/qobject-output-visitor.h"
> -#include "qapi/opts-visitor.h"
> +#include "qapi/qobject-input-visitor.h"
> +#include "qemu/option.h"
>  
>  void user_creatable_complete(Object *obj, Error **errp)
>  {
> @@ -63,12 +64,16 @@ Object *user_creatable_add(const QDict *qdict,
>  if (local_err) {
>  goto out_visit;
>  }
> -visit_check_struct(v, _err);
> +
> +obj = user_creatable_add_type(type, id, pdict, v, _err);
>  if (local_err) {
>  goto out_visit;
>  }
>  
> -obj = user_creatable_add_type(type, id, pdict, v, _err);
> +visit_check_struct(v, _err);
> +if (local_err) {
> +goto out_visit;
> +}
>  
>  out_visit:
>  visit_end_struct(v, NULL);

Can you explain why you swap the order of visit_check_struct() and
user_creatable_add_type()?  It smells like a bug fix to me...

Odd: opts_check_struct() does nothing unless depth == 0.  But depth is
at least 1 between opts_start_struct() and opts_end_struct().  Bug?

> @@ -114,7 +119,7 @@ Object *user_creatable_add_type(const char *type, const 
> char *id,
>  
>  assert(qdict);
>  obj = object_new(type);
> -visit_start_struct(v, NULL, NULL, 0, _err);
> +visit_start_struct(v, "props", NULL, 0, _err);
>  if (local_err) {
>  goto out;
>  }

Why this hunk?

> @@ -158,14 +163,32 @@ Object *user_creatable_add_opts(QemuOpts *opts, Error 
> **errp)
>  {
>  Visitor *v;
>  QDict *pdict;
> +QObject *pobj;
>  Object *obj = NULL;
>  
> -v = opts_visitor_new(opts);
> -pdict = qemu_opts_to_qdict(opts, NULL, QEMU_OPTS_REPEAT_POLICY_LAST,
> +pdict = qemu_opts_to_qdict(opts, NULL,
> +   QEMU_OPTS_REPEAT_POLICY_ALL,
> _abort);
>  
> -obj = user_creatable_add(pdict, v, errp);
> +pobj = qdict_crumple(pdict, true, errp);
> +if (!pobj) {
> +goto cleanup;
> +}
> +/*
> + * We need autocreate_list=true + permit_int_ranges=true
> + * in order to maintain compat with OptsVisitor creation
> + * of the 'numa' object which had an int16List property.

I can't find a "numa" object in the object-add sense, only a NumaOptions
QAPI object created by -numa.  Is that what you mean?

> + *
> +

Re: [Qemu-block] [PATCH] qcow2: Optimize L2 table cache size based on image and cluster sizes

2016-10-19 Thread Frank Myhr 
On 10/18/2016 11:25 AM, Alberto Garcia wrote:
> On Fri 07 Oct 2016 03:58:29 PM CEST, Ed Swierk wrote:
>> Same here, using libvirt. l2-cache-size=max would be ideal. Or if
>> there were a cache-coverage-size option that takes an absolute number,
>> libvirt could set it to the image size.
> 
> I can see the benefit of both approaches: setting the disk size covered
> by the cache or setting a percentage, I personally like a bit more the
> former but it wouldn't provide a way to say "create the largest cache
> needed for this disk".

I prefer percentage (or just 'max') because it eliminates a source of
performance problems when increasing the size of a VM's disk(s). Take the case
of a VM doing heavy random I/O on a 10GB disk. Admin1 initially sets
l2-cache-size=1.25M or l2-cache-coverage-size=10G. At some point the disk fills
up as they always do, and admin2 (or admin1 on a bad day) increases its size to
20GB: problem solved. Except admin2 forgot to also double l2-cache-size or
l2-cache-coverage-size. So performance with larger drive takes a nosedive (~*6X*
worse in your worst-case benchmark) until some admin figures it out.

https://blogs.igalia.com/berto/2015/12/17/improving-disk-io-performance-in-qemu-2-5-with-the-qcow2-l2-cache/

Sure, admins shouldn't make this mistake. But guaranteed they (cough, I) will.
Using percentage makes this mistake impossible.


> Would 'l2-cache-coverage-size' work for everyone? It would simply take
> the disk size (16G, 1T, etc) and it would conflict with l2-cache-size.

The above said, l2-cache-coverage-size would work for me if libvirt could set
it. Or if it defaulted to entire image size, eliminating the need for libvirt to
set it, and making impossible the admin mistake above.


> Do we need something similar for the refcount cache?

I don't know. AFAICT the current default refcount behavior is fine. But might be
good to keep l2 and refcount options symmetrical.

Frank

Re: [Qemu-block] [PATCH v4] raw_bsd: add offset and size options

2016-10-19 Thread Kevin Wolf 
Am 19.10.2016 um 17:38 hat Eric Blake geschrieben:
> On 10/19/2016 07:27 AM, Tomáš Golembiovský wrote:
> > Added two new options 'offset' and 'size'. This makes it possible to use
> > only part of the file as a device. This can be used e.g. to limit the
> > access only to single partition in a disk image or use a disk inside a
> > tar archive (like OVA).
> > 
> > When 'size' is specified we do our best to honour it.
> > 
> > Signed-off-by: Tomáš Golembiovský 
> > ---
> >  block/raw_bsd.c  | 168 
> > ++-
> >  qapi/block-core.json |  16 -
> >  2 files changed, 180 insertions(+), 4 deletions(-)
> > 
> > diff --git a/block/raw_bsd.c b/block/raw_bsd.c
> > index 588d408..25b5ba8 100644
> > --- a/block/raw_bsd.c
> > +++ b/block/raw_bsd.c
> > @@ -31,6 +31,30 @@
> >  #include "qapi/error.h"
> >  #include "qemu/option.h"
> >  
> > +typedef struct BDRVRawState {
> > +uint64_t offset;
> > +uint64_t size;
> > +bool has_size;
> > +} BDRVRawState;
> 
> This seems like it is duplicating a struct that QAPI should be able to
> give us for free, if we would just use it.

Possibly it has the same fields as BlockdevOptionsRaw, but I would use
QAPI structs only for configuring a block device and not for its state
after opening it (i.e. bs->opaque).

So I agree with the explicit definition here.

Kevin


pgpfo2HKwnNCl.pgp
Description: PGP signature

Re: [Qemu-block] [PATCH v4] raw_bsd: add offset and size options

2016-10-19 Thread Eric Blake 
On 10/19/2016 07:27 AM, Tomáš Golembiovský wrote:
> Added two new options 'offset' and 'size'. This makes it possible to use
> only part of the file as a device. This can be used e.g. to limit the
> access only to single partition in a disk image or use a disk inside a
> tar archive (like OVA).
> 
> When 'size' is specified we do our best to honour it.
> 
> Signed-off-by: Tomáš Golembiovský 
> ---
>  block/raw_bsd.c  | 168 
> ++-
>  qapi/block-core.json |  16 -
>  2 files changed, 180 insertions(+), 4 deletions(-)
> 
> diff --git a/block/raw_bsd.c b/block/raw_bsd.c
> index 588d408..25b5ba8 100644
> --- a/block/raw_bsd.c
> +++ b/block/raw_bsd.c
> @@ -31,6 +31,30 @@
>  #include "qapi/error.h"
>  #include "qemu/option.h"
>  
> +typedef struct BDRVRawState {
> +uint64_t offset;
> +uint64_t size;
> +bool has_size;
> +} BDRVRawState;

This seems like it is duplicating a struct that QAPI should be able to
give us for free, if we would just use it.


> +/* Check size and offset */
> +if (real_size < s->offset || (real_size - s->offset) < s->size) {
> +error_setg(errp, "The sum of offset (%"PRIu64") and size "
> +"(%"PRIu64") has to be smaller or equal to the "
> +" actual size of the containing file (%"PRId64").",
> +s->offset, s->size, real_size);

No trailing '.' in error_setg() error messages.  Also, most uses of
PRIu64 and PRId64 are separated by spaces from the rest of the "",
rather than adjacent.

> +ret = -EINVAL;
> +goto fail;
> +}
> +
> +/* Make sure size is multiple of BDRV_SECTOR_SIZE to prevent rounding
> + * up and leaking out of the specified area. */
> +if (s->size != QEMU_ALIGN_DOWN(s->size, BDRV_SECTOR_SIZE)) {

Better would be:

if (!QEMU_IS_ALIGNED(s->size, BDRV_SECTOR_SIZE)) {

> +s->size = QEMU_ALIGN_DOWN(s->size, BDRV_SECTOR_SIZE);

Dead assignment, since...

> +error_setg(errp, "Specified size is not multiple of %llu!",
> +BDRV_SECTOR_SIZE);
> +ret = -EINVAL;
> +goto fail;

...you are failing anyways.

> +}
> +
> +ret = 0;
> +
> +fail:

Naming a label fail: when it is intended to be reached by fallthrough on
the success path is annoying.  end: might be a better name.

> +
> +qemu_opts_del(opts);
> +
> +return ret;
> +}
> +
> @@ -117,8 +239,10 @@ static int64_t coroutine_fn 
> raw_co_get_block_status(BlockDriverState *bs,
>  int nb_sectors, int *pnum,
>  BlockDriverState **file)
>  {
> +BDRVRawState *s = bs->opaque;
>  *pnum = nb_sectors;
>  *file = bs->file->bs;
> +sector_num += s->offset / BDRV_SECTOR_SIZE;

Not your fault (nor necessarily for you to fix), but we should really
switch block status code to be byte-based.

>  return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID | BDRV_BLOCK_DATA |
> (sector_num << BDRV_SECTOR_BITS);
>  }
> @@ -138,7 +262,28 @@ static int coroutine_fn raw_co_pdiscard(BlockDriverState 
> *bs,
>  
>  static int64_t raw_getlength(BlockDriverState *bs)
>  {
> -return bdrv_getlength(bs->file->bs);
> +int64_t len;
> +BDRVRawState *s = bs->opaque;
> +
> +/* Update size. It should not change unles the file was externaly

s/unles/unless/
s/externaly/externally/

If the file is being externally modified, the user deserves broken behavior.

> +++ b/qapi/block-core.json
> @@ -2224,6 +2224,20 @@
>'data': { 'filename': 'str' } }
>  
>  ##
> +# @BlockdevOptionsRaw
> +#
> +# Driver specific block device options for the raw driver.
> +#
> +# @offset:  #optional position where the block device starts
> +# @size:#optional the assumed size of the device
> +#
> +# Since: 2.8
> +##
> +{ 'struct': 'BlockdevOptionsRaw',
> +  'base': 'BlockdevOptionsGenericFormat',
> +  'data': { 'offset': 'int', 'size': 'int' } }

Umm, if you want these to actually be optional, you have to spell them
'*offset' and '*size'.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-block] [Qemu-devel] [PATCH v3 4/4] qemu-iotests: Test creating floppy drives

2016-10-19 Thread Kevin Wolf 
Am 19.10.2016 um 15:47 hat John Snow geschrieben:
> 
> 
> On 10/19/2016 03:37 AM, Kevin Wolf wrote:
> >Am 18.10.2016 um 21:53 hat Eric Blake geschrieben:
> >>On 10/18/2016 02:45 PM, John Snow wrote:
> >>>
> >>>
> >>>On 10/18/2016 06:22 AM, Kevin Wolf wrote:
> This tests the different supported methods to create floppy drives and
> how they interact.
> 
> >>
> +function check_floppy_qtree()
> +{
> +echo
> +echo Testing: "$@" | _filter_testdir
> +
> +# QEMU_OPTIONS contains -nodefaults, we don't want that here
> because the
> +# defaults are part of what should be checked here
> +echo "info qtree" |
> +QEMU_OPTIONS="" do_run_qemu "$@" | _filter_win32 |
> +grep -zo '[[:cntrl:]]\( *\)dev: isa-fdc.*\([[:cntrl:]]\1
> .*\)*[[:cntrl:]] *dev:'
> >>>
> >>>This grep invocation doesn't appear to actually terminate with the '-z'
> >>>option here. Not sure why, I haven't looked into the bash framework
> >>>much, hopefully it's not too hard for you to reproduce and correct.
> >
> >No, obviously I can't reproduce, otherwise I wouldn't have written the
> >test case like this. It passes just fine for me on RHEL 7.
> >
> 
> Wasn't sure if it was something that popped up more recently or not.
> Obviously it worked at some point.
> 
> I'm on Fedora 24, using bash 4.3.42-7.fc24 and grep 2.25-1.fc24.
> 
> >Just to clarify, it's grep that doesn't terminate, or qemu? Also, what
> >do you mean by the "bash framework"?
> >
> 
> It seems like it's the grep invocation; I don't see any QEMU
> processes in `ps`, the only thing I can find is the grep invocation.
> (Why would grep hang if qemu has exited?)

I haven't seen it myself, but from your description this sounds more
like a grep bug to me, honestly.

> By the 'bash framework' I meant the shell related infrastructure for
> iotests. I'm more familiar with the python parts.

If you mean the functions for spwaning a qemu instance in the background
and then controlling it from the script, this isn't even using it. I'm
just piping some static data into a monitor on stdio and waiting for the
qemu process to exit.

Essentially just something like this:

echo -e "info qtree\nquit" |
qemu-system-x86_64 -monitor stdio |
grep ...

Kevin

Re: [Qemu-block] [Qemu-devel] [PATCH v3 4/4] qemu-iotests: Test creating floppy drives

2016-10-19 Thread John Snow 



On 10/19/2016 03:37 AM, Kevin Wolf wrote:

Am 18.10.2016 um 21:53 hat Eric Blake geschrieben:

On 10/18/2016 02:45 PM, John Snow wrote:



On 10/18/2016 06:22 AM, Kevin Wolf wrote:

This tests the different supported methods to create floppy drives and
how they interact.




+function check_floppy_qtree()
+{
+echo
+echo Testing: "$@" | _filter_testdir
+
+# QEMU_OPTIONS contains -nodefaults, we don't want that here
because the
+# defaults are part of what should be checked here
+echo "info qtree" |
+QEMU_OPTIONS="" do_run_qemu "$@" | _filter_win32 |
+grep -zo '[[:cntrl:]]\( *\)dev: isa-fdc.*\([[:cntrl:]]\1
.*\)*[[:cntrl:]] *dev:'


This grep invocation doesn't appear to actually terminate with the '-z'
option here. Not sure why, I haven't looked into the bash framework
much, hopefully it's not too hard for you to reproduce and correct.


No, obviously I can't reproduce, otherwise I wouldn't have written the
test case like this. It passes just fine for me on RHEL 7.



Wasn't sure if it was something that popped up more recently or not. 
Obviously it worked at some point.


I'm on Fedora 24, using bash 4.3.42-7.fc24 and grep 2.25-1.fc24.


Just to clarify, it's grep that doesn't terminate, or qemu? Also, what
do you mean by the "bash framework"?



It seems like it's the grep invocation; I don't see any QEMU processes 
in `ps`, the only thing I can find is the grep invocation. (Why would 
grep hang if qemu has exited?)


By the 'bash framework' I meant the shell related infrastructure for 
iotests. I'm more familiar with the python parts.



Is 'grep -z' even portable to BSD, or is it just a GNU extension?  Would
it be better to run the output through tr to convert things to a saner
subset of characters before then grepping a text file?


Is qemu-iotests even supposed to run on BSD? All our test cases specify
"_supported_os Linux". (And I don't think this means Linux kernel with
BSD userland :-))

Anyway, the tr thing you mean would be translating all newlines into
something else (which is hopefully otherwise unused), then grep, then
translate back?

What this line is supposed to do (if it wasn't obvious) is extracting
the full information for a single device from 'info qtree'. I don't
really mind how it's done, but multiline operation seems to be something
that isn't trivial with most tools... I think I've done multiline sed
before, so maybe that would be another option.

Kevin

Re: [Qemu-block] [Qemu-devel] [PATCH v14 13/21] qdict: allow qdict_crumple to accept compound types as values

2016-10-19 Thread Eric Blake 
On 10/18/2016 10:35 AM, Markus Armbruster wrote:

 But even if I realised that QemuOpts support this syntax, I think we
 would still have to use the dotted syntax because it's explicit about
 the index and we need that because the list can contains dicts.

 Compare this:

 driver=quorum,
 child.0.driver=file,child.0.filename=disk1.img,
 child.1.driver=host_device,child.1.filename=/dev/sdb,
 child.2.driver=nbd,child.2.host=localhost

 And this:

 driver=quorum,
 child.driver=file,child.filename=disk1.img,
 child.driver=host_device,child.filename=/dev/sdb,
 child.driver=nbd,child.host=localhost
>>>
>>> Aside: both are about equally illegible, to be honest.

Furthermore, both of these currently require really long command lines.
Overnight, I was thinking about whether it would be worth improving
QemuOpts to ignore whitespace after (unquoted) commas, so you can do:

-drive 'driver=quorum,
child.0.driver=file,child.0.filename=disk1.img,
child.1.driver=host_device, child.1.filename=/dev/sdb,
child.2.driver=nbd, child.2.host=localhost
'

I think this one should be just fine - we don't have ANY keys that start
with leading whitespace, so ignoring space will still let us parse out
key names, but allow for much more legibility in the command lines.  And
this is true whether we use dotted notation...

>>> If you'd rather invent syntax closer to QemuOpts than reuse JSON, you
>>> could try
>>>
>>> driver=quorum,
>>> child=[{ driver=file, filename=disk1.img },
>>>{ driver=host_device, filename=/dev/sdb },
>>>{ driver=nbd, host=localhost } ]
>>
>> This looks a lot more agreeable as something that I have to type on the
>> command line.

...or new syntax (where of course the new syntax has already
demonstrated that we want to support strategic ignoring of whitespace).

So I'll probably propose a patch along those lines soon, as it seems
like an orthogonal improvement.

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



signature.asc
Description: OpenPGP digital signature

[Qemu-block] [PATCH v4] Add 'offset' and 'size' options

2016-10-19 Thread Tomáš Golembiovský 
v3 -> v4:
- fix stupid compilation error and formatting issue

v2 -> v3:
- changed overflow check to make it clearer
- produce error instead of warning when size is not multiple of sector
  size

v1 -> v2:
- options were moved from 'file' driver into 'raw' driver as suggested
- added support for writing, reopen and truncate when possible

Tomáš Golembiovský (1):
  raw_bsd: add offset and size options

 block/raw_bsd.c  | 168 ++-
 qapi/block-core.json |  16 -
 2 files changed, 180 insertions(+), 4 deletions(-)

-- 
2.10.0

[Qemu-block] [PATCH 2/2] qapi: allow blockdev-add for NFS

2016-10-19 Thread Ashijeet Acharya 
Introduce new object 'BlockdevOptionsNFS' in qapi/block-core.json to
support blockdev-add for NFS network protocol driver. Also make a new
struct NFSServer to support tcp connection.

Signed-off-by: Ashijeet Acharya 
---
 qapi/block-core.json | 56 
 1 file changed, 52 insertions(+), 4 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 9d797b8..3ab028d 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1714,9 +1714,9 @@
 { 'enum': 'BlockdevDriver',
   'data': [ 'archipelago', 'blkdebug', 'blkverify', 'bochs', 'cloop',
 'dmg', 'file', 'ftp', 'ftps', 'gluster', 'host_cdrom',
-'host_device', 'http', 'https', 'luks', 'null-aio', 'null-co',
-'parallels', 'qcow', 'qcow2', 'qed', 'quorum', 'raw',
-   'replication', 'tftp', 'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat' ] }
+'host_device', 'http', 'https', 'luks', 'nfs', 'null-aio',
+'null-co', 'parallels', 'qcow', 'qcow2', 'qed', 'quorum', 'raw',
+'replication', 'tftp', 'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat' ] }
 
 ##
 # @BlockdevOptionsFile
@@ -2212,6 +2212,54 @@
 '*top-id': 'str' } }
 
 ##
+# @NFSServer
+#
+# Captures the address of the socket
+#
+# @type:transport type used for NFS (only TCP supported)
+#
+# @host:host part of the address
+#
+# Since 2.8
+##
+{ 'struct': 'NFSServer',
+  'data': { 'type': 'str',
+'host': 'str' } }
+
+##
+# @BlockdevOptionsNfs
+#
+# Driver specific block device option for NFS
+#
+# @server:host address
+#
+# @path:  path of the image on the host
+#
+# @uid:   #optional UID value to use when talking to the server
+#
+# @gid:   #optional GID value to use when talking to the server
+#
+# @tcp-syncnt:#optional number of SYNs during the session establishment
+#
+# @readahead: #optional set the readahead size in bytes
+#
+# @pagecache: #optional set the pagecache size in bytes
+#
+# @debug: #optional set the NFS debug level (max 2)
+#
+# Since 2.8
+##
+{ 'struct': 'BlockdevOptionsNfs',
+  'data': { 'server': 'NFSServer',
+'path': 'str',
+'*uid': 'int',
+'*gid': 'int',
+'*tcp-syncnt': 'int',
+'*readahead': 'int',
+'*pagecache': 'int',
+'*debug': 'int' } }
+
+##
 # @BlockdevOptionsCurl
 #
 # Driver specific block device options for the curl backend.
@@ -2269,7 +2317,7 @@
 # TODO iscsi: Wait for structured options
   'luks':   'BlockdevOptionsLUKS',
 # TODO nbd: Should take InetSocketAddress for 'host'?
-# TODO nfs: Wait for structured options
+  'nfs':'BlockdevOptionsNfs',
   'null-aio':   'BlockdevOptionsNull',
   'null-co':'BlockdevOptionsNull',
   'parallels':  'BlockdevOptionsGenericFormat',
-- 
2.6.2

[Qemu-block] [PATCH 1/2] block/nfs: Introduce runtime_opts in NFS

2016-10-19 Thread Ashijeet Acharya 
Make NFS block driver use various fine grained runtime_opts.
Set .bdrv_parse_filename() to nfs_parse_filename() and introduce two
new functions nfs_parse_filename() and nfs_parse_uri() to help parsing
the URI. This will help us to prepare the NFS for blockdev-add.

Signed-off-by: Ashijeet Acharya 
---
 block/nfs.c | 360 +++-
 1 file changed, 261 insertions(+), 99 deletions(-)

diff --git a/block/nfs.c b/block/nfs.c
index 8602a44..5eb909e 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -35,8 +35,12 @@
 #include "qemu/uri.h"
 #include "qemu/cutils.h"
 #include "sysemu/sysemu.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/qmp/qint.h"
+#include "qapi/qmp/qstring.h"
 #include 
 
+
 #define QEMU_NFS_MAX_READAHEAD_SIZE 1048576
 #define QEMU_NFS_MAX_PAGECACHE_SIZE (8388608 / NFS_BLKSIZE)
 #define QEMU_NFS_MAX_DEBUG_LEVEL 2
@@ -61,6 +65,127 @@ typedef struct NFSRPC {
 NFSClient *client;
 } NFSRPC;
 
+static int nfs_parse_uri(const char *filename, QDict *options, Error **errp)
+{
+URI *uri = NULL;
+QueryParams *qp = NULL;
+int ret = 0, i;
+const char *p;
+
+uri = uri_parse(filename);
+if (!uri) {
+error_setg(errp, "Invalid URI specified");
+ret = -EINVAL;
+goto out;
+}
+if (strcmp(uri->scheme, "nfs") != 0) {
+error_setg(errp, "URI scheme must be 'nfs'");
+ret = -EINVAL;
+goto out;
+}
+
+if (!uri->server || strcmp(uri->server, "") == 0) {
+error_setg(errp, "missing hostname in URI");
+ret = -EINVAL;
+goto out;
+}
+
+if (!uri->path || strcmp(uri->path, "") == 0) {
+error_setg(errp, "missing file path in URI");
+ret = -EINVAL;
+goto out;
+}
+
+p = uri->path ? uri->path : "/";
+p += strspn(p, "/");
+if (p[0]) {
+qdict_put(options, "export", qstring_from_str(p));
+}
+
+qp = query_params_parse(uri->query);
+if (!qp) {
+error_setg(errp, "could not parse query parameters");
+ret = -EINVAL;
+goto out;
+}
+
+qdict_put(options, "host", qstring_from_str(uri->server));
+
+qdict_put(options, "path", qstring_from_str(uri->path));
+
+for (i = 0; i < qp->n; i++) {
+if (!qp->p[i].value) {
+error_setg(errp, "Value for NFS parameter expected: %s",
+   qp->p[i].name);
+goto out;
+}
+if (parse_uint_full(qp->p[i].value, NULL, 0)) {
+error_setg(errp, "Illegal value for NFS parameter: %s",
+   qp->p[i].name);
+goto out;
+}
+if (!strcmp(qp->p[i].name, "uid")) {
+qdict_put(options, "uid",
+  qstring_from_str(qp->p[i].value));
+} else if (!strcmp(qp->p[i].name, "gid")) {
+qdict_put(options, "gid",
+  qstring_from_str(qp->p[i].value));
+} else if (!strcmp(qp->p[i].name, "tcp-syncnt")) {
+qdict_put(options, "tcp-syncnt",
+  qstring_from_str(qp->p[i].value));
+} else if (!strcmp(qp->p[i].name, "readahead")) {
+qdict_put(options, "readahead",
+  qstring_from_str(qp->p[i].value));
+} else if (!strcmp(qp->p[i].name, "pagecache")) {
+qdict_put(options, "pagecache",
+  qstring_from_str(qp->p[i].value));
+} else if (!strcmp(qp->p[i].name, "debug")) {
+qdict_put(options, "debug",
+  qstring_from_str(qp->p[i].value));
+} else {
+error_setg(errp, "Unknown NFS parameter name: %s",
+   qp->p[i].name);
+goto out;
+}
+}
+out:
+if (qp) {
+query_params_free(qp);
+}
+if (uri) {
+uri_free(uri);
+}
+return ret;
+}
+
+static void nfs_parse_filename(const char *filename, QDict *options,
+   Error **errp)
+{
+int ret = 0;
+
+if (qdict_haskey(options, "host") ||
+qdict_haskey(options, "path") ||
+qdict_haskey(options, "uid") ||
+qdict_haskey(options, "gid") ||
+qdict_haskey(options, "tcp-syncnt") ||
+qdict_haskey(options, "readahead") ||
+qdict_haskey(options, "pagecache") ||
+qdict_haskey(options, "debug")) {
+error_setg(errp, "host/path/uid/gid/tcp-syncnt/readahead/pagecache"
+ "/debug and a filename may not be used at the same"
+ " time");
+return;
+}
+
+if (strstr(filename, "://")) {
+ret = nfs_parse_uri(filename, options, errp);
+if (ret < 0) {
+error_setg(errp, "No valid URL specified");
+}
+return;
+}
+}
+
 static void nfs_process_read(void *arg);
 static void nfs_process_write(void *arg);
 
@@ -228,15 +353,49 @@ static int coroutine_fn nfs_co_flush(BlockDriverState *bs)
 return task.ret;
 }
 
-/*

[Qemu-block] [PATCH 0/2] allow blockdev-add for NFS

2016-10-19 Thread Ashijeet Acharya 
This series adds blockdev-add support for NFS block driver.

Patch 1 helps to prepare NFS driver to make use of several runtime_opts
as they appear in the URI. This will make NFS to do things similar to
the way other drivers available in the block layer do.

Patch 2 helps to allow blockdev-add support for the NFS block driver
by making the NFS option available.

Ashijeet Acharya (2):
  block/nfs: Introduce runtime_opts in NFS
  qapi: allow blockdev-add for NFS

 block/nfs.c  | 360 +--
 qapi/block-core.json |  56 +++-
 2 files changed, 313 insertions(+), 103 deletions(-)

-- 
2.6.2

[Qemu-block] [PATCH v4] raw_bsd: add offset and size options

2016-10-19 Thread Tomáš Golembiovský 
Added two new options 'offset' and 'size'. This makes it possible to use
only part of the file as a device. This can be used e.g. to limit the
access only to single partition in a disk image or use a disk inside a
tar archive (like OVA).

When 'size' is specified we do our best to honour it.

Signed-off-by: Tomáš Golembiovský 
---
 block/raw_bsd.c  | 168 ++-
 qapi/block-core.json |  16 -
 2 files changed, 180 insertions(+), 4 deletions(-)

diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index 588d408..25b5ba8 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -31,6 +31,30 @@
 #include "qapi/error.h"
 #include "qemu/option.h"
 
+typedef struct BDRVRawState {
+uint64_t offset;
+uint64_t size;
+bool has_size;
+} BDRVRawState;
+
+static QemuOptsList raw_runtime_opts = {
+.name = "raw",
+.head = QTAILQ_HEAD_INITIALIZER(raw_runtime_opts.head),
+.desc = {
+{
+.name = "offset",
+.type = QEMU_OPT_SIZE,
+.help = "offset in the disk where the image starts",
+},
+{
+.name = "size",
+.type = QEMU_OPT_SIZE,
+.help = "virtual disk size",
+},
+{ /* end of list */ }
+},
+};
+
 static QemuOptsList raw_create_opts = {
 .name = "raw-create-opts",
 .head = QTAILQ_HEAD_INITIALIZER(raw_create_opts.head),
@@ -44,17 +68,106 @@ static QemuOptsList raw_create_opts = {
 }
 };
 
+static int raw_read_options(QDict *options, BlockDriverState *bs,
+BDRVRawState *s, Error **errp)
+{
+Error *local_err = NULL;
+QemuOpts *opts = NULL;
+int64_t real_size = 0;
+int ret;
+
+real_size = bdrv_getlength(bs->file->bs);
+if (real_size < 0) {
+error_setg_errno(errp, -real_size, "Could not get image size");
+return real_size;
+}
+
+opts = qemu_opts_create(_runtime_opts, NULL, 0, _abort);
+qemu_opts_absorb_qdict(opts, options, _err);
+if (local_err) {
+error_propagate(errp, local_err);
+ret = -EINVAL;
+goto fail;
+}
+
+s->offset = qemu_opt_get_size(opts, "offset", 0);
+if (qemu_opt_find(opts, "size") != NULL) {
+s->size = qemu_opt_get_size(opts, "size", 0);
+s->has_size = true;
+} else {
+s->has_size = false;
+s->size = real_size;
+}
+
+/* Check size and offset */
+if (real_size < s->offset || (real_size - s->offset) < s->size) {
+error_setg(errp, "The sum of offset (%"PRIu64") and size "
+"(%"PRIu64") has to be smaller or equal to the "
+" actual size of the containing file (%"PRId64").",
+s->offset, s->size, real_size);
+ret = -EINVAL;
+goto fail;
+}
+
+/* Make sure size is multiple of BDRV_SECTOR_SIZE to prevent rounding
+ * up and leaking out of the specified area. */
+if (s->size != QEMU_ALIGN_DOWN(s->size, BDRV_SECTOR_SIZE)) {
+s->size = QEMU_ALIGN_DOWN(s->size, BDRV_SECTOR_SIZE);
+error_setg(errp, "Specified size is not multiple of %llu!",
+BDRV_SECTOR_SIZE);
+ret = -EINVAL;
+goto fail;
+}
+
+ret = 0;
+
+fail:
+
+qemu_opts_del(opts);
+
+return ret;
+}
+
 static int raw_reopen_prepare(BDRVReopenState *reopen_state,
   BlockReopenQueue *queue, Error **errp)
 {
-return 0;
+assert(reopen_state != NULL);
+assert(reopen_state->bs != NULL);
+
+reopen_state->opaque = g_new0(BDRVRawState, 1);
+
+return raw_read_options(
+reopen_state->options,
+reopen_state->bs,
+reopen_state->opaque,
+errp);
+}
+
+static void raw_reopen_commit(BDRVReopenState *state)
+{
+BDRVRawState *new_s = state->opaque;
+BDRVRawState *s = state->bs->opaque;
+
+memcpy(s, new_s, sizeof(BDRVRawState));
+
+g_free(state->opaque);
+state->opaque = NULL;
+}
+
+static void raw_reopen_abort(BDRVReopenState *state)
+{
+g_free(state->opaque);
+state->opaque = NULL;
 }
 
 static int coroutine_fn raw_co_preadv(BlockDriverState *bs, uint64_t offset,
   uint64_t bytes, QEMUIOVector *qiov,
   int flags)
 {
+BDRVRawState *s = bs->opaque;
+
 BLKDBG_EVENT(bs->file, BLKDBG_READ_AIO);
+offset += s->offset;
 return bdrv_co_preadv(bs->file, offset, bytes, qiov, flags);
 }
 
@@ -62,11 +175,18 @@ static int coroutine_fn raw_co_pwritev(BlockDriverState 
*bs, uint64_t offset,
uint64_t bytes, QEMUIOVector *qiov,
int flags)
 {
+BDRVRawState *s = bs->opaque;
 void *buf = NULL;
 BlockDriver *drv;
 QEMUIOVector local_qiov;
 int ret;
 
+if (s->has_size && (offset > s->size || bytes > (s->size - offset))) {
+/* There's not enough space for the data. Don't write anything and just
+ * fail to prevent

Re: [Qemu-block] [Qemu-devel] [PATCH v3] raw_bsd: add offset and size options

2016-10-19 Thread no-reply 
Hi,

Your series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 
3b5b149eb55b2d31551f6f8b6b2a1349a9452526.1476876311.git.tgole...@redhat.com
Subject: [Qemu-devel] [PATCH v3] raw_bsd: add offset and size options

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git show --no-patch --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
aa8070a raw_bsd: add offset and size options

=== OUTPUT BEGIN ===
Checking PATCH 1/1: raw_bsd: add offset and size options...
ERROR: externs should be avoided in .c files
#103: FILE: block/raw_bsd.c:116:
+error_setg here(errp, "Specified size is not multiple of %llu!",

ERROR: space prohibited before that close parenthesis ')'
#241: FILE: block/raw_bsd.c:312:
+if (INT64_MAX - offset < s->offset ) {

total: 2 errors, 0 warnings, 275 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@freelists.org

Re: [Qemu-block] [Qemu-devel] [PATCH v3] raw_bsd: add offset and size options

2016-10-19 Thread no-reply 
Hi,

Your series failed automatic build test. Please find the testing commands and
their output below. If you have docker installed, you can probably reproduce it
locally.

Type: series
Message-id: 
3b5b149eb55b2d31551f6f8b6b2a1349a9452526.1476876311.git.tgole...@redhat.com
Subject: [Qemu-devel] [PATCH v3] raw_bsd: add offset and size options

=== TEST SCRIPT BEGIN ===
#!/bin/bash
set -e
git submodule update --init dtc
# Let docker tests dump environment info
export SHOW_ENV=1
export J=16
make docker-test-quick@centos6
make docker-test-mingw@fedora
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]  
patchew/1476100421-28772-1-git-send-email-pbonz...@redhat.com -> 
patchew/1476100421-28772-1-git-send-email-pbonz...@redhat.com
 * [new tag] 
patchew/3b5b149eb55b2d31551f6f8b6b2a1349a9452526.1476876311.git.tgole...@redhat.com
 -> 
patchew/3b5b149eb55b2d31551f6f8b6b2a1349a9452526.1476876311.git.tgole...@redhat.com
Switched to a new branch 'test'
aa8070a raw_bsd: add offset and size options

=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into 'dtc'...
Submodule path 'dtc': checked out '65cc4d2748a2c2e6f27f1cf39e07a5dbabd80ebf'
  BUILD   centos6
make[1]: Entering directory '/var/tmp/patchew-tester-tmp-nc7m_jq4/src'
  ARCHIVE qemu.tgz
  ARCHIVE dtc.tgz
  COPYRUNNER
RUN test-quick in qemu:centos6 
Packages installed:
SDL-devel-1.2.14-7.el6_7.1.x86_64
ccache-3.1.6-2.el6.x86_64
epel-release-6-8.noarch
gcc-4.4.7-17.el6.x86_64
git-1.7.1-4.el6_7.1.x86_64
glib2-devel-2.28.8-5.el6.x86_64
libfdt-devel-1.4.0-1.el6.x86_64
make-3.81-23.el6.x86_64
package g++ is not installed
pixman-devel-0.32.8-1.el6.x86_64
tar-1.23-15.el6_8.x86_64
zlib-devel-1.2.3-29.el6.x86_64

Environment variables:
PACKAGES=libfdt-devel ccache tar git make gcc g++ zlib-devel 
glib2-devel SDL-devel pixman-devel epel-release
HOSTNAME=061c1f1ee2d9
TERM=xterm
MAKEFLAGS= -j16
HISTSIZE=1000
J=16
USER=root
CCACHE_DIR=/var/tmp/ccache
EXTRA_CONFIGURE_OPTS=
V=
SHOW_ENV=1
MAIL=/var/spool/mail/root
PATH=/usr/lib/ccache:/usr/lib64/ccache:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
PWD=/
LANG=en_US.UTF-8
TARGET_LIST=
HISTCONTROL=ignoredups
SHLVL=1
HOME=/root
TEST_DIR=/tmp/qemu-test
LOGNAME=root
LESSOPEN=||/usr/bin/lesspipe.sh %s
FEATURES= dtc
DEBUG=
G_BROKEN_FILENAMES=1
CCACHE_HASHDIR=
_=/usr/bin/env

Configure options:
--enable-werror --target-list=x86_64-softmmu,aarch64-softmmu 
--prefix=/var/tmp/qemu-build/install
No C++ compiler available; disabling C++ specific optional code
Install prefix/var/tmp/qemu-build/install
BIOS directory/var/tmp/qemu-build/install/share/qemu
binary directory  /var/tmp/qemu-build/install/bin
library directory /var/tmp/qemu-build/install/lib
module directory  /var/tmp/qemu-build/install/lib/qemu
libexec directory /var/tmp/qemu-build/install/libexec
include directory /var/tmp/qemu-build/install/include
config directory  /var/tmp/qemu-build/install/etc
local state directory   /var/tmp/qemu-build/install/var
Manual directory  /var/tmp/qemu-build/install/share/man
ELF interp prefix /usr/gnemul/qemu-%M
Source path   /tmp/qemu-test/src
C compilercc
Host C compiler   cc
C++ compiler  
Objective-C compiler cc
ARFLAGS   rv
CFLAGS-O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -g 
QEMU_CFLAGS   -I/usr/include/pixman-1-pthread -I/usr/include/glib-2.0 
-I/usr/lib64/glib-2.0/include   -fPIE -DPIE -m64 -D_GNU_SOURCE 
-D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes 
-Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes 
-fno-strict-aliasing -fno-common -fwrapv  -Wendif-labels -Wmissing-include-dirs 
-Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self 
-Wignored-qualifiers -Wold-style-declaration -Wold-style-definition 
-Wtype-limits -fstack-protector-all
LDFLAGS   -Wl,--warn-common -Wl,-z,relro -Wl,-z,now -pie -m64 -g 
make  make
install   install
pythonpython -B
smbd  /usr/sbin/smbd
module supportno
host CPU  x86_64
host big endian   no
target list   x86_64-softmmu aarch64-softmmu
tcg debug enabled no
gprof enabled no
sparse enabledno
strip binariesyes
profiler  no
static build  no
pixmansystem
SDL support   yes (1.2.14)
GTK support   no 
GTK GL supportno
VTE support   no 
TLS priority  NORMAL
GNUTLS supportno
GNUTLS rndno
libgcrypt no
libgcrypt kdf no
nettleno 
nettle kdfno
libtasn1  no
curses supportno
virgl support no
curl support  no
mingw32 support   no
Audio drivers oss
Block whitelist (rw) 
Block whitelist (ro) 
VirtFS supportno
VNC support   yes
VNC SASL support  no
VNC JPEG support  no
VNC PNG support   no
xen support   no
brlapi supportno
bluez

[Qemu-block] [PATCH v3] raw_bsd: add offset and size options

2016-10-19 Thread Tomáš Golembiovský 
Added two new options 'offset' and 'size'. This makes it possible to use
only part of the file as a device. This can be used e.g. to limit the
access only to single partition in a disk image or use a disk inside a
tar archive (like OVA).

When 'size' is specified we do our best to honour it.

Signed-off-by: Tomáš Golembiovský 
---
 block/raw_bsd.c  | 168 ++-
 qapi/block-core.json |  16 -
 2 files changed, 180 insertions(+), 4 deletions(-)

diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index 588d408..f57b6eb 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -31,6 +31,30 @@
 #include "qapi/error.h"
 #include "qemu/option.h"
 
+typedef struct BDRVRawState {
+uint64_t offset;
+uint64_t size;
+bool has_size;
+} BDRVRawState;
+
+static QemuOptsList raw_runtime_opts = {
+.name = "raw",
+.head = QTAILQ_HEAD_INITIALIZER(raw_runtime_opts.head),
+.desc = {
+{
+.name = "offset",
+.type = QEMU_OPT_SIZE,
+.help = "offset in the disk where the image starts",
+},
+{
+.name = "size",
+.type = QEMU_OPT_SIZE,
+.help = "virtual disk size",
+},
+{ /* end of list */ }
+},
+};
+
 static QemuOptsList raw_create_opts = {
 .name = "raw-create-opts",
 .head = QTAILQ_HEAD_INITIALIZER(raw_create_opts.head),
@@ -44,17 +68,106 @@ static QemuOptsList raw_create_opts = {
 }
 };
 
+static int raw_read_options(QDict *options, BlockDriverState *bs,
+BDRVRawState *s, Error **errp)
+{
+Error *local_err = NULL;
+QemuOpts *opts = NULL;
+int64_t real_size = 0;
+int ret;
+
+real_size = bdrv_getlength(bs->file->bs);
+if (real_size < 0) {
+error_setg_errno(errp, -real_size, "Could not get image size");
+return real_size;
+}
+
+opts = qemu_opts_create(_runtime_opts, NULL, 0, _abort);
+qemu_opts_absorb_qdict(opts, options, _err);
+if (local_err) {
+error_propagate(errp, local_err);
+ret = -EINVAL;
+goto fail;
+}
+
+s->offset = qemu_opt_get_size(opts, "offset", 0);
+if (qemu_opt_find(opts, "size") != NULL) {
+s->size = qemu_opt_get_size(opts, "size", 0);
+s->has_size = true;
+} else {
+s->has_size = false;
+s->size = real_size;
+}
+
+/* Check size and offset */
+if (real_size < s->offset || (real_size - s->offset) < s->size) {
+error_setg(errp, "The sum of offset (%"PRIu64") and size "
+"(%"PRIu64") has to be smaller or equal to the "
+" actual size of the containing file (%"PRId64").",
+s->offset, s->size, real_size);
+ret = -EINVAL;
+goto fail;
+}
+
+/* Make sure size is multiple of BDRV_SECTOR_SIZE to prevent rounding
+ * up and leaking out of the specified area. */
+if (s->size != QEMU_ALIGN_DOWN(s->size, BDRV_SECTOR_SIZE)) {
+s->size = QEMU_ALIGN_DOWN(s->size, BDRV_SECTOR_SIZE);
+error_setg here(errp, "Specified size is not multiple of %llu!",
+BDRV_SECTOR_SIZE);
+ret = -EINVAL;
+goto fail;
+}
+
+ret = 0;
+
+fail:
+
+qemu_opts_del(opts);
+
+return ret;
+}
+
 static int raw_reopen_prepare(BDRVReopenState *reopen_state,
   BlockReopenQueue *queue, Error **errp)
 {
-return 0;
+assert(reopen_state != NULL);
+assert(reopen_state->bs != NULL);
+
+reopen_state->opaque = g_new0(BDRVRawState, 1);
+
+return raw_read_options(
+reopen_state->options,
+reopen_state->bs,
+reopen_state->opaque,
+errp);
+}
+
+static void raw_reopen_commit(BDRVReopenState *state)
+{
+BDRVRawState *new_s = state->opaque;
+BDRVRawState *s = state->bs->opaque;
+
+memcpy(s, new_s, sizeof(BDRVRawState));
+
+g_free(state->opaque);
+state->opaque = NULL;
+}
+
+static void raw_reopen_abort(BDRVReopenState *state)
+{
+g_free(state->opaque);
+state->opaque = NULL;
 }
 
 static int coroutine_fn raw_co_preadv(BlockDriverState *bs, uint64_t offset,
   uint64_t bytes, QEMUIOVector *qiov,
   int flags)
 {
+BDRVRawState *s = bs->opaque;
+
 BLKDBG_EVENT(bs->file, BLKDBG_READ_AIO);
+offset += s->offset;
 return bdrv_co_preadv(bs->file, offset, bytes, qiov, flags);
 }
 
@@ -62,11 +175,18 @@ static int coroutine_fn raw_co_pwritev(BlockDriverState 
*bs, uint64_t offset,
uint64_t bytes, QEMUIOVector *qiov,
int flags)
 {
+BDRVRawState *s = bs->opaque;
 void *buf = NULL;
 BlockDriver *drv;
 QEMUIOVector local_qiov;
 int ret;
 
+if (s->has_size && (offset > s->size || bytes > (s->size - offset))) {
+/* There's not enough space for the data. Don't write anything and just
+ * fail to prevent

[Qemu-block] [PATCH v3] Add 'offset' and 'size' options

2016-10-19 Thread Tomáš Golembiovský 
v2 -> v3:
- changed overflow check to make it clearer
- produce error instead of warning when size is not multiple of sector
  size

v1 -> v2:
- options were moved from 'file' driver into 'raw' driver as suggested
- added support for writing, reopen and truncate when possible

Tomáš Golembiovský (1):
  raw_bsd: add offset and size options

 block/raw_bsd.c  | 168 ++-
 qapi/block-core.json |  16 -
 2 files changed, 180 insertions(+), 4 deletions(-)

-- 
2.10.0

Re: [Qemu-block] [PATCH 16/20] qemu-img: call aio_context_acquire/release around block job

2016-10-19 Thread Paolo Bonzini 


On 19/10/2016 02:54, Fam Zheng wrote:
> On Mon, 10/17 15:54, Paolo Bonzini wrote:
>> This will be needed by bdrv_reopen_multiple, which calls
>> bdrv_drain_all and thus will *release* the AioContext.
> 
> Looks okay, but I wonder how bdrv_drain_all releasing AioContext break 
> anything?

If you don't acquire it first, you get an assertion failure for
releasing a mutex you haven't acquired yet.

Initially I made aio_context_acquire a nop for the default QEMU context,
but then decided it was a hack.

Paolo

> Fam
> 
>>
>> Signed-off-by: Paolo Bonzini 
>> ---
>> v1->v2: new [qemu-iotests]
>>
>>  qemu-img.c | 6 ++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/qemu-img.c b/qemu-img.c
>> index 02c07b9..ad7c964 100644
>> --- a/qemu-img.c
>> +++ b/qemu-img.c
>> @@ -795,6 +795,7 @@ static void run_block_job(BlockJob *job, Error **errp)
>>  {
>>  AioContext *aio_context = blk_get_aio_context(job->blk);
>>  
>> +aio_context_acquire(aio_context);
>>  do {
>>  aio_poll(aio_context, true);
>>  qemu_progress_print(job->len ?
>> @@ -802,6 +803,7 @@ static void run_block_job(BlockJob *job, Error **errp)
>>  } while (!job->ready);
>>  
>>  block_job_complete_sync(job, errp);
>> +aio_context_release(aio_context);
>>  
>>  /* A block job may finish instantaneously without publishing any 
>> progress,
>>   * so just signal completion here */
>> @@ -819,6 +821,7 @@ static int img_commit(int argc, char **argv)
>>  Error *local_err = NULL;
>>  CommonBlockJobCBInfo cbi;
>>  bool image_opts = false;
>> +AioContext *aio_context;
>>  
>>  fmt = NULL;
>>  cache = BDRV_DEFAULT_CACHE;
>> @@ -928,8 +931,11 @@ static int img_commit(int argc, char **argv)
>>  .bs   = bs,
>>  };
>>  
>> +aio_context = bdrv_get_aio_context(bs);
>> +aio_context_acquire(aio_context);
>>  commit_active_start("commit", bs, base_bs, 0, BLOCKDEV_ON_ERROR_REPORT,
>>  common_block_job_cb, , _err, false);
>> +aio_context_release(aio_context);
>>  if (local_err) {
>>  goto done;
>>  }
>> -- 
>> 2.7.4
>>
>>

Re: [Qemu-block] [Qemu-devel] [PATCH v14 13/21] qdict: allow qdict_crumple to accept compound types as values

2016-10-19 Thread Daniel P. Berrange 
On Wed, Oct 19, 2016 at 11:25:27AM +0200, Kevin Wolf wrote:
> Am 18.10.2016 um 17:35 hat Markus Armbruster geschrieben:
> > Kevin Wolf  writes:
> > > Of course, you could argue that flat QDicts are the wrong data structure
> > > in the first place and instead of flatting everything we should have
> > > done the equivalent of qdict_crumple from the beginning, but they were
> > > the natural extension of what was already there and happened to work
> > > good enough, so this is what we're currently using.
> > 
> > We didn't "flatten everything", because QemuOpts is and has always been
> > flat to begin with.  Rather we've started to crumple a few things, and
> > in a separate layer.
> 
> That's the QemuOpts point of view.
> 
> I was talking from a block layer view. There we get data in two
> different formats, QMP and the command line. The first is structured,
> the second is flat. We need to make both uniform before we can pass them
> to the actual block layer functions.
> 
> The current code chose to flatten what we get from QMP blockdev-add and
> use that throughout the block layer. We could instead have decided that
> we leave the blockdev-add input as it is, crumple what we get from
> QemuOpts and use nested QObjects throughout the block layer.

I would very much like to see BlockdevOptions structs passed around
the block drivers, instead of QemuOpts - I think it'd lead to much
clearer code than the QemuOpts stuff we have today IMHO

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://entangle-photo.org   -o-http://search.cpan.org/~danberr/ :|

Re: [Qemu-block] [Qemu-devel] [PATCH v14 13/21] qdict: allow qdict_crumple to accept compound types as values

2016-10-19 Thread Kevin Wolf 
Am 18.10.2016 um 17:35 hat Markus Armbruster geschrieben:
> Kevin Wolf  writes:
> > Of course, you could argue that flat QDicts are the wrong data structure
> > in the first place and instead of flatting everything we should have
> > done the equivalent of qdict_crumple from the beginning, but they were
> > the natural extension of what was already there and happened to work
> > good enough, so this is what we're currently using.
> 
> We didn't "flatten everything", because QemuOpts is and has always been
> flat to begin with.  Rather we've started to crumple a few things, and
> in a separate layer.

That's the QemuOpts point of view.

I was talking from a block layer view. There we get data in two
different formats, QMP and the command line. The first is structured,
the second is flat. We need to make both uniform before we can pass them
to the actual block layer functions.

The current code chose to flatten what we get from QMP blockdev-add and
use that throughout the block layer. We could instead have decided that
we leave the blockdev-add input as it is, crumple what we get from
QemuOpts and use nested QObjects throughout the block layer.

> I now think this is the wrong approach.  We have clearly outgrown flat
> options.  We should redo QemuOpts to support what we need instead of
> bolting on an optional crumpling layer.
> 
> I guess I reached the place you described, just on a different path :)

Yes, I think the conclusion is easy to agree on.

> > Okay, so I like JSON. It's a great format for our monitor protocol. We
> > even have pretty printers so that it's more or less readable as output
> > for human users. However, there is one thing for which it is horrible:
> > Getting user input.
> >
> > Yes, getting rid of the comma escaping is a first step, but typing JSON
> > on the command line remains a PITA. Mostly because of the quotes (which
> > you probably have to escape), but also things like the useless outer
> > brackets.
> 
> As long as you don't need "'" in your JSON, you can simply enclose in
> "'" and be done.  Since "'" can only occur in JSON strings, and the same
> strings would be present in any other syntax, any other syntax would
> be pretty much the same PITA.

I've written enough scripts (qemu-iotests cases) that produce JSON with
shell variables in it, so if anything you can use "" quoting for the
shell and make use of qemu's extension that '' is accepted in JSON, too.

Anyway, the quotes aren't only nasty because of the escaping, but also
just because I have to type them.

> >> If you'd rather invent syntax closer to QemuOpts than reuse JSON, you
> >> could try
> >> 
> >> driver=quorum,
> >> child=[{ driver=file, filename=disk1.img },
> >>{ driver=host_device, filename=/dev/sdb },
> >>{ driver=nbd, host=localhost } ]
> >
> > This looks a lot more agreeable as something that I have to type on the
> > command line.
> >
> > What's more, this is a direct extension of the existing syntax. You
> > complained about how the step from simple configurations with -hda to
> > more complex ones with -drive completely changes the syntax (and rightly
> > so). Going from simple QemuOpts syntax to doing JSON as soon as you get
> > structured values and lists would be another similar step. In contrast,
> > the new syntax you're suggesting above is a natural extension of what's
> > there.
> 
> Point taken.  I just don't like inventing syntax, because as a rule, way
> too much syntax gets invented, and almost always badly.
> 
> >> I'd go with some existing syntax, though.  The one we already use is
> >> JSON.
> >
> > The one we already use in this context is comma separated key/value
> > pairs as parsed by QemuOpts.
> 
> Which is flat, and flat doesn't cut it anymore.
> 
> If you make it non-flat with dotted key convention, the dotted key
> convention becomes part of the syntax.  Counts as inventing syntax in my
> book.

Yes, it is. Though in the context of command line options, dotted syntax
is an invention already made, whereas JSON would be a new invention.
(Well, not completely, because for block devices we already have json: -
but that works a little different again.)

> >> Your dotted key convention requires two rules: 1. names must not look
> >> like integers, and 2. names must not contain '.'.
> >> 
> >> We can avoid rule 2 by requiring '.' to be escaped.  Dan's
> >> qdict_crumple() currently does that, to your surprise.  Adding the
> >> escaping to existing options is a compatibility break, however.  So, if
> >> names with '.' already exist, we can either break compatibility by
> >> renaming them, or break it by requiring the '.' to be escaped.
> >
> > I don't think we should support any escaping and rather forbid '.'
> > completely in names.
> 
> I think we should adopt QAPI's naming rules.

Which includes what I said, so fine with me.

> > If you're concerned about compatibility issues if we find a dot in a
> > name only in one of the later

Re: [Qemu-block] [Qemu-devel] [PATCH v3 4/4] qemu-iotests: Test creating floppy drives

2016-10-19 Thread Kevin Wolf 
Am 18.10.2016 um 21:53 hat Eric Blake geschrieben:
> On 10/18/2016 02:45 PM, John Snow wrote:
> > 
> > 
> > On 10/18/2016 06:22 AM, Kevin Wolf wrote:
> >> This tests the different supported methods to create floppy drives and
> >> how they interact.
> >>
> 
> >> +function check_floppy_qtree()
> >> +{
> >> +echo
> >> +echo Testing: "$@" | _filter_testdir
> >> +
> >> +# QEMU_OPTIONS contains -nodefaults, we don't want that here
> >> because the
> >> +# defaults are part of what should be checked here
> >> +echo "info qtree" |
> >> +QEMU_OPTIONS="" do_run_qemu "$@" | _filter_win32 |
> >> +grep -zo '[[:cntrl:]]\( *\)dev: isa-fdc.*\([[:cntrl:]]\1
> >> .*\)*[[:cntrl:]] *dev:'
> > 
> > This grep invocation doesn't appear to actually terminate with the '-z'
> > option here. Not sure why, I haven't looked into the bash framework
> > much, hopefully it's not too hard for you to reproduce and correct.

No, obviously I can't reproduce, otherwise I wouldn't have written the
test case like this. It passes just fine for me on RHEL 7.

Just to clarify, it's grep that doesn't terminate, or qemu? Also, what
do you mean by the "bash framework"?

> Is 'grep -z' even portable to BSD, or is it just a GNU extension?  Would
> it be better to run the output through tr to convert things to a saner
> subset of characters before then grepping a text file?

Is qemu-iotests even supposed to run on BSD? All our test cases specify
"_supported_os Linux". (And I don't think this means Linux kernel with
BSD userland :-))

Anyway, the tr thing you mean would be translating all newlines into
something else (which is hopefully otherwise unused), then grep, then
translate back?

What this line is supposed to do (if it wasn't obvious) is extracting
the full information for a single device from 'info qtree'. I don't
really mind how it's done, but multiline operation seems to be something
that isn't trivial with most tools... I think I've done multiline sed
before, so maybe that would be another option.

Kevin


pgpSsbq2KMIZe.pgp
Description: PGP signature