s cached , if not cache the page.
s/ ,/,/
> +} else if (migrate_use_xbzrle() && stage != 3) {
> +/* In stage 1 we only cache the pages before sending them
> + from the cache (uncompressed).
> + We doen't use compression for stage 3.
round down to the nearest power of 2.
s/round/rounded/
> +
> +Set cache size to be used by XBZRLE migration, the cache size will be round
> down
s/round/rounded/
> +to the nearset power of 2
s/nearset/nearest/
> +
> +Arguments:
> +
> +- "value": cache
is active:
> +
> +-> { "execute": "query-migrate" }
> +<- {
> + "return":{
> + "status":"active",
> + "ram":{
> +"total":1057024,
> +"remaining":1053304,
> +"transferred":3720
> + },
Where's the capabilities member?
> + "cache":{
> + "size": 1024
No TABs.
> +"xbzrle_transferred":20971520,
Especially not when you mix TAB and space indentation in the same example.
> + "xbzrle_pages":2444343,
> + "xbzrle_cache_miss:2244,
s/miss:/miss"/
> + "xbzrle_overflow":34434
> + }
> + }
> + }
> +
> EQMP
>
> {
>
--
Eric Blake ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
eeping it at /dev/fd/21 could be done via 'pass-fd -f drive-virtio1',
where -f is an optional bool parameter to force a reassociation of a
given name back to the previously assigned value instead of the normal
error path for accidentally passing an fd to an already in-use name.
--
rguments:
> +
> +keys array:
> +- "key": key sequence (json-string)
I'm not sure if json-string is the right designation any more. Rather,
it is a json-array of key enum values.
> +
> +- hold-time: time to delay key up events, milliseconds (josn-int, optional)
27;. The fd is created as a
> result of passing via SCM_RIGHTS, which assigns the next available fd.
> We don't have control over what fd is assigned, do we?
The use of -f says that 'pass-fd' uses dup2() to reuse an existing fd.
--
Eric Blake ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
t only exists on Linux and
Cygwin. Does this need to have conditional code to allow compilation on
BSD, such as:
#ifndef MSG_CMSG_CLOEXEC
# define MSG_CMSG_CLOEXEC 0
#endif
as well as fallback code that sets FD_CLOEXEC manually via fcntl() when
MSG_CMSG_CLOEXEC is missing?
--
Eric Blake eb
to worry about that aspect of O_ACCMODE
here.]
> +errno = EACCES;
> + return -1;
> +}
> +
> +if (fcntl_setfl(fd, O_CLOEXEC, 1) < 0) {
Again, broken. Besides, why are you attempting it both here and in
qemu_dup()? Shouldn't once be enough?
> +return -1;
> +}
> +
> +return qemu_dup(fd, flags);
--
Eric Blake ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
ou want to use dup3(fd, monfd->fd, O_CLOEXEC) (and fall back
to dup2()+fcntl(F_GETFD/F_SETFD) on platforms where dup3 is not
available; it has been proposed for the next revision of POSIX but right
now is pretty much limited to Linux and Cygwin). Otherwise, you are
undoing the fact that patch
included, macros with the
prefixes shown may be defined. After the last inclusion of a given
header, an application may use identifiers with the corresponding
prefixes for its own purpose, provided their use is preceded by a #undef
of the corresponding macro.
...
PRI[Xa-z], SCN[Xa-z]
--
Eric Bl
tionStats containing detailed migration status,
>> -# only returned if status is 'active'
>> +# @ram: #optional @MigrationStats containing detailed migration
>> +# status, only returned if status is 'active' or
>> +# 'completed'.
e only
other flags in your list not supported by Linux are O_LARGEFILE (which I
said was pointless), O_NOCTTY (which only has an impact at open() and
not later on, so it is not worth worrying about), and O_SYNC (so for
that one, you should error out if not set correctly, as the difference
between O
en on Linux.
Agreed. Furthermore, since dup3() has been proposed for addition into
POSIX[1], it won't be long before other platforms add it. Always favor
feature checks (a configure probe for dup3) over platform checks
(hard-coding the assumption that Linux and Cygwin are the only platfo
def Eric had pointed out in v1.
Agreed - no POSIX collision in this series.
--
Eric Blake ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
er
will add the diffstat. Why 'git send-email --help' doesn't list the
fact that it inherits useful options from 'git format-patch' is beyond
me, but I long ago learned to use --cover-letter instead of --compose to
get nicer results.
--
Eric Blake ebl...@redhat.co
-local naming used in
the previous command), but the fd is again used atomically
Under proposal 3, there is no need to dup fd's, merely only to check
that qemu_open("/dev/fdlist/n", flags) has compatible flags with the fd
received via FDSET. And the fact that things are made atomic m
, you have then
missed triggering the assert and proceed to do more unefined behavior
with a negative shift. You can solve that, and use one less conjunct,
by using:
assert(start >= 0 && length > 0 && (unsigned) start + length <= 64);
Same comment for field32().
--
E
with '-' rather than squasheverythingtogether?
>>
>> Yes, that's right.
>
> still use sendkey for hmp ?
Libvirt won't care if you rename the hmp command, once the QMP command
is in place. However, for back-compat, since hmp already had 'sendkey',
it
by length, although I
could see cases of intentionally wanting to pass in -1 to set all bits
within the mask and a tight assert would prevent that usage.
You marked this function signature as returning uint64_t, but didn't
return anything. I think the logical return is the new contents of
*pva
led it out as 'parameters' instead of 'params',
but I still think 'capabilities' would be a more consistent name for the
array.
> +SQMP
> +migrate_set_parameters
> +---
> +
> +Enable migration capabilities
Thanks to the state portion of each MigrationCapabilityInfo, this
command can actually enable _or disable_ migration capabilities.
--
Eric Blake ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
01 02 03 04 05 06 00 08 09 0a 0b 0c
2984 zeros
new buffer, 4096 bytes:
1100 zeros
02 03 04 04 05 06 07 08 00 00 09 0a 00 0b 0c 0d
2980 zeros
one possible encoded buffer, 18 bytes:
cc 08 03 02 03 04 03 0a 07 08 00 00 09 0a 00 0b 0c 0d
--
Eric Blake ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
ng-down code, and call it from multiple places
prior to checking for size equality.
> +/* move all data from old cache */
> +for (i = 0; i < cache->max_num_items; i++) {
> +old_it = &cache->page_cache[i];
> +if (old_it->it_addr != -1) {
> + /* check for
estriction that you always provide an
additional argument beyond the format string, then life is much simpler
for complying with C99:
#define DPRINTF(fmt, ...) \
do { fprintf(stdout, "prefix: " fmt, __VA_ARGS__); } while (0)
but it means you can't use DPRINTF("string&q
place where a valid stream has at least two bytes (the
smallest possible nzrun is exactly two bytes, 1 for the length, and 1
byte of data). I would replace this with:
/* invalid input, since an nzrun must have data */
if (i >= slen - 1) {
return -1;
}
> +
> +/* nzrun */
>
t; +} else if (flags & RAM_SAVE_FLAG_XBZRLE) {
We should probably be erroring out if we detect this incoming flag, but
the management app explicitly disabled the xbzrle migration capability,
since that represents a case of user error, and failing the migration is
better than proceeding to decode things anyway.
--
Eric Blake ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
@value: cache size in bytes
> +#
> +# The size will be round down to the nearest power of 2.
s/round/rounded/
> +Example:
> +
> +-> { "execute": "migrate_set_cachesize", "arguments": { "value": 512M } }
512M is not a json-int. I really don't think you even saw my v12
comments on this patch.
--
Eric Blake ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
5. Migration is being performed and XBZRLE is active:
> +
> +-> { "execute": "query-migrate" }
> +<- {
> + "return":{
> + "status":"active",
> + "params" : { "xbzrle" : "on" },
true rather than "on"
> + "ram":{
> +"total":1057024,
> +"remaining":1053304,
> +"transferred":3720
> + },
> + "cache":{
> +"cache-size": 1024
Missing a trailing comma
> +"xbzrle-transferred":20971520,
> +"xbzrle-pages":2444343,
> +"xbzrle-cache-miss":2244,
> +"xbzrle-overflow":34434
> + }
> + }
> + }
> +
> EQMP
>
> {
>
--
Eric Blake ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
On 06/28/2012 06:06 AM, Max Filippov wrote:
> On Wed, Jun 27, 2012 at 8:55 PM, Eric Blake wrote:
>> On 06/27/2012 04:34 AM, Orit Wasserman wrote:
>
>> if (!is_power_of_2(num_pages)) {
>>num_pages |= num_pages >> 1;
>>num_pages |= num_pages >
Signed-off-by: Peter Maydell
Reviewed-by: Eric Blake
--
Eric Blake ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
gt; qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
>
> This hunk should have been squashed into 9/13.
>
--
Eric Blake ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
ptional 'nfds' JSON argument for atomic management of fd passing?
Which commands need to reopen a file with different access, and do we
bite the bullet to special case all of those commands to allow fd
passing or can we make qemu_open() coupled with high-level fd passing
generic enough to satisf
mu closes the set of fds when the timer
> expires
>B) If @close == false, libvirt must issue close-fds command to close
> the set of fds
>
> *How to solve this has yet to be determined. Kevin mentioned
> potentially using a bottom-half or a timer in qemu_close
s" } =>
{ "return" : { "sets": [
{ "name": "fdset1",
"fds": [ { "fd": 4, "monitor": true, "refcount": 1 } ] },
{ "name": "fdset2",
"fds": [ { "fd": 5, "m
s are still in use by qemu, even though they are no longer
available to 'remove-fd' from the monitor, and if the monitor is worried
about keeping the fd set alive it can call 'add-fd' to again associate a
new fd with the set. The lifetime of a set is thus as long as any of
its associated fds have a non-zero refcount.
--
Eric Blake ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
then
the enabled parameter becomes optional on input (it should still always
be present on query-migration-capabilities output, though).
> +migrate_set_parameters
> +---
> +
> +Enable/Disable migration capabilities
> +
> +- "xbzrle": xbzrle support
> +
> +Arguments
mit 23 bytes
e8 07 14 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f 00 00 67 00 69
list every single zrun (except trailing), even if only one byte long,
transmit 24 bytes
e8 07 0f 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f 02 01 67 01 01 69
avoid a 1-byte zrun, transmit 23 bytes
e8 07 0f 01 02 03
lue)
{
if (!is_power_of_2(value)) {
value = 0x8000ULL >> clz64(value);
}
return value;
}
since clz64() is already part of qemu sources.
--
Eric Blake ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
t_addr != -1) {
> +/* check for collision , if there is, keep the first value */
s/ ,/,/ (no space before comma in English)
Shouldn't we instead prefer to keep the page with larger age, regardless
of whether we found it first or second? That is, a cache is more likely
to b
Furthermore, hasn't this
patch (and several others) were already picked up by Juan; you should
rebase on top of his pull request.
--
Eric Blake ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
d decoding */
No, if we got here, then we were handed an invalid stream. Remember,
the protocol says that every zrun must be followed by an nzrun.
> +if (i == slen - 1) {
> +return d;
> +}
> +
> +/* nzrun */
> +if ((slen - i) < 2 && *(src
On 07/03/2012 03:32 PM, Eric Blake wrote:
>> +ret = uleb128_decode_small(src + i, &count);
>> +if (ret < 0) {
>
> An nzrun should be a non-zero value; I'd write this as (ret <= 0) to
> rule out an attempt to pass a zero-length nzrun.
Corre
On 07/03/2012 03:39 PM, Eric Blake wrote:
> On 07/03/2012 03:32 PM, Eric Blake wrote:
>
>>> +ret = uleb128_decode_small(src + i, &count);
>>> +if (ret < 0) {
>>
>> An nzrun should be a non-zero value; I'd write this as (ret <=
ot;active",
> + "params" : { "xbzrle" : "off" },
neither does this. Should be:
"capabilities": [ { "capability":"xbzrle", "state":false } ],
> @@ -2136,6 +2147,7 @@ Examples:
> <- {
>"return":{
>
On 07/03/2012 11:15 PM, Shrinidhi Joshi wrote:
> On Saturday 16 June 2012 03:41 AM, Eric Blake wrote:
>> On 06/15/2012 02:48 PM, Supriya Kannery wrote:
>> > raw-posix driver changes for bdrv_reopen_xx functions to
>> > safely reopen image files. Reopening of imag
s (which probably picked up a couple
other fixes in the process):
https://lists.gnu.org/archive/html/qemu-devel/2012-07/msg00369.html
--
Eric Blake ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
atform for flushing out these sorts of type
discrepancies (since cygwin inherits from newlib).
http://sourceware.org/cgi-bin/cvsweb.cgi/src/newlib/libc/include/stdint.h.diff?r1=1.8&r2=1.9&cvsroot=src&f=h
--
Eric Blake ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
>
>>>> qmp-command.h:
> void qmp_sendkey(KeyCodesList * keys, bool has_hold_time, int64_t
> hold_time, Error **errp);
>
> This patch makes qapi can generate List struct for enum.
Grammar nit:
s/makes qapi can/lets/qapi/
--
Eric Blake ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f 68 00 00 67 00 69
+3074 zeros
encoded buffer:
--
Eric Blake ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
opers.
> Signed-off-by: Benoit Hudzia
> Signed-off-by: Petter Svard
> Signed-off-by: Aidan Shribman
> Signed-off-by: Orit Wasserman
I think I touched this one heavily enough that it warrants adding:
Signed-off-by: Eric Blake
Other than the commit message, I'm happy with this pat
mentioned in my v13 review changing this to:
s/Unmodifed page skipping/Skipping unmodified page/
it looks like you didn't pick up my v13 comments yet (and my fault for
not complaining at v14). As a result, I stopped reviewing here.
--
Eric Blake ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
THROTTLE (32 << 20) /* Migration speed throttling */
>
> -/* Migration XBZRLE cache size */
> +/* Migration XBZRLE default cache size */
Again from my v12 review, this hunk should be squashed into the patch
that first introduces the comment.
> +migrate_set_cachesize
> +-
+# Since: 0.14.0, 'capabilities' and 'cache' since 1.
s/1.$/1.2/
> +5. Migration is being performed and XBZRLE is active:
> +
> +-> { "execute": "query-migrate" }
> +<- {
> + "return":{
> + "status":"active",
> + "capabilities" : [ { "capability": "xbzrle", "state" : true } ],
> + "ram":{
> +"total":1057024,
> +"remaining":1053304,
> +"transferred":3720
s/$/,/
--
Eric Blake ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
nitely agreeing that tying the refcount to the set rather
than to individual fds within the set makes sense; you still avoid the
fd leak in that all fds in the set are closed when both the monitor has
disavowed the set and when qemu_close() has finished using any of the fds.
--
Eric Blake ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
ncern (that is, if you are willing to state that use
of newer qemu is intended to be coupled with newer libvirt that has been
taught to use 'help' instead of '?'), then this patch is probably fine.
The alternative is to update 'qemu -h' output to mention both forms,
rather than completely eradicating the ? form from documentation.
--
Eric Blake ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
On 07/09/2012 06:10 AM, Peter Maydell wrote:
> On 9 July 2012 13:07, Eric Blake wrote:
>> That is, we are filtering based on the explicit presence of a literal
>> '?' in the help output to determine whether we can further filter based
>> on '-device device,
On 07/09/2012 06:10 AM, Alexander Graf wrote:
>
> This fixes a lot of subtle breakage in the GNU toolchain (libgmp) which
> hicks up on QEMU's non-existent CPU models.
s/hicks up/hiccups/
>
> v2 -> v3:
>
> - fix typo in commit message
but not all of
On 07/09/2012 07:48 AM, Alexander Graf wrote:
>
> On 09.07.2012, at 15:47, Eric Blake wrote:
>
>> On 07/09/2012 06:10 AM, Alexander Graf wrote:
>>
>>>
>>> This fixes a lot of subtle breakage in the GNU toolchain (libgmp) which
>>> hicks up on
can only be performed if you are also
doing optimization. You have to use a tool like clang or Coverity if
you want more reliable uninitialized-use analysis even while building
-O0 debug images.
--
Eric Blake ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
towards a 'pass-fd'
solution instead of a hook fd solution is that libvirt would have less
code to write to get it working. But it was originally Dan's complaint
that an rpc solution has too much risk for deadlock or leaks; if we are
confident that we can avoid deadlock, and that the idea of passing in
fds in response to an rpc involves less bookkeeping and speculation on
libvirt's part about which monitor commands will require opening an fd,
then maybe it really is a better technical solution, even if it costs
more libvirt code to implement.
--
Eric Blake ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
size
...new QMP commands should prefer '-' over '_'. While the HMP version
is fine with migrate_set_cachesize, the QMP command should be
migrate-set-cachesize (or even 'migrate-set-cache-size').
> +
> +Set cache size to be used by XBZRLE migration, the cac
ince: 1.1.3
Likewise for 1.2.
> +
> +- "Dimm": Dimm name (json-str)
> +- "request": type of hot request: hot-add or hot-remove (json-str)
> +- "result": result of the hotplug request for this Dimm success or failure
> (json-str)
This may need tweaks (such as s/Dimm/dimm/) based on resolution of above
comments.
--
Eric Blake ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
y', especially if we merge
balloon and hotplug information into one command.
> +#
> +# Returns total memory in bytes, including hotplugged dimms
> +#
> +# Returns: a l
truncated
--
Eric Blake ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
5,8 +280,9 @@
>> # 'cancelled'. If this field is not returned, no migration process
Did we ever decide whether to favor US (canceled) or UK (cancelled)?
>> # has been initiated
>> #
>> -# @ram: #optional @MigrationStats containing detailed migration status,
>> -# only returned if status is 'active'
>> +# @ram: #optional @MigrationStats containing detailed migration
>> +# status, only returned if status is 'active' or
>> +# 'completed'. 'comppleted' (since 1.2)
s/comppleted/completed/
>> #
>> # @disk: #optional @MigrationStats containing detailed disk migration
>> #status, only returned if status is 'active' and it is a block
>
>
>
--
Eric Blake ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
ef
>> -find . -name '*.[od]' -exec rm -f {} +
>> +find . -name '*.[od]' -type f -exec rm -f {} +
>
> What does this change have to do with this patch?
Because this patch introduces a directory
scripts/qemu-guest-agent/fsfreeze-hook.d/mysql-flush.sh.sample that
belongs to the repo and therefore must not be cleaned.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
but I won't insist.
> +# for InnoDB, wait until every log is flushed
> +INNODB_STATUS=$(mktemp /tmp/mysql-flush.XX)
> +[ $? -ne 0 ] && exit 2
> +trap "rm -f $INNODB_STATUS" SIGINT
POSIX says that 'trap foo INT' is required to work, but 'trap foo
SIGINT' is optional. Also, shouldn't you also worry about HUP, ALRM,
and TERM?
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
we should store them under some other
file name and only at 'make install' time insert them into a .d at the
install destination. It would clean up this confusion about the
.gitignore as well as the change to the 'find' command during 'make clean'.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
dex = physical_cluster_index / l2_dedup_cluster_entries
> +
> +The index in the level 2 deduplication table is:
> +l2_dedup_index = physical_cluster_index % l2_dedup_cluster_entries
> +
> == Host cluster management ==
>
> qcow2 manages the allocation of host clusters by main
passing
an error message through to the user. I didn't spot any issues with the
changes in this series, although I admit my testing was rather light.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
upplying an actual file name. Does your implementation
allow for pipes, or must it be seekable?
> +#
> +# @format: format of the backup file
Rather than letting this be a free-form string, it would be nicer as an
enum of actually supported formats.
> +#
> +# @config-filename: #optional
+++
> 1 file changed, 26 insertions(+)
Reviewed-by: Eric Blake
>
> diff --git a/host-utils.h b/host-utils.h
> index 821db93..231d580 100644
> --- a/host-utils.h
> +++ b/host-utils.h
> @@ -24,6 +24,7 @@
> */
>
> #include "compiler.h" /* QE
bitmap.h
> create mode 100644 tests/test-hbitmap.c
I've looked through this several times, but did another once-over, and
you can feel free to 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
s | 2 +-
> block.c| 94
> ++
> block.h| 6 ++--
> block/mirror.c | 12 ++--
> block_int.h| 4 +--
> trace-events | 1 +
> 6 files changed, 33 insertions(+), 86 deletions(-)
Reviewed-by: Eric Blake
gt; + * free the errot object.
s/errot/error/
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
27;*telnet': 'bool'} }
>
> # For future extensibility...
> { 'ChardevDummy', 'data': {} }
>
> { 'union': 'ChardevBackend', 'data': {
> 'socket': 'ChardevSocket',
> 'udp': 'UDPSocketAddress',
> 'file': 'ChardevFile',
> 'null': 'ChardevDummy',
> 'msmouse': 'ChardevDummy',
> 'braille': 'ChardevDummy',
> 'stdio': 'ChardevDummy',
> 'vc': 'ChardevVC',
>
> # Solely for HMP usage.
> 'legacy': 'str'
> }
>
> { 'command': 'chardev-add', 'data': {
> 'backend': 'ChardevBackend', 'id': 'str', '*mux': 'bool' } }
Yes, this looks nicer.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
snapshot whose id becomes 3).
>
>> +#
>> +# Returns: Nothing on success
One thing is for certain - if 'name' remains optional, then you MUST
return the 'id' that was auto-allocated. And even if name is not
optional, returning the 'id' that was either
Set the whole virtual machine to the snapshot identified by the tag
> +# or the unique snapshot id. It must be snapshot of whole VM.
> +#
> +# @name: tag|id of existing snapshot
> +#
> +# Returns: Nothing on success
> +# If an error occurs, GenericError with error messa
urns: Nothing on success
> +# If an error occurs, GenericError with error message
> +#
> +# Since: 1.3
Another case of 1.4.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
On 12/12/2012 06:46 AM, Paolo Bonzini wrote:
> This is needed in the following patch.
>
> Reviewed-by: Laszlo Ersek
> Signed-off-by: Paolo Bonzini
> ---
> block.c | 16
> block.h | 4
> 2 files changed, 12 insertions(+), 8 deletions(-)
Reviewed-b
; tests/qemu-iotests/041 | 21
> tests/qemu-iotests/041.out | 4 ++--
> trace-events | 1 +
> 5 files changed, 78 insertions(+), 23 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
On 12/12/2012 06:46 AM, Paolo Bonzini wrote:
> Reviewed-by: Laszlo Ersek
> Signed-off-by: Paolo Bonzini
> ---
> block-migration.c | 2 +-
> block.c | 5 ++---
> block/mirror.c| 2 +-
> 3 files changed, 4 insertions(+), 5 deletions(-)
Reviewed-by: Eric Blake
+667,12 @@
> #
> # @count: number of dirty bytes according to the dirty bitmap
> #
> +# @granularity: granularity of the dirty bitmap in bytes
Mention that this field was added in 1.4.
> +#
> # Since: 1.3
> ##
> { 'type': 'BlockDirtyInfo',
>
the image format defines
> +a cluster size, the cluster size or 4096, whichever is larger. If it
> +does not define a cluster size, the default value of the granularity
> +is 65536.
That doesn't quite cover the fact that you clamp granularity to 64k if
the image format has a cluster size larger than 64k.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
/* Allocate a MirrorOp that is used as an AIO callback. */
> +op = g_slice_new(MirrorOp);
> +op->s = s;
> +op->iov.iov_base = s->buf;
> +op->iov.iov_len = nb_sectors * 512;
Why two spaces?
I'm not an expert in this area of code, so my review
tmap (json-int, optional)
> +- "buf_size": maximum amount of data in flight from source to target
> + (json-int, default 10M)
Oh, so unlike granularity, this one does NOT have to be a power of 2.
But why is the default 10M if you clamp it to granularity, which
defaults to 64k? (a
Paolo Bonzini
> ---
> block/mirror.c | 109
> ++---
> trace-events | 4 ++-
> 2 files changed, 100 insertions(+), 13 deletions(-)
Again, my review is weak, but feel free to add:
Reviewed-by: Eric Blake
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualiza
> 2 files changed, 69 insertions(+), 32 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
> +
> +void qemu_mmap_free(QEMUMmapArea *mm)
> +{
> +munmap(mm->mem, mm->size);
> +close(mm->fd);
> +}
You unlink()d the file on failure during the initial map, but nowhere
else. Is that intentional?
> diff --git a/oslib-win32.c b/oslib-win32.c
My review gets
p->iov.iov_len = nb_sectors * 512;
> +op->sector_num = sector_num;
> +op->nb_sectors = nb_sectors;
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
g a client-provided data block for the
> + * actual bitmap and allocating memory only for the compressed levels.
> + * If @data is NULL, this function is equivalent to @hbitmap_alloc.
> + */
> +HBitmap *hbitmap_alloc_with_data(uint64_t size, int granularity, void *data);
But this document
3 files changed, 126 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
from the
guest, not having a guarantee of an atomic set of stats is not really
much of a loss.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
int64_t sector_num, int nb_sectors, int *pnum)
Indentation looks off.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
+@item -q
> +Quiet mode - do not print any output (except errors). There's no progres bar
s/progres/progress/
> +in case both @var{-q} and @var{-p} options are used.
Should it be a hard error if both -q and -p are given? Otherwise, when
dealing with conflicting options, it's more typical to have the semantic
of last one wins: 'qemu-img -q -p' prints progress, and only 'qemu-img
-p -q' is quiet.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
" of %s: %s",
> + sectors_to_bytes(sect_num), filename, strerror(-ret));
> +return ret;
> +}
> + ret = is_allocated_sectors(buffer, sect_count, &pnum);
Is this logic backwards? Isn't it wasteful to read a sector prior to
seeing if it was actually allocated?
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
tely in this case.
>
> Commit the changes recorded in @var{filename} in its base image.
>
> +@item compare [-f @var{fmt}] [-F @var{fmt}] [-p] [-s] [-q] @var{filename1}
> @var{filename2}
> +
> +Check if two images have the same content. You can compare images with
> +differ
ions(+), 28 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 out to be a dict exposing multiple sub-properties at once. And if
you can get that to work, you are back to your goal of no new QMP command.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
error_free(*err); \
> + } \
> +error_set(err, ERROR_CLASS_GENERIC_ERROR, fmt, ## __VA_ARGS__); \
> +} while (/*CONSTCOND*/0)
qemu-queue.h is the only other file in qemu.git that uses /*CONSTCOND*/
notation, and that's because of the file origins; do we really need it here?
--
Eric
us snapshots, where the work is divided into several
phases. You'll need qemu to prepare for the snapshot, then the external
tools to create the snapshot, then resume qemu with possibly different
file names as a result of the snapshot being created. I'm certainly
interested in seeing what
_alloc_with_data(bitmap_size, granularity_bits,
> mm.mem);
> +if (!load) {
> +hbitmap_copy(new_bitmap, bs->dirty_bitmap);
Here's where my comment earlier in the series, about
hbitmap_alloc_with_data repopulating the upper layers of the hbitmap on
reused data, becomes
201 - 300 of 23401 matches
Mail list logo
