handler not found for "
> + "event type %s",
> +InputEventKind_lookup[event->kind]);
> +return;
Ouch. You can return mid-loop. I'd be more comfortable with a two-pass
algorithm (first pass ensures all list elements have a handler, second
actually calls qemu_input_event_send) or with a return that gives an
integer count of how many list items were processed.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
35 insertions(+), 24 deletions(-)
> create mode 100644 util/id.c
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
qapi-schema.json | 4 +++-
> qdev-monitor.c | 7 ++-
> qmp.c| 13 ++---
> 3 files changed, 19 insertions(+), 5 deletions(-)
>
Reviewed-by: Eric Blake
(would be nice if qmp-commands.hx had an example for
device-list-properties, but that can be a separate pa
++--
> tests/qemu-iotests/095.out | 16 --
> 6 files changed, 26 insertions(+), 68 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
put accordingly.
>
> Suggested-by: Eric Blake
Thanks for tackling this :)
> Signed-off-by: Max Reitz
> ---
> block/qcow2.c | 3 +++
> qapi/block-core.json | 6 +-
> tests/qemu-iotests/065 | 12 ++--
> tests/qemu-iotests/067.out | 10 +
es changed, 12 insertions(+)
>
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
+
> qmp-commands.hx | 63
> ++
> ui/input.c | 37 +
> 3 files changed, 117 insertions(+)
Reviewed-by: Eric Blake
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization l
't when unsigned is 64 bits. Purely theoretical; I'm not
> aware of such a system. Limit it to UINT32_MAX instead.
>
> Signed-off-by: Markus Armbruster
> ---
> hw/virtio/virtio-balloon.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Eric Blak
verState
> *chr, Error **errp)
>
> if (s->is_listen) {
> fd = socket_listen(s->addr, errp);
> -} else {
> +} else if (s->reconnect_time) {
> + fd = socket_connect(s->addr, errp, qemu_chr_socket_connected, chr);
> + return (f
n to JSON), then this is fine; otherwise, I'd
prefer that we use double quotes in QMP.
But if we decide single quotes are tolerable,
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
off-by: Markus Armbruster
> ---
> tests/drive_del-test.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
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
=> drive_del-test.c} (56%)
Reviewed-by: Eric Blake
> --- a/tests/qdev-monitor-test.c
> +++ b/tests/drive_del-test.c
> @@ -1,5 +1,5 @@
> /*
> - * qdev-monitor.c test cases
> + * blockdev.c test cases
> *
> * Copyright (C) 2013 Red Hat Inc.
Worth bumping this to i
On 10/02/2014 08:51 AM, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster
> ---
> tests/drive_del-test.c | 9 ++---
> 1 file changed, 2 insertions(+), 7 deletions(-)
Reviewed-by: Eric Blake
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization
> +
> +response = qmp("{'execute': 'human-monitor-command',"
> + " 'arguments': {"
> + " 'command-line': 'drive_add 0 if=none,id=drive0'"
> + "}}");
I&#x
n of single quotes in QMP, but it's existing style.
> +static void test_drive_del_device_del(void)
> +{
> +/* Start with a drive used by an device that unplugs instantaneously */
s/an device/a device/
New tests are always good; with the typo fix:
Reviewed-by: Eric Blake
--
Eric Blake ebla
ge
printed in check_report_connect_error, you are probably doing
translators a disservice by splitting a sentence into two parts
separated by a number of lines of code. (Spoken by one who has never
contributed a translation, so take with a grain of salt)
> return;
> }
>
> +s->connect_err_reported = 0;
s/0/false/
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
On 10/03/2014 11:47 AM, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert"
>
> Signed-off-by: Dr. David Alan Gilbert
> Reviewed-by: Eric Blake
> ---
> include/migration/migration.h | 1 +
> migration.c | 9 +
>
.hx | 2 --
> hmp.c| 8
> hmp.h| 1 -
> monitor.c| 7 ---
> qapi-schema.json | 11 ---
> qmp-commands.hx | 22 --
> vl.c | 5 -
> 7 files changed, 56 deletions(-)
Reviewed-by: Eric Blake
tag always be supplied, then we cannot create qcow2
files with a snapshot that lacks a tag using just QMP; but even if we do
that, we STILL have to support use of existing qcow2 files that were
created by earlier qemu and/or other tools that have snapshots without a
tag.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
nerate a tag. I'm
still on the fence on whether to always have a tag in place (whether by
QMP generation or by mandatory argument) vs. allowing anonymous
snapshots. But again, libvirt always names its snapshots, and I trust
your judgment as maintainer of this area of code in which way we should go.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
unless we set a coding standard in HACKING telling which
way new code should use. I can live with the churn in this commit as
long as you are changing other things in the same hunk at the same time,
but I wasn't specifically asking for capitalization changes.
--
Eric Blake eblake
'enum': 'SnapshotLookup',
'data': [ 'id-only', 'tag-only', 'default' ] }
{ 'command': 'vm-snapshot-load',
'data': { 'name': 'str', '*mode': 'SnapshotLookup' } }
where m
On 04/10/2013 06:40 AM, Luiz Capitulino wrote:
> On Wed, 10 Apr 2013 06:24:11 -0600
> Eric Blake wrote:
>
>>> - If you want to overwrite an existing snapshot, you could specify
>>> the 'id' or the 'name' argument or both of them and also yo
vior as the QMP command
Except that with HMP, arguments are positional. You can't provide 'id'
in isolation; by listing id:i? second, the parser requires that the
first argument encountered is a name.
I think a better interface might be:
HMP loadvm:
- args_type = "id:-
* qapi-schema.json: JSON doesn't allow trailing commas.
Signed-off-by: Eric Blake
---
a multi-line regex search for ',[ \n]*[]}]' didn't turn up any
other violations
qapi-schema.json | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/qapi-schema.json b/qa
Enabled by default (since 1.5)
> ##
> { 'enum': 'MigrationCapability',
> - 'data': ['xbzrle'] }
> + 'data': ['xbzrle', 'chunk_register_destination'] }
QMP prefers '-' over
#x27;command': 'migrate_check_for_zero', 'data': {'value': 'bool'} }
You can set the capability, but how do you query its current setting? I
dislike write-only interfaces.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
and link performing a worst-case stress test:
s/infinband/infiniband/
> +
> +RDMA Throughput With $ stress --vm-bytes 1024M --vm 1 --vm-keep
> +Approximately 30 gpbs (little better than the paper)
which paper? Call that out in your high-level summary
...
> +
> +An *exhaustive* paper (2010) shows additional performance details
> +linked on the QEMU wiki:
Missing the actual reference? And it would help to mention it at the
beginning of the file.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
On 04/11/2013 01:52 AM, Orit Wasserman wrote:
> On 04/11/2013 05:39 AM, Michael R. Hines wrote:
>> On 04/10/2013 10:26 PM, Eric Blake wrote:
>>>
>>> New QMP commands should be named with '-' rather than '_', as in
>>> 'migrate-chec
he behavior of management
apps written against the 1.3 interface when dealing with ImageInfo. If
you want to avoid a (sec/nsec) pair, you would have to invent a new type
instead of reusing the already-existing SnapshotInfo.
Hmm, as I typed that, I did another search of qemu-schema.json - we have
the type 'ImageInfo' defined, but none of the existing 'command's ever
call out the use of that type. Is it a type we are only using
internally to date, and where this is the first QMP command that would
actually expose SnapshotInfo or ImageInfo to a management app? Then
maybe we _CAN_ modify SnapshotInfo, clean up all the internal code, and
provide a saner type for the first time that QMP can actually use it.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
5
> +##
> +{ 'command': 'query-mac-table', 'returns': ['MacTableInfo'] }
Seems useful.
> +-> { "execute": "query-mac-table"}
> +<- {"return": [
> +{
> +"name": "virtio-net-pci.0",
> +&qu
to rush it into 1.5 and regret the design, and no
need to hold up getting RDMA into 1.5 just because of a debate about a
knob that can be deferred to a later release when we've had more time to
play with RDMA.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
but that sounds like it
is making the internal state use a sane default based on existing
external interface, and should be fine.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
nging that output to delete a field might break existing tools;
adding a field might be safer, but then you'd have redundant information
in the old style for old tools, as well as the added field.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
priate, but does that mean we have to reject the
file completely if it wasn't sized to a multiple to begin with?
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
ve file=TEST_DIR/t.qcow2,file.driver=qcow2: The 'qcow2' block
> driver is not suitable for the bottom level
> +qemu: -drive file=TEST_DIR/t.qcow2,file.driver=qcow2: could not open disk
> image TEST_DIR/t.qcow2: Invalid argument
Maybe a better error message would be this?
A
On 04/15/2013 06:10 AM, Kevin Wolf wrote:
> Am 11.04.2013 um 11:20 hat Markus Armbruster geschrieben:
>> [Cc: Wenchao Xia because of overlap with his work]
>>
>> Eric Blake writes:
>>
>>> On 04/10/2013 08:05 AM, Pavel Hrdina wrote:
>>>> Her
e converted image still
reports that the guest sees a size of 512, to prove that the virtual
size was not expanded as a result of compression tail padding?
With that question answered, you have:
Series: 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
+++
> block/mirror.c| 2 +-
> include/block/block.h | 2 +-
> 3 files changed, 25 insertions(+), 6 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
--
> 1 file changed, 27 insertions(+), 4 deletions(-)
Quite a few lines added for a net reduction in parameters, but the
results look nice.
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
l, so I have no qualms with adding this to a fixed version:
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
e from 4/15. Same caveat about it
being trivial enough to add this once fixed:
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
On 04/12/2013 02:47 PM, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf
> ---
> block/blkdebug.c | 103
> +--
> 1 file changed, 77 insertions(+), 26 deletions(-)
>
Reviewed-by: Eric Blake
--
Eric Blake eblake redhat com
eturn;
> }
> -filename += strlen("blkverify:");
>
> /* Parse the raw image filename */
> c = strchr(filename, ':');
> if (c == NULL) {
> -return -EINVAL;
> +error_setg(errp, "blkdebug requires raw copy and origina
efer indenting the entire BlockDriver
initialization consistently, as done in this patch, but didn't ding
patch 6 or 7 at the time because they looked innocuous enough in isolation).
Since I only called out whitespace issues (and even then, only for
commits other than this) and a weakness in the commit message, the code
itself deserves:
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
code, should you re-indent the second
line of this signature?
> +
> +filename = qemu_opt_get(opts, "filename");
> +
>
> s->glfs = qemu_gluster_init(gconf, filename);
Why the two blank lines?
Comments deal only with whitespace, so feel free to add:
Reviewed
by: Kevin Wolf
> ---
> block/iscsi.c | 39 +--
> 1 file changed, 37 insertions(+), 2 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
by: Kevin Wolf
> ---
> block/rbd.c | 31 ++-
> 1 file changed, 30 insertions(+), 1 deletion(-)
>
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
by: Kevin Wolf
> ---
> block/sheepdog.c | 32 +++-
> 1 file changed, 31 insertions(+), 1 deletion(-)
>
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
quot;,
> .instance_size = sizeof(BDRVVVFATState),
> -.bdrv_file_open = vvfat_open,
> -.bdrv_rebind = vvfat_rebind,
> +.bdrv_parse_filename= vvfat_parse_filename,
> +.bdrv_file_open = vvfat_open,
> +.bdrv_rebind= vvfat_rebind,
> .
e to avoid the churn of touching it up twice).
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
eview that, as
libvirt is still reluctant to use fdsets for image manipulation on the
command line if we can't follow it up with hot-plug manipulation, but I
love where this is heading.
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
On 04/15/2013 02:47 PM, Laszlo Ersek wrote:
> From: Michael S. Tsirkin
>
>
> Signed-off-by: Laszlo Ersek
If Michael is the original author, don't we need his S-o-b?
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
acpi.c
Sorry - I just did a superficial review of the commit message, but this
is in code that I'm not familiar enough with to do any sort of formal
review.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
m/tpm_passthrough.c | 74
> +++---
> 1 file changed, 17 insertions(+), 57 deletions(-)
Reviewed-by: Eric Blake
and matches a counterpart change you proposed for libvirt.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.
t; +
Should be usable from libvirt's perspective, even if hot-plugging more
than one cpu requires more than one QMP call. Do we have a counterpart
QMP call to easily determine which cpu ids can still be hotplugged? If
so, should we mention that in the documentation of this command?
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
yping:
_filter_qemu()
{
sed "s#^${QEMU_PROG##*/}:#QEMU_PROG#"
}
That said, your more verbose version is still functionally correct, so
you can add:
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
"str": "01:80:c2:00:00:21",
> "str": "00:00:00:00:00:00"
> ]
No. This is not valid JSON. ':' is only allowed inside {}. The point
of an array is to either have an array of objects (as in
roublesome code...]
> #pragma GCC diagnostic pop
Older gcc does not understand #pragma GCC diagnostic push. Are you sure
this solution works with all versions of gcc in common use for compiling
qemu?
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library
7;chunk_register_destination'] }
>
> x-chunk_register_destination.
>
> Or better, "x-rdma_chunk_registration".
Or better, "x-rdma-chunk-registration", since QMP should prefer the use
of '-' over '_'.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
that anyone will run
the testsuite while trying to stress extreme corner-case $QEMU_PROG
naming conventions, I can certainly live with keeping $(basename
$QEMU_PROG) form for legibility.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
19 insertions(+), 19 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
input parsing is not touched, to make it easier in review.
>
> Signed-off-by: Wenchao Xia
> ---
> blockdev.c | 146 +--
> 1 files changed, 82 insertions(+), 64 deletions(-)
>
--
Eric Blake eblake redhat com+
On 04/13/2013 05:11 AM, Wenchao Xia wrote:
> The code is simply moved into a separate function.
>
> Signed-off-by: Wenchao Xia
> ---
> blockdev.c | 20 +---
> 1 files changed, 13 insertions(+), 7 deletions(-)
Reviewed-by: Eric Blake
--
Eric Blake
On 04/13/2013 05:11 AM, Wenchao Xia wrote:
In the subject: s/rolling back/rollback/
> Signed-off-by: Wenchao Xia
> ---
> blockdev.c | 12 +---
> 1 files changed, 9 insertions(+), 3 deletions(-)
With the subject fix, feel free to add:
Reviewed-by: Eric Blake
--
> +typedef struct BdrvActionOps {
> +/* Prepare the work, must NOT be NULL. */
> +int (*prepare)(BlockdevAction *action, void **p_opaque, Error **errp);
Based on the comments on 1/5, should prepare have a void signature and
just let errp serve to indicate failure?
--
Eric Blake ebl
e | 2 +
> tests/i440fx-test.c | 148
>
> 2 files changed, 150 insertions(+)
> create mode 100644 tests/i440fx-test.c
>
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
On 04/16/2013 09:56 AM, Eric Blake wrote:
> On 04/16/2013 08:45 AM, Anthony Liguori wrote:
>> This test compares all of the default register values against the
>> spec. It turns out we deviate in quite a few places. These
>> places are really only visible to the BIOS though
50 ---
> block/sheepdog.c | 61
> hmp-commands.hx | 48 +++---
> hmp.c | 119 +++
> hmp.h | 3 +
> include/block/block.h | 17 ++-
> include/block/block_int.h | 17 ++-
> include/sysemu/sysemu.h | 12 +-
> migration.c | 15 +-
> monitor.c | 12 --
> qapi-schema.json | 54 +++
> qemu-img.c| 54 ---
> qmp-commands.hx | 127 +++-
> savevm.c | 363
> +-
> vl.c | 7 +-
> 18 files changed, 782 insertions(+), 364 deletions(-)
>
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
, 11 insertions(+), 6 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
's probably better done as a
separate commit.
As my findings are best addressed in other patches, I'm okay with this
patch as-is, and you can use:
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
i, ret, available;
> +int nb_sns, i, available;
> int total;
> int *available_snapshots;
> char buf[256];
> @@ -2505,8 +2532,8 @@ void do_info_snapshots(Monitor *mon, const QDict *qdict)
>
> while ((bs1 = bdrv_next(bs1))) {
> if (b
to succeed with no data, you still have to return {}.
Maybe I'm not familiar enough with QMP, and you can get away with this
after all. On the other hand, I'd much rather have error reporting here
anyways, since libvirt will be using QMP, and hiding the error to only
occur in HMP means t
evant (and changing from -1 to 0 makes no
difference to this code doing a short-circuit). But again, calling that
out as a contract, where we can verify that each implementation of the
callback function obeys the contract, would make me feel a bit better.
> +++ b/savevm.c
> @@ -2206,7 +2206,7 @@ static int bdrv_snapshot_find(BlockDriverState *bs,
> QEMUSnapshotInfo *sn_info,
> return found;
> }
>
> -nb_sns = bdrv_snapshot_list(bs, &sn_tab);
> +nb_sns = bdrv_snapshot_list(bs, &sn_tab, NULL);
> if (nb_sns < 0) {
> return found;
Oops. qemu-img wasn't the only caller. Here, changing -1 to 0 on error
return means we fall through instead of returning 0 early. Did you mean
for that to happen? Calling with NULL says you don't care about error
reporting - is that right?
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
(errp, "saveVM v2 format is obsolete and don't work
> anymore");
As long as you are touching this, fix the grammar:
s/don't/doesn't/
As my findings were pre-existing problems:
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
Management apps like libvirt don't know to pay attention to
stderr unless there is a non-zero exit status.
* migration.c (process_incoming_migration_co): Exit with non-zero
status on failure.
Signed-off-by: Eric Blake
---
Noticed while reviewing:
https://lists.gnu.org/archive/html/qemu-
p code later, besides qemu-img code.
Not a full review (yet), but can we get this particular cleanup rebased
to be done first in isolation, since Pavel's patches would benefit from
being based on top of this cleanup as well?
https://lists.gnu.org/archive/html/qemu-devel/2013-04/msg03287.html
--
Er
port("Device '%s' does not have the requested snapshot
> '%s'",
> - bdrv_get_device_name(bs), name);
> -return -ENOENT;
> +if (!bdrv_snapshot_find(bs, &sn, name, id, false)) {
> +retur
ondition is probably dead code, if the QMP command never
attempts to generate id_str itself.
At any rate, the conversion to errp looks reasonable.
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
; 3 files changed, 26 insertions(+), 25 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
ed series containing both of your improvements as a
single series, so that we are only changing bdrv_snapshot_list semantics
once.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
t; - 'data': ['xbzrle'] }
> + 'data': ['xbzrle', 'x-chunk-register-destination'] }
But given the x- prefix, which is already a designation that the option
is experimental and may be pulled, I can live without documentation here
(where JUST t
ng RDMA.
> +3. Also, some form of balloon-device usage tracking would also
> + help alleviate some issues.
Maybe also mention that the protocol will be renamed from -incoming
x-rdma:... to -incoming rdma:... when it has been through more testing
to stabilize the protocol.
--
Eric Blake
the idea of find() setting errp on lookup failure. Callers can
ignore errors in situations where a failed lookup triggers a reasonable
fallback, but in case the caller wants to report an error, the lower
down that we generate an error message, the more likely the error
message is to contain fu
On 04/17/2013 12:14 PM, Eric Blake wrote:
> Or, written another way, to implement the same results in only two coded
> loops:
Missed a line:
>
> if name is set:
> if there is a snapshot with that name (loop 1):
> if id is set:
>
On 04/16/2013 07:09 PM, Eric Blake wrote:
> On 04/13/2013 02:56 AM, Wenchao Xia wrote:
>> To make it clear about id and name in searching, the API is changed
>> a bit to distinguish them, and caller can choose to search by id or name.
>> Searching will be done with higher
>
> Signed-off-by: Wenchao Xia
> Reviewed-by: Eric Blake
> Reviewed-by: Kevin Wolf
> ---
> +
> +/* Check logic is connected with load_vmstate():
> + Only check the devices that can snapshot, other devices that can't
> + take snapshot, fo
et of/
s/got/listed/
> in "qemu-img info" as static snapshot info, and this patch clearly
> tips it.
s/tips/identifies/
>
> Signed-off-by: Wenchao Xia
> ---
> block/qapi.c | 13 -
> 1 files changed, 12 insertions(+), 1 deletions(-)
Reviewed-by: Er
goto err;
But then this caller would need to pass in a local error parameter,
where Pavel's idea of qemu_img_handle_error would make it easier.
https://lists.gnu.org/archive/html/qemu-devel/2013-04/msg03288.html
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
of all snapshots
Does it really share the same snapshot ID across all block devices, or
is it just the snapshot tag that gets shared? That is, if I have just
disk a, take one snapshot, then hotplug disk b, then take another
snapshot, doesn't that leave me with:
disk a: id 1 tag A, id 2 tag B
es, I think libvirt will be able to drive the new command by knowing
how many heads appear per device, then passing in the appropriate named
device to the QMP command. And yes, I'll review patch 5 regarding
interface design.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virt
>file) {
>> +bdrv_snapshot_delete(bs->file, snapshot_id, errp);
>> +} else {
>> + error_setg(errp, "snapshots are not supported on device '%s'",
>> + bdrv_get_device_name(bs));
>
> Same thing with the device name here.
Same thing about this function being reached via vm-snapshot-delete
where we aren't passing in a device name, so knowing which device failed
is important.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
e new command, if not, it
gives the user a nice error unless screen 0 was selected.
Option 2 is probably slightly nicer for guaranteeing a sane error
message back to the user, but option 1 (the approach of this series)
still seems workable.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
l incorretly be:
>
>{ "timestamp": { "seconds": 1366228965, "microseconds": 245466 },
> "event": "BALLOON_CHANGE", "data": { "actual": 5368709120 } }
>
> To fix it this commit casts it to ram_addr_t, which is ram_siz
On 04/18/2013 03:09 PM, Richard W.M. Jones wrote:
> From: "Richard W.M. Jones"
>
> ---
> block/ssh.c | 6 ++
> 1 file changed, 2 insertions(+), 4 deletions(-)
Reviewed-by: Eric Blake
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization l
on demand' or
'all at once', just that I'm supposed to read rdma.txt to decide if I
want to move away from the default.
/me reads patch 11 again... and wonders why the docs came last instead
of first in the series...
I guess the opposite sense could be named 'x-rdm
#x27; minds
during the rest of the series to validate that the implementation
matches docs. On later review (such as git bisect landing here), it
means the docs are in-tree for any other commit that references them.
If it were me, I'd rebase things to put docs in patch 1 on the v5 ser
On 04/16/2013 07:51 AM, Lluís Vilanova wrote:
> Adds the "input(...)" primitive to the syntax of QAPI schema files.
Interesting idea, but isn't "include(...)" a more common name than
"input(...)"?
--
Eric Blake eblake redhat com+1-919-301-32
> +
> +input("instrument/qapi-schema.json")
See my comments on 13/23 about possibly naming this include() instead of
input().
> +Example:
> +
> +-> { "execute": "instr-load", "arguments": { "path":
> "/tmp/libtrace-instr
ot;instr", "QEMU_INSTR", true, handle_arg_instrument,
> + "path", "load a dynamic trace instrumentation library"},
> +#endif
> +#if defined(CONFIG_TRACE_INSTRUMENT)
> +{"instr-arg", "QEMU_INSTR_ARGS", true, handle_a
801 - 900 of 23401 matches
Mail list logo
