Re: [Qemu-block] [Qemu-devel] [PATCH] qemu-img: add check for zero-length job len

2015-11-02 Thread Eric Blake
block-stream has no work to do, but can complete instantly. Will this result in such a job never reporting 100% complete? If so, that's bad. If you can answer my concerns that we don't have a design bug, then the code changes look correct, and you can add: Reviewed-by: Eric Blak

Re: [Qemu-block] [Qemu-devel] [PATCH] qcow2: avoid misaligned 64bit bswap

2015-11-02 Thread Eric Blake
observed in iotests 15, 26, > 44, 115 and 121. > > Signed-off-by: John Snow > --- > block/qcow2-refcount.c | 11 +++ > 1 file changed, 7 insertions(+), 4 deletions(-) Reviewed-by: Eric Blake -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt vir

Re: [Qemu-block] [Qemu-devel] [PATCH v3 2/3] block: Remove inner quotation marks in iotest 085

2015-11-03 Thread Eric Blake
1 file changed, 8 insertions(+), 8 deletions(-) Reviewed-by: Eric Blake I might have worded the commit message differently, though: block: Remove incorrect "" in iotest 085 We had the patterns: cmd="..."${variable}"..." _send_qemu_cmd ... "..."${variabl

Re: [Qemu-block] [Qemu-devel] [PATCH v3 3/3] block: test 'blockdev-snapshot' using a file BDS as the overlay

2015-11-03 Thread Eric Blake
; {"return": {}} > > +=== Invalid command - cannot create a snapshot using a file BDS === > + > +{"error": {"class": "GenericError", "desc": "The snapshot does not support > backing images"}} This message could use

Re: [Qemu-block] [Qemu-devel] [PATCH v3 1/3] block: Disallow snapshots if the overlay doesn't support backing files

2015-11-03 Thread Eric Blake
'node-name': 'new0', >'file': { 'driver': 'file', > 'filename': 'new.qcow2', > 'node-name': 'file0' } } } } > > { 'exec

Re: [Qemu-block] [PATCH v10 12/14] block: add transactional properties

2015-11-03 Thread Eric Blake
perties: Optional structure of additional options to control the Elsewhere, we've spelled it '#optional'; Marc-Andre has patches that rely on that spelling to turn it .json into documentation. But otherwise, looks good on first glance. -- Eric Blake eblake redhat com+1-919

Re: [Qemu-block] [Qemu-devel] [PATCH v10 12/14] block: add transactional properties

2015-11-03 Thread Eric Blake
On 11/03/2015 10:31 AM, John Snow wrote: > > > On 11/03/2015 10:23 AM, Eric Blake wrote: >> On 10/23/2015 05:56 PM, John Snow wrote: >>> Add both transactional properties to the QMP transactional interface, >>> and add the BlockJobTxn that we create as a result

Re: [Qemu-block] [PATCH v6 14/15] block: Rewrite bdrv_close_all()

2015-11-05 Thread Eric Blake
own ErrorClass. Eric, what do you think? Needing a unique ErrorClass is only important if we expect a client (libvirt) would behave differently based on that error class (clients are not allowed to parse the error message). But what is the scenario that we are trying to test here, rewritten in terms of libvirt API commands? Should libvirt behave any differently because a blockjob was running than for any other failure, if the end result is still that libvirt can't eject or hot-unplug the disk because of a failure? -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature

Re: [Qemu-block] [PATCH v2 0/8] blockdev: Further BlockBackend work

2015-11-09 Thread Eric Blake
Is there any way to make 'git backport-diff' automatically do this, or does it have to be done manually when renaming patches? > 007/8:[0213] [FC] 'block: Move some bdrv_*_all() functions to BB' > 008/8:[0027] [FC] 'block: Remove bdrv_states' > -- 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 1/8] qapi: Drop QERR_UNKNOWN_BLOCK_FORMAT_FEATURE

2015-11-09 Thread Eric Blake
| 6 +- > block/qcow2.c | 24 +--- > block/qed.c| 7 ++- > block/vmdk.c | 7 ++- > include/qapi/qmp/qerror.h | 3 --- > tests/qemu-iotests/036.out | 16 > 6 files changed, 18 inserti

Re: [Qemu-block] [PATCH v4 10/21] block: New option to define the intervals for collecting I/O statistics

2015-11-10 Thread Eric Blake
3,7 @@ > '*read-only': 'bool', > '*stats-account-invalid': 'bool', > '*stats-account-failed': 'bool', > +'*stats-intervals': 'str', My fault for not reviewing

Re: [Qemu-block] [PATCH] docs: update bitmaps.md

2015-11-10 Thread Eric Blake
on notice on failed jobs? I guess it's because you can configure jobs to report errors but continue on, so the error notification alone doesn't say whether the job ends. > + > +```json > +{ "timestamp": { "seconds": 1447193702, "micros

[Qemu-block] [PATCH v11 19/28] qapi: Change munging of CamelCase enum values

2015-11-10 Thread Eric Blake
ully, only two enums are affected: ErrorClass and InputButton. Suggested by: Markus Armbruster Signed-off-by: Eric Blake --- v11: new patch --- backends/rng-egd.c| 2 +- balloon.c | 4 ++-- block.c | 2 +- blockdev-nbd.c

[Qemu-block] [PATCH v11 18/28] qerror: more error_setg() usage

2015-11-10 Thread Eric Blake
A few uses of error_set(ERROR_CLASS_GENERIC_ERROR) have snuck in since c6bd8c706. Nuke them. Signed-off-by: Eric Blake --- v11: new patch --- block.c | 3 +-- docs/writing-qmp-commands.txt | 20 +--- hw/i386/pc.c | 2 +- hw/net/rocker

[Qemu-block] [PATCH v11 21/28] qapi: Convert qtype_code into qapi enum type

2015-11-10 Thread Eric Blake
it definitions once, even when two qapi files are being compiled into the same binary (the way we already handled builtin list types like 'intList'). We may need to revisit how multiple qapi files share common types, but that's a project for another day. Signed-off-by: Eric Blake

Re: [Qemu-block] [PATCH for-2.5] block: make 'stats-intervals' a list of integers

2015-11-11 Thread Eric Blake
ervals[i] != NULL; i++) { > +case QTYPE_QSTRING: { > +case QTYPE_QINT: { Why are we accepting both string and int here, but typing it as 'int' in qapi? I guess its due to how command line parsing passes in strings rather than ints? But that should b

Re: [Qemu-block] [Qemu-devel] [PATCH v11 18/28] qerror: more error_setg() usage

2015-11-11 Thread Eric Blake
On 11/11/2015 07:23 AM, Andreas Färber wrote: > Am 11.11.2015 um 15:21 schrieb Markus Armbruster: >> Eric Blake writes: >> >>> A few uses of error_set(ERROR_CLASS_GENERIC_ERROR) have snuck in >>> since c6bd8c706. Nuke them. >> >> Doesn't real

Re: [Qemu-block] [Qemu-devel] [PATCH v11 19/28] qapi: Change munging of CamelCase enum values

2015-11-11 Thread Eric Blake
[hmm, wonder why scripts/get-maintainer.pl didn't loop in Gerd to the patch itself] On 11/11/2015 07:50 AM, Markus Armbruster wrote: > Eric Blake writes: > >> When munging enum values, the fact that we were passing the entire >> prefix + value through camel_to_upper()

Re: [Qemu-block] [Qemu-devel] [PATCH v11 19/28] qapi: Change munging of CamelCase enum values

2015-11-11 Thread Eric Blake
nt. Interesting hack. Certainly more legible, and I'm willing to attempt such a rename. As far as I'm concerned, though, this does not fix a bug, so much as make it easier for later qapi patches to be more robust. So any renaming we do is not under pressure to have to make it in by 2.5.

Re: [Qemu-block] [Qemu-devel] [PATCH v11 18/28] qerror: more error_setg() usage

2015-11-11 Thread Eric Blake
On 11/11/2015 07:21 AM, Markus Armbruster wrote: > Eric Blake writes: > >> A few uses of error_set(ERROR_CLASS_GENERIC_ERROR) have snuck in >> since c6bd8c706. Nuke them. > > Doesn't really belong to this series, but that's okay. If you're going to mod

Re: [Qemu-block] [Qemu-devel] [PULL v2 00/40] Block layer patches

2015-11-11 Thread Eric Blake
^~~ > > Isn't that actually a bug in the system headers? If I understand the > spec correctly, SIZE_MAX should be size_t. ("this expression shall have > the same type as would an expression that is an object of the > correspondin

Re: [Qemu-block] [Qemu-devel] [PATCH v11 21/28] qapi: Convert qtype_code into qapi enum type

2015-11-11 Thread Eric Blake
On 11/11/2015 09:42 AM, Markus Armbruster wrote: > Eric Blake writes: > >> What's more meta than using qapi to define qapi? :) >> >> Convert qtype_code into a full-fledged[*] builtin qapi enum type, >> so that a subsequent patch can then use it as the disc

Re: [Qemu-block] [Qemu-devel] [PATCH v11 18/28] qerror: more error_setg() usage

2015-11-11 Thread Eric Blake
On 11/11/2015 10:31 AM, Markus Armbruster wrote: > Eric Blake writes: > >> On 11/11/2015 07:21 AM, Markus Armbruster wrote: >>> Eric Blake writes: >>> >>>> A few uses of error_set(ERROR_CLASS_GENERIC_ERROR) have snuck in >>>> since c6bd8c

Re: [Qemu-block] [PATCH 0/4] block/gluster: add support for multiple gluster servers

2015-11-12 Thread Eric Blake
, image] -> [host, volume, path] > block/gluster: code cleanup > block/gluster: using new qapi schema > block/gluster: add support for multiple gluster servers > -- 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 v11 19/28] qapi: Change munging of CamelCase enum values

2015-11-13 Thread Eric Blake
On 11/10/2015 11:51 PM, Eric Blake wrote: > When munging enum values, the fact that we were passing the entire > prefix + value through camel_to_upper() meant that enum values > spelled with CamelCase could be turned into CAMEL_CASE. However, > this provides a potential collision (bot

Re: [Qemu-block] [Qemu-devel] [PATCH v11 19/28] qapi: Change munging of CamelCase enum values

2015-11-13 Thread Eric Blake
es, while still searching both dictionaries for lookups). At any rate, I'm playing with patches along these lines. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature

[Qemu-block] blkdebug event names [was: What to do about QAPI naming convention violations]

2015-11-16 Thread Eric Blake
ake the change to conventional spelling. At this point, I think 2.5 is too late, so the change will have to go into 2.6, but I'm preparing a patch for fixing this interface. -- 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 v11 21/28] qapi: Convert qtype_code into qapi enum type

2015-11-17 Thread Eric Blake
hema, or they generate builtins. > > * When they generate builtins, they always use well-known file names. > > * When they generate for a schema, they always generate the #include for > the well-known builtin .h. They never generate builtins. I'd probably go w

[Qemu-block] [PATCH v12 19/36] blkdebug: Merge hand-rolled and qapi BlkdebugEvent enum

2015-11-18 Thread Eric Blake
Wolf Signed-off-by: Eric Blake --- v12: new patch --- block.c | 2 +- block/blkdebug.c | 79 +++ docs/blkdebug.txt | 7 +++-- include/block/block.h | 62 + include/

[Qemu-block] [PATCH v12 22/36] qapi: Don't let implicit enum MAX member collide

2015-11-18 Thread Eric Blake
)s] = NULL, |+// %(max_index)s | }; | ''', |max_index=max_index) then running: $ cat qapi-{types,event}.c tests/test-qapi-types.c | sed -n 's,^// \(.*\)MAX,s|\1MAX|\1_MAX|g,p' > list $ git grep -l _MAX | xargs sed -i -f list The only things not generate

[Qemu-block] [PATCH v12 20/36] blkdebug: Avoid '.' in enum values

2015-11-18 Thread Eric Blake
/g" done followed by a manual touchup to test 77 to keep the test working. Reported-by: Markus Armbruster CC: Kevin Wolf Signed-off-by: Eric Blake --- v12: new commit. Arguably, test 77 should have failed immediately on an unrecognized event name instead of hanging forever, but that fix

[Qemu-block] [PATCH v12 29/36] qobject: Rename qtype_code to QType

2015-11-18 Thread Eric Blake
The name QType is more in line with our conventions for qapi types, and matches the fact that each enum member has a prefix of QTYPE_. Signed-off-by: Eric Blake --- v12: new patch split off of 21/28 --- block/qapi.c | 4 ++-- include/hw/qdev-core.h | 2 +- include/qapi/qmp

Re: [Qemu-block] [Qemu-devel] [PATCH v12 19/36] blkdebug: Merge hand-rolled and qapi BlkdebugEvent enum

2015-11-18 Thread Eric Blake
On 11/18/2015 05:08 AM, Kevin Wolf wrote: > Am 18.11.2015 um 11:30 hat Markus Armbruster geschrieben: >> Eric Blake writes: >> >>> No need to keep two separate enums, where editing one is likely >>> to forget the other. Now that we can specify a qapi enum prefix,

Re: [Qemu-block] [Qemu-devel] [PATCH v12 22/36] qapi: Don't let implicit enum MAX member collide

2015-11-18 Thread Eric Blake
On 11/18/2015 06:12 AM, Markus Armbruster wrote: > Eric Blake writes: > >> Now that we guarantee the user doesn't have any enum values >> beginning with a single underscore, we can use that for our >> own purposes. Renaming ENUM_MAX to ENUM__MAX makes it ob

Re: [Qemu-block] [Qemu-devel] [PATCH v12 29/36] qobject: Rename qtype_code to QType

2015-11-18 Thread Eric Blake
On 11/18/2015 11:25 AM, Markus Armbruster wrote: > Eric Blake writes: > >> The name QType is more in line with our conventions for qapi >> types, and matches the fact that each enum member has a prefix >> of QTYPE_. >> >> Signed-off-by: Eric Blake > >

[Qemu-block] [PATCH v13 02/14] qobject: Rename qtype_code to QType

2015-11-20 Thread Eric Blake
, which also wants CamelCase type names. Signed-off-by: Eric Blake --- v13: commit message touchup, rebase to cleaner QObject v12: new patch split off of 21/28 --- block/qapi.c | 4 ++-- include/hw/qdev-core.h | 2 +- include/qapi/qmp/qobject.h | 8 qobject/qdict.c

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.5] block/qapi: Plug memory leak on query-block error path

2015-11-20 Thread Eric Blake
rr); > -goto err; > +g_free(info); > +qapi_free_BlockInfoList(head); > +return NULL; > } > Only info was leaked, but inlining the entire 'err' label into its one use was easier than hoisting the declaration of info out of the loop. Revie

Re: [Qemu-block] [Qemu-devel] [PATCH] block/qapi: Plug memory leak on query-block error path

2015-11-20 Thread Eric Blake
On 11/20/2015 05:53 AM, Markus Armbruster wrote: > Spotted by Coverity. Worth mentioning that commit 553a7e87 was the culprit. > > Signed-off-by: Markus Armbruster > --- > block/qapi.c | 8 +++- > 1 file changed, 3 insertions(+), 5 deletions(-) > -- Eric Blake eb

Re: [Qemu-block] [Qemu-devel] [PATCH WIP 01/30] crypto: add QCryptoSecret object class for password/key handling

2015-11-20 Thread Eric Blake
d in JSON only valid UTF-8 sequences can be > used > +# @base64: arbitrary base64 encoded binary data > +# Since: 2.5 You've missed 2.5. Probably need to tweak the whole series to call out 2.6. > +## > +{ 'enum': 'QCryptoSecretFormat', > + 'prefix': 'QCRYPTO_SECRET_FORMAT', > + 'data': ['raw', 'base64']} > diff --git a/qemu-options.hx b/qemu-options.hx > index 0eea4ee..dd3f7f8 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -3670,6 +3670,7 @@ queue @var{all|rx|tx} is an option that can be applied > to any netfilter. > @option{tx}: the filter is attached to the transmit queue of the netdev, > where it will receive packets sent by the netdev. > > + > @item -object > filter-dump,id=@var{id},netdev=@var{dev},file=@var{filename}][,maxlen=@var{len}] Why the added blank line here? -- 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 WIP 04/30] qcow2: add a 'keyid' parameter to qcow2 options

2015-11-20 Thread Eric Blake
'int', > '*l2-cache-size': 'int', > '*refcount-cache-size': 'int', > -'*cache-clean-interval': 'int' } } > +'*cache-clean-interval': 'int', > +'*keyid': 'str' } } > > > ## > -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature

Re: [Qemu-block] [Patch v7 3/3] qmp: add monitor command to add/remove a child

2015-11-23 Thread Eric Blake
uot;test_new_node", > +"file": { "driver": "file", > + "filename": "test.raw" } } } } > +<- { "return": {} } > +-> { "execute": "x-blockdev-change", > +"arguments": { "parent": "disk1", > + "node": "new_node" } } > +<- { "return": {} } > + > +Delete a quorum's node > +-> { "execute": "x-blockdev-change", > +"arguments": { "parent": "disk1", > + "child": "children.2" } } > +<- { "return": {} } > + > +EQMP > + > +{ > .name = "query-named-block-nodes", > .args_type = "", > .mhandler.cmd_new = qmp_marshal_query_named_block_nodes, > -- 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 for-2.6 01/14] block: Add "file" output parameter to block status query functions

2015-11-23 Thread Eric Blake
e right value, let's clear it explicitly > right before calling .bdrv_get_block_status. > > Signed-off-by: Fam Zheng > --- Reviewed-by: Eric Blake Is there ever going to be a need to expose this through QMP, or is this just used for the qemu-img map subcommand? -- Eric Bl

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.6 12/14] qemu-img: In 'map', use QDict to generate JSON output

2015-11-23 Thread Eric Blake
65536, "start": 0, "zero": false, "depth": 0, "data": true} ...but the output shows that QDict output is tied to internal hashing and therefore different than first-in-first-out creation order. JSON-wise, it's still valid. But are we guaranteed that our hashing is stable? If so, is that something an attacker could attempt to exploit as a Denial of Service, by intentionally creating predictable collisions? [I'm guessing not, as the denial of service is mainly an issue if a user can degrade performance of typical lookups from nominal O(1) to O(n) by supplying LOTS of user-provided input, but the keys we output in JSON are generally under our control via .json files and not something where the user is dynamically creating lots of keys - but still worth asking.] And if it is not stable, then will our test break when someone else's system hashes differently than yours? -- 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 for-2.6 02/14] qcow: Assign bs->file->bs to file in qcow_co_get_block_status

2015-11-24 Thread Eric Blake
On 11/23/2015 10:21 PM, Fam Zheng wrote: > Signed-off-by: Fam Zheng > --- > block/qcow.c | 1 + > 1 file changed, 1 insertion(+) Reviewed-by: Eric Blake > > diff --git a/block/qcow.c b/block/qcow.c > index 558f443..b59383f 100644 > --- a/block/qcow.c > +++ b/bloc

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.6 03/14] qcow2: Assign bs->file->bs to file in qcow2_co_get_block_status

2015-11-24 Thread Eric Blake
On 11/23/2015 10:22 PM, Fam Zheng wrote: > Signed-off-by: Fam Zheng > --- > block/qcow2.c | 1 + > 1 file changed, 1 insertion(+) Reviewed-by: Eric Blake > > diff --git a/block/qcow2.c b/block/qcow2.c > index 836888c..7634c42 100644 > --- a/block/qcow2.c > +++ b

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.6 04/14] raw: Assign bs to file in raw_co_get_block_status

2015-11-24 Thread Eric Blake
On 11/23/2015 10:22 PM, Fam Zheng wrote: > Signed-off-by: Fam Zheng > --- > block/raw-posix.c | 1 + > block/raw_bsd.c | 1 + > 2 files changed, 2 insertions(+) > Reviewed-by: Eric Blake -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.6 05/14] iscsi: Assign bs to file in iscsi_co_get_block_status

2015-11-24 Thread Eric Blake
On 11/23/2015 10:22 PM, Fam Zheng wrote: > Signed-off-by: Fam Zheng > --- > block/iscsi.c | 3 +++ > 1 file changed, 3 insertions(+) Reviewed-by: Eric Blake > > diff --git a/block/iscsi.c b/block/iscsi.c > index 2d1e230..8c7f1b3 100644 > --- a/block/iscsi.c > +++

Re: [Qemu-block] [PATCH v2 12/14] qemu-img: Make MapEntry a QAPI struct

2015-11-25 Thread Eric Blake
>has_filename = true; > +e->filename = bs->filename; > +} > return 0; > } Are we guaranteed that bs->filename is non-NULL when offset is set? Or does this need to be if (e->has_offset && bs->filename)? > > @@ -2307,9 +2301,11 @@ static

Re: [Qemu-block] [PATCH v2 13/14] qemu-img: Use QAPI visitor to generate JSON

2015-11-25 Thread Eric Blake
int get_block_status(BlockDriverState *bs, > int64_t sector_num, > e->offset = ret & BDRV_BLOCK_OFFSET_MASK; > e->has_offset = !!(ret & BDRV_BLOCK_OFFSET_VALID); > e->depth = depth; > -if (e->has_offset) { > +if (file && e->has_offset) { > e->has_filename = true; > -e->filename = bs->filename; > +e->filename = file->filename; Does this hunk belong in a different patch? Reviewed-by: Eric Blake -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature

Re: [Qemu-block] [PATCH v2 14/14] iotests: Add "qemu-img map" test for VMDK extents

2015-11-25 Thread Eric Blake
On 11/25/2015 12:39 AM, Fam Zheng wrote: > Signed-off-by: Fam Zheng > --- > tests/qemu-iotests/059 | 10 ++ > tests/qemu-iotests/059.out | 38 ++ > 2 files changed, 48 insertions(+) Reviewed-by: Eric Blake -- Eric Blake eb

Re: [Qemu-block] [PATCH v2 06/14] parallels: Assign bs->file->bs to file in parallels_co_get_block_status

2015-11-25 Thread Eric Blake
On 11/25/2015 12:39 AM, Fam Zheng wrote: > Signed-off-by: Fam Zheng > --- > block/parallels.c | 1 + > 1 file changed, 1 insertion(+) Reviewed-by: Eric Blake > > diff --git a/block/parallels.c b/block/parallels.c > index d1146f1..6552f32 100644 > --- a/block/p

Re: [Qemu-block] [PATCH v2 07/14] qed: Assign bs->file->bs to file in bdrv_qed_co_get_block_status

2015-11-25 Thread Eric Blake
On 11/25/2015 12:39 AM, Fam Zheng wrote: > Signed-off-by: Fam Zheng > --- > block/qed.c | 3 +++ > 1 file changed, 3 insertions(+) > Reviewed-by: Eric Blake -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.

Re: [Qemu-block] [Qemu-devel] [PATCH v8] block/raw-posix.c: Make physical devices usable in QEMU under Mac OS X host

2015-11-25 Thread Eric Blake
p;& defined(__MACH__) > +/* if a physical device experienced an error while being opened */ > +if (strncmp(file_name, "/dev/", 5) == 0 && ret != 0) { > +printUnmountingDirections(file_name); Is this advice appropriate to ALL things under /dev/, or just cdroms? > +return -1; > +} > +#endif /* defined(__APPLE__) && defined(__MACH__) */ > + > /* Since this does ioctl the device must be already opened */ > bs->sg = hdev_is_sg(bs); > > -- 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 v9] block/raw-posix.c: Make physical devices usable in QEMU under Mac OS X host

2015-11-25 Thread Eric Blake
, "Error: Failed to find a working partition on " > + > "disc!\n"); and I already pointed out on v8 that this is not the correct usage of error_setg(). So, here's hoping v10 addresses the comments here and elsewhere. -- 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 v9] block/raw-posix.c: Make physical devices usable in QEMU under Mac OS X host

2015-11-25 Thread Eric Blake
On 11/25/2015 09:23 PM, Eric Blake wrote: >> +static kern_return_t FindEjectableOpticalMedia(io_iterator_t *mediaIterator, >> +char >> *mediaType) > > Unusual indentation; more typical is: >

Re: [Qemu-block] [PATCH v2 01/14] block: Add "file" output parameter to block status query functions

2015-11-25 Thread Eric Blake
same place) > BDRV_BLOCK_OFFSET_VALID in ret is set. > > Until block drivers fill in the right value, let's clear it explicitly > right before calling .bdrv_get_block_status. > > Signed-off-by: Fam Zheng > --- With that fix, Reviewed-by: Eric Blake -- Eric Blake

Re: [Qemu-block] [PATCH v2 08/14] sheepdog: Assign bs to file in sd_co_get_block_status

2015-11-25 Thread Eric Blake
On 11/25/2015 12:39 AM, Fam Zheng wrote: > Signed-off-by: Fam Zheng > --- > block/sheepdog.c | 3 +++ > 1 file changed, 3 insertions(+) Reviewed-by: Eric Blake > > diff --git a/block/sheepdog.c b/block/sheepdog.c > index 0f6789e..d5e7ff8 100644 > --- a/block/

Re: [Qemu-block] [PATCH v2 09/14] vdi: Assign bs->file->bs to file in vdi_co_get_block_status

2015-11-25 Thread Eric Blake
On 11/25/2015 12:39 AM, Fam Zheng wrote: > Signed-off-by: Fam Zheng > --- > block/vdi.c | 1 + > 1 file changed, 1 insertion(+) Reviewed-by: Eric Blake > > diff --git a/block/vdi.c b/block/vdi.c > index 2199fd3..6b1a57b 100644 > --- a/block/vdi.c > +++ b/blo

Re: [Qemu-block] [PATCH v2 10/14] vpc: Assign bs->file->bs to file in vpc_co_get_block_status

2015-11-25 Thread Eric Blake
On 11/25/2015 12:39 AM, Fam Zheng wrote: > Signed-off-by: Fam Zheng > --- > block/vpc.c | 2 ++ > 1 file changed, 2 insertions(+) Reviewed-by: Eric Blake > > diff --git a/block/vpc.c b/block/vpc.c > index 912f5d0..412ff41 100644 > --- a/block/vpc.c > +++ b/blo

Re: [Qemu-block] [PATCH v3 13/15] qemu-img: Make MapEntry a QAPI struct

2015-11-25 Thread Eric Blake
able/mergeable/ here and in the patch body > > Signed-off-by: Fam Zheng > --- > qapi/block-core.json | 27 > qemu-img.c | 71 > +++- > 2 files changed, 69 insertions(+), 29 deletions(-) > Wit

Re: [Qemu-block] [PATCH v3 12/15] qemu-img: In "map", use the returned "file" from bdrv_get_block_status

2015-11-25 Thread Eric Blake
On 11/25/2015 10:05 PM, Fam Zheng wrote: > Now all drivers should return a correct "file", we can make use of it, > even with the recursion into backing chain above. > > Signed-off-by: Fam Zheng > --- > qemu-img.c | 2 +- > 1 file changed, 1 insertion(+), 1 delet

Re: [Qemu-block] [Qemu-devel] [PATCH v9] block/raw-posix.c: Make physical devices usable in QEMU under Mac OS X host

2015-11-30 Thread Eric Blake
Kevin Wolf wanted it this way. What would you do instead? Keven and I both want you to use error_setg(), but to use it correctly - and the correct way is to NOT supply a trailing \n. > > Thank you very much for reviewing my patches. The least you can do for showing that gratitude is to actuall

Re: [Qemu-block] [Qemu-devel] [PATCH v10] block/raw-posix.c: Make physical devices usable in QEMU under Mac OS X host

2015-11-30 Thread Eric Blake
bothered by the fact that I already pointed this out in v9 and you still didn't fix it for v10 that I am not even paying attention to actual code, and just looking at style violations. You have effectively lost me as a valid reviewer on this patch. I don't like feeling like this, as I try hard to be welcoming to new contributors, but in the open source world, you have to return the favor by learning from the advice you are given, rather than repeating the same mistakes. -- 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 v9] block/raw-posix.c: Make physical devices usable in QEMU under Mac OS X host

2015-11-30 Thread Eric Blake
provides the context that an error message is being printed. The whole point of using wrapper functions is that the common functionality (like an 'error:' prefix, or '\n' suffix) is done in the wrapper, not at every call site. If you were using raw printf(), then

Re: [Qemu-block] [Qemu-devel] [PATCH v10] block/raw-posix.c: Make physical devices usable in QEMU under Mac OS X host

2015-11-30 Thread Eric Blake
R to read, because it violates the conventions that you have already trained to read. > I don't remember hearing about not using \n in the error_report() call, but I > will > fix this in the next patch. v8: https://lists.gnu.org/archive/html/qemu-devel/2015-11/msg05806.html &

[Qemu-block] [PATCH v14 02/15] qobject: Rename qtype_code to QType

2015-12-01 Thread Eric Blake
, which also wants CamelCase type names. Signed-off-by: Eric Blake --- v14: rebase to context changes v13: commit message touchup, rebase to cleaner QObject v12: new patch split off of 21/28 --- block/qapi.c | 4 ++-- include/hw/qdev-core.h | 2 +- include/qapi/qmp/qobject.h | 8

Re: [Qemu-block] [PATCH for-2.5] qcow2: always initialize specific image info

2015-12-07 Thread Eric Blake
we end up initializing all fields due to the assignment here; same if qcow_version is exactly 3. The only time qcow2 remains uninitialized is if qcow_version is 0, 1, or > 3; but we refuse to handle qcow files with out-of-range versions. So I don't see how you are plugging any uninitialize

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.5?] qcow2: always initialize specific image info

2015-12-07 Thread Eric Blake
On 12/07/2015 10:51 AM, Eric Blake wrote: > [adding qemu-devel - ALL patches should go to qemu-devel, even if they > are also going to a sub-list like qemu-block] > > On 12/07/2015 10:07 AM, Roman Kagan wrote: >> qcow2_get_specific_info() used to have a code path which would l

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.6 v2 04/10] fdc: add default drive type option

2015-12-07 Thread Eric Blake
t still concise name. > +# @none: No drive connected > +# @auto: Automatically determined by inserted media at boot > +# > +# Since: 2.6 > +## > +{ 'enum': 'FloppyDriveType', > + 'data': ['144', '288', '120', 'none', 'auto']} Reviewed-by: Eric Blake -- 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 1/1] qcow2: insert assert into qcow2_get_specific_info()

2015-12-10 Thread Eric Blake
sert(false); Only covers us if we don't turn on NDEBUG during compile; but then again, lots of spots in the code base assume assert() will never be crippled. Reviewed-by: Eric Blake -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature

Re: [Qemu-block] [PATCH v3 14/15] qemu-img: Use QAPI visitor to generate JSON

2015-12-10 Thread Eric Blake
On 11/25/2015 10:05 PM, Fam Zheng wrote: > A visible improvement is that "filename" is now included in the output > if it's valid. > > Reviewed-by: Eric Blake > Signed-off-by: Fam Zheng > --- > qemu-img.c | 34 ++-

[Qemu-block] [PATCH 01/11] qapi: Rename qjson.h to qobject-json.h

2015-12-10 Thread Eric Blake
We have two different JSON visitors in the tree; and having both named 'qjson.h' can cause include confusion. Rename the qapi version. Kill trailing whitespace in the renamed tests/check-qobject-json.c to keep checkpatch.pl happy. Signed-off-by: Eric Blake --- M

[Qemu-block] [PATCH 11/11] RFC: qemu-img: Use new JSON output formatter

2015-12-10 Thread Eric Blake
of us will get to rebase on the other: https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg01756.html Signed-off-by: Eric Blake Cc: Fam Zheng --- qemu-img.c | 70 ++ 1 file changed, 29 insertions(+), 41 deletions(-) diff --git a

[Qemu-block] [PATCH for-2.5?] qemu-iotests: Reduce racy output in 028

2015-12-10 Thread Eric Blake
Silence the output during the repetitions, then add a final clean command to keep the expected output useful; once patched, I was finally able to run the test 20 times in a row with no failures. Signed-off-by: Eric Blake --- Not sure if this is the best fix, or if it is even appropriate for inc

Re: [Qemu-block] [PATCH V2] qemu-img / curl: When fetching Content-Size use GET instead of HEAD.

2015-12-11 Thread Eric Blake
l, CURLOPT_HEADERFUNCTION, > curl_header_cb); > curl_easy_setopt(state->curl, CURLOPT_HEADERDATA, s); > +curl_easy_setopt(state->curl, CURLOPT_CUSTOMREQUEST, "GET"); > if (curl_easy_perform(state->curl)) > goto out; > curl_e

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.5 1/1] blockdev: Mark {insert, remove}-medium experimental

2015-12-11 Thread Eric Blake
ed-off-by: Max Reitz > Acked-by: Markus Armbruster > Acked-by: Kevin Wolf > --- Reviewed-by: Eric Blake > +++ b/qmp-commands.hx > @@ -4203,13 +4203,13 @@ Example: > EQMP > > { > -.name = "blockdev-remove-medium",

Re: [Qemu-block] [Qemu-devel] ping [PATCH v11] block/raw-posix.c: Make physical devices usable in QEMU under Mac OS X host

2015-12-11 Thread Eric Blake
ct). An interface that is hard to use correctly should be avoided in favor of one that is less mental effort. > > Is this what you had in mind: > > mediaType = g_strdup(matching_array[index]); Pretty much, coupled with g_free(mediaType) later on. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature

Re: [Qemu-block] [PATCH V2] fix:readcapacity 10 failure was shown even 16 sent

2015-12-14 Thread Eric Blake
command."); While you're touching this, please also drop the trailing '.' > } else if (!iscsilun->block_size || > iscsilun->block_size % BDRV_SECTOR_SIZE) { > error_setg(errp, "iSCSI: the target returned an invalid " >

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.6 v2 04/10] fdc: add default drive type option

2015-12-15 Thread Eric Blake
; > Sound reasonable? On paper, yes that sounds reasonable. I'm also not familiar enough with machine types to know if it will let you keep 2.5 and earlier machines with a fallback of 144, and newer machines with a fallback of 288, but sounds promising. -- 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] RFC: Operation Blockers in QEMU Block Nodes

2015-12-16 Thread Eric Blake
- > * This is still a "honor" system, in that each handler / job is > responsible for acquiring the NAC permission, and properly identifying > all nodes affected correctly by their operation. > > This should be done before any action is taken by the handler - t

Re: [Qemu-block] [Qemu-devel] [PATCH v3 04/11] fdc: add default drive type option

2015-12-16 Thread Eric Blake
acted on yet in favor of > making those changes a little more explicitly clear in standalone patches > later in this patch set. > > Signed-off-by: John Snow > --- > @@ -566,6 +569,7 @@ struct FDCtrl { > /* Timers state */ > uint8_t timer0; > uint8_t timer1;

Re: [Qemu-block] [Qemu-devel] [PATCH 1/1] HMP: Add equivalent to x-blockdev-change

2015-12-17 Thread Eric Blake
error_setg(&err, "One of -a or -d must be set"); Maybe s/One/Exactly one/ ? Limited in that we may eventually want to allow both add and delete at the same time; but HMP does not have hard-and-fast back-compat rules. So I'm fine with fixing the minor issues mentioned above, and

Re: [Qemu-block] [Qemu-devel] [PATCH 4/4] tests: Use Python 2.6 "except E as ..." syntax

2015-12-17 Thread Eric Blake
/qed.py| 2 +- > 2 files changed, 7 insertions(+), 7 deletions(-) Reviewed-by: Eric Blake -- 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 2/4] scripts/qmp: Use Python 2.6 "except E as ..." syntax

2015-12-17 Thread Eric Blake
/qmp/qmp| 4 ++-- > scripts/qmp/qmp-shell | 2 +- > scripts/qmp/qmp.py | 4 ++-- > 4 files changed, 6 insertions(+), 6 deletions(-) Reviewed-by: Eric Blake -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.

Re: [Qemu-block] [Qemu-devel] [PATCH 1/4] qapi: Use Python 2.6 "except E as ..." syntax

2015-12-17 Thread Eric Blake
off-by: Markus Armbruster > --- > scripts/qapi.py | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) Reviewed-by: Eric Blake -- 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 3/4] Revert "tracetool: use Python 2.4-compatible exception handling syntax"

2015-12-17 Thread Eric Blake
the reason why we can revert. Reviewed-by: Eric Blake -- 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 3/4] Revert "tracetool: use Python 2.4-compatible exception handling syntax"

2015-12-17 Thread Eric Blake
On 12/17/2015 11:37 AM, Markus Armbruster wrote: > Eric Blake writes: > >> On 12/17/2015 03:06 AM, Markus Armbruster wrote: >>> This reverts commit 662da3854e3f490223373b40afdcfcc339d14aa5. >>> >>> Signed-off-by: Markus Armbruster >>> --- >&g

Re: [Qemu-block] [Qemu-devel] [PATCH 1/4] qapi: Use Python 2.6 "except E as ..." syntax

2015-12-17 Thread Eric Blake
On 12/17/2015 11:35 AM, Markus Armbruster wrote: > Eric Blake writes: > >> On 12/17/2015 03:06 AM, Markus Armbruster wrote: >>> PEP 8 calls for it, because it's forward compatibile with Python 3. >> >> And possible as a cleanup only because we require 2.

Re: [Qemu-block] [Qemu-devel] [PATCH v3 01/11] fdc: move pick_geometry

2015-12-18 Thread Eric Blake
--- > 1 file changed, 45 insertions(+), 45 deletions(-) > Reviewed-by: Eric Blake -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature

Re: [Qemu-block] Jobs 2.0 QAPI [RFC]

2015-12-18 Thread Eric Blake
belongs to that Job's Subclass (e.g. > BlockJobOptions) but allow any field inside to be absent. > > Then we could have commands like this: > > job-set-option \ > arguments={ 'id': '#job789', > 'options': { > 'speed': 1 > } > } > > And so on. These would remain a flexible way of setting any various > post-launch options a job would need, and the relevant job-subsystem can > be responsible for rejecting certain options, etc. Keeping type-safety via a flat union may require it to look more like: job-set-option \ arguments={ 'id': '#job789', 'sys': 'block', 'speed': 1 } where the use of the 'sys' discriminator tells what other fields are being supplied, and we can avoid the "options":{} nesting. Then we'd need a sanity check in the code that the 'sys' requested by job-set-option matches the sys that owns '#job789', and fail if they differ. > > I believe that covers all the existing jobs interface in the QMP, and > that should be sufficiently vague and open-ended enough to behave well > with a generic jobs system if we roll one out in the future. > > Eric, Markus: Please inform me of all the myriad ways my fever dreams > for QAPI are wrong. :~) At this stage, your concepts seem reasonable, even if the concrete way of representing a subclass in qapi is not quite spelled out. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature

Re: [Qemu-block] [PATCH] qemu-iotests: Reduce racy output in 028

2015-12-18 Thread Eric Blake
On 12/11/2015 06:23 PM, John Snow wrote: > ping > > On 12/10/2015 10:27 PM, Eric Blake wrote: >> On my machine, './check -qcow2 028' was failing about 80% of the >> time, due to a race in how many times the repeated attempts >> to run 'info block

Re: [Qemu-block] Jobs 2.0 QAPI [RFC]

2015-12-18 Thread Eric Blake
u/ericb.git/shortlog/refs/heads/qapi has all my pending patches in the state last posted to the list, although it will be non-fast-forwarded as I rebase to latest (I already know that today's state of that branch doesn't build on today's qemu.git master, now that 2.6 is open and som

[Qemu-block] [PATCH v8 15/35] qom: Swap 'name' next to visitor in ObjectPropertyAccessor

2015-12-21 Thread Eric Blake
onst char *name, + (Object *obj, Visitor *v, const char *name, void *opaque, Error **errp) { ... } @@ identifier rule1.fn; expression obj, v, opaque, name, errp; @@ fn(obj, v, - opaque, name, + name, opaque, errp) Signed-off-by: Eric Blake --

[Qemu-block] [PATCH v8 14/35] qapi: Swap visit_* arguments for consistent 'name' placement

2015-12-21 Thread Eric Blake
type(V, NAME, OBJ, ARG1, ERR) | -visit_type_enum(V, OBJ, ARG1, ARG2, NAME, ERR) +visit_type_enum(V, NAME, OBJ, ARG1, ARG2, ERR) | -VISIT_TYPE(V, OBJ, NAME, ERR) +VISIT_TYPE(V, NAME, OBJ, ERR) ) Signed-off-by: Eric Blake --- v8: new patch --- backends/hostmem.c

[Qemu-block] [PATCH v2 14/14] qemu-img: Use new JSON output formatter

2015-12-21 Thread Eric Blake
-by: Eric Blake --- v2: Drop RFC, adjust expected output of 043; rebase to 'name' motion Overlaps with Fam's qemu-img edits, although he has expressed interest in getting this one in first. https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg01756.html https://lists.gnu.org/ar

[Qemu-block] [PATCH v2 01/14] qapi: Rename (one) qjson.h to qobject-json.h

2015-12-21 Thread Eric Blake
a QObject subtype. Kill trailing whitespace in the renamed tests/check-qobject-json.c to keep checkpatch.pl happy. Signed-off-by: Eric Blake Reviewed-by: Paolo Bonzini --- v2: retitle, enhance commit message, rebase to master --- MAINTAINERS | 2 +-

Re: [Qemu-block] [Qemu-devel] [PATCH 1/7] qom: add user_creatable_add & user_creatable_del methods

2015-12-22 Thread Eric Blake
-- > qom/object_interfaces.c | 76 > + > vl.c| 8 +++-- > 6 files changed, 127 insertions(+), 77 deletions(-) Looks like fairly straightforward code motion. Reviewed-by: Eric Blake -- Eric Blake eblake redhat com+1-919-3

Re: [Qemu-block] [Qemu-devel] [PATCH 2/7] qemu-img: add support for --object command line arg

2015-12-22 Thread Eric Blake
in first, I'll have to touch up your additions to match what I do elsewhere in my series. > @@ -319,6 +397,13 @@ static int img_create(int argc, char **argv) > case 'q': > quiet = true; > break; > +case OPTION_OBJECT: > +opts = qemu_opts_parse_noisily(qemu_find_opts("object"), > + optarg, true); > +if (!opts) { > +exit(1); Not for this patch, but maybe someday we should switch to exit(EXIT_FAILURE) throughout the file. -- 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 3/7] qemu-nbd: add support for --object command line arg

2015-12-22 Thread Eric Blake
ype} object class > + identified by @var{id}. See the @code{qemu(1)} manual > + page for full details of the properties supported. > + The only object type that it makes sense to define > + is the @code{secret} object, which is used to supply > + passwords and/or encryption keys.

Re: [Qemu-block] [Qemu-devel] [PATCH 4/7] qemu-io: add support for --object command line arg

2015-12-22 Thread Eric Blake
> + > +ov = opts_visitor_new(opts); > +pdict = qemu_opts_to_qdict(opts, NULL); > + > +visit_start_struct(opts_get_visitor(ov), &dummy, NULL, NULL, 0, &err); Same comments as on 3/7. We now have 5 very similar functions (hmp.c, vl.c, and your three additions); should

<    1   2   3   4   5   6   7   8   9   10   >