Re: [Qemu-devel] Created virtio-vsock wiki page

[Christoffer Dall]

On Tue, Aug 25, 2015 at 04:43:13PM +0100, Stefan Hajnoczi wrote:
> I have created a wiki page for virtio-vsock.
> 
> It links to my git repos and the draft virtio specification:
> http://qemu-project.org/Features/VirtioVsock
> 
> I'll expand and update it over the coming days and weeks.
> 
> Please let me know if you'd like to see specific information on there
> (e.g. step-by-step build & QEMU invocation guide).
> 
Do you have a test suite or a simple set of guest/host test programs?

-Christoffer

Re: [Qemu-devel] [PATCH v2 10/12] qga: add an optionnal qemu-ga.conf system configuration

[Markus Armbruster]

marcandre.lur...@redhat.com writes:

> From: Marc-André Lureau 
>
> Learn to configure the agent with a system configuration.
>
> This may simplify command-line handling, especially when the blacklist
> is long.
>
> Among the other benefits, this may standardize the configuration of a
> init service (instead of distro-specific init keys/files)
>
> Signed-off-by: Marc-André Lureau 

This uses GLib's Key-value file parser.  Note that we have our own .ini
parser qemu_config_parse().  It predates our use of GLib.  Having two
different parsers risks inconsistency.  Since qga is already using
GLib's, using it some more there is better than adding a use of our own
parser, so no objection to your patch on that ground.

Replacing qemu_config_parse()'s parsing guts by GLib probably won't save
code, but it could be nice for consistency.  Well outside this patch's
scope, of course.

Re: [Qemu-devel] [PATCH] arm: Use g_new() & friends where that makes obvious sense

[Markus Armbruster]

Eric Blake  writes:

> On 08/25/2015 11:39 AM, Markus Armbruster wrote:
>> g_new(T, n) is neater than g_malloc(sizeof(T) * n).  It's also safer,
>> for two reasons.  One, it catches multiplication overflowing size_t.
>> Two, it returns T * rather than void *, which lets the compiler catch
>> more type errors.
>> 
>> This commit only touches allocations with size arguments of the form
>> sizeof(T).
>> 
>> Coccinelle semantic patch:
>> 
>> @@
>> type T;
>> @@
>> -g_malloc(sizeof(T))
>> +g_new(T, 1)
>> @@
>> type T;
>
> I find it slightly easier to read coccinelle if there is a blank line
> before every second @@, to call attention to metatype vs. changes
> (coccinelle doesn't care either way).
>
> And it's probably possible to write the coccinelle more succinctly, by
> grouping similar rules together using something like this (although I
> didn't actually test it):
>
> @@
> type T;
> @@
> (
>  g_malloc
> |
>  g_try_malloc
> |
>  g_malloc0
> |
>  g_try_malloc0
> )
> -(sizeof(T))
> +(T, 1)
>
> but it doesn't matter in the long run.

I tend to write really stupid semantic patches, because when I try to
write a clever one, I usually relearn I'm not nearly clever enough for
that.

Amazing tool all the same.

>> Signed-off-by: Markus Armbruster 
>> ---
>
> Both the coccinelle rule and the resulting conversion look sane.
> Reviewed-by: Eric Blake 
>
>> +++ b/hw/gpio/omap_gpio.c
>> @@ -710,8 +710,8 @@ static int omap2_gpio_init(SysBusDevice *sbd)
>>  } else {
>>  s->modulecount = 6;
>>  }
>> -s->modules = g_malloc0(s->modulecount * sizeof(struct omap2_gpio_s));
>> -s->handler = g_malloc0(s->modulecount * 32 * sizeof(qemu_irq));
>> +s->modules = g_new0(struct omap2_gpio_s, s->modulecount);
>> +s->handler = g_new0(qemu_irq, s->modulecount * 32);
>>  qdev_init_gpio_in(dev, omap2_gpio_set, s->modulecount * 32);
>>  qdev_init_gpio_out(dev, s->handler, s->modulecount * 32);
>
> Thankfully, the '* 32' here does not overflow even pre-patch, since
> s->modulecount was set to a maximum of 6 in the preceding if/else block.

Thanks!

Re: [Qemu-devel] KVM guest gets aborted if blockcommit is called

[Christian Rößner]

> Am 25.08.2015 um 08:02 schrieb Christian Rößner :
> 
> Hello,
> 
> I wrote this mail to the qemu-discuss mailing list, but today I am unsure, if 
> I chose the right list. So I copy and paste this mail here in hope someone 
> can respond :-)
> 
> I have reproducable problems with some code in qemu-coroutine.c:
> 
> 
> void qemu_coroutine_enter(Coroutine *co, void *opaque)
> {
>Coroutine *self = qemu_coroutine_self();
>CoroutineAction ret;
> 
>trace_qemu_coroutine_enter(self, co, opaque);
> 
>if (co->caller) {
>fprintf(stderr, "Co-routine re-entered recursively\n");
>abort();   <— This one triggers 4 or 5 out of ten tests to use 
> the blockcommit feature
>}

Caught Co-routine SIGABRT while a blockcommit operation was running.

Recompiled with debugging symbols and I connected gdb to the process:

(gdb) bt
#0  0x7f4b6e6ccb8e in raise () from /lib64/libc.so.6
#1  0x7f4b6e6ce391 in abort () from /lib64/libc.so.6
#2  0x555a316a8c39 in qemu_coroutine_enter (co=0x555a34651a50, opaque=0x0)
at 
/var/tmp/portage/app-emulation/qemu-2.4.0/work/qemu-2.4.0/qemu-coroutine.c:111
#3  0x555a316a8eda in qemu_co_queue_run_restart (co=co@entry=0x555a33d271b0)
at 
/var/tmp/portage/app-emulation/qemu-2.4.0/work/qemu-2.4.0/qemu-coroutine-lock.c:59
#4  0x555a316a8b53 in qemu_coroutine_enter (co=0x555a33d271b0, 
opaque=)
at 
/var/tmp/portage/app-emulation/qemu-2.4.0/work/qemu-2.4.0/qemu-coroutine.c:118
#5  0x555a316e3adf in bdrv_co_aio_rw_vector (bs=bs@entry=0x555a336a6be0,
sector_num=sector_num@entry=113551488, qiov=qiov@entry=0x555a3367d2c8,
nb_sectors=nb_sectors@entry=15360, flags=flags@entry=(unknown: 0),
cb=cb@entry=0x555a316e1fe0 , opaque=0x555a3367d2c0, 
is_write=is_write@entry=false)
at /var/tmp/portage/app-emulation/qemu-2.4.0/work/qemu-2.4.0/block/io.c:2142
#6  0x555a316e4b1e in bdrv_aio_readv (bs=bs@entry=0x555a336a6be0,
sector_num=sector_num@entry=113551488, qiov=qiov@entry=0x555a3367d2c8,
nb_sectors=nb_sectors@entry=15360, cb=cb@entry=0x555a316e1fe0 
,
opaque=opaque@entry=0x555a3367d2c0)
at /var/tmp/portage/app-emulation/qemu-2.4.0/work/qemu-2.4.0/block/io.c:1744
#7  0x555a316e2ccf in mirror_iteration (s=0x555a34a0c250)
at 
/var/tmp/portage/app-emulation/qemu-2.4.0/work/qemu-2.4.0/block/mirror.c:302
#8  mirror_run (opaque=0x555a34a0c250)
at 
/var/tmp/portage/app-emulation/qemu-2.4.0/work/qemu-2.4.0/block/mirror.c:512
#9  0x555a316a9a5a in coroutine_trampoline (i0=, 
i1=)
at 
/var/tmp/portage/app-emulation/qemu-2.4.0/work/qemu-2.4.0/coroutine-ucontext.c:80
#10 0x7f4b6e6df4a0 in ?? () from /lib64/libc.so.6
#11 0x7ffe67b71840 in ?? ()
#12 0x in ?? ()
(gdb)

Please, could someone reply to me :-)

Thanks

Christian

smime.p7s
Description: S/MIME cryptographic signature

Re: [Qemu-devel] [PATCH] CODING_STYLE: update line length and mixed declaration rules

[Markus Armbruster]

John Snow  writes:

> On 08/25/2015 02:20 PM, Markus Armbruster wrote:
>> Paolo Bonzini  writes:
>> 
>>> On 19/06/2015 10:09, Andreas Färber wrote:
>> -Lines are 80 characters; not longer.
>> +Lines should be 80 characters; try not to make them longer.
>> +
>> +Sometimes it is hard to do, especially when dealing with QEMU subsystems
>> +that use long function or symbol names.  Even in that case, do not make
>> +lines _much_ longer than 80 characters.
 Anthony had always allowed sensible exceptions to that rule, so +1 for
 reformulating it here.

 However, I would suggest that in that case we should lower the
 recommendation/warning to 78 chars, with the rationale of not only the
 actual code but also two-way diffs (79 chars plus ±/space) and
 three-way diffs (78 chars plus 2x ±/space) fitting into standard 80x24
 windows.
>>>
>>> Good idea.
>> 
>> I personally prefer a slightly lower limit, to keep quoted patches in
>> e-mail neatly under 80.  How much writability to trade for readability
>> is subjective, and I won't argue against 78.
>> 
>
> As long as we don't update the checkpatch tool to whine about this,
> since it might break a good amount of existing context, and this will
> just inconvenience everyone and provide no real benefit.
>
> Maybe if you can have it warn only for NEW lines and not for context,
> but if that's not possible I'm against shortening the existing limit.
>
> IMO: The 80 char width rule makes good sense, but forcing the margins
> even smaller on today's clearly-no-longer-a-terminal devices is just a
> little too much.

The reason for line length limit isn't antiquated hardware or antiquated
habits, it's us antiquated human beings: we tend to have trouble
following long lines with our eyes (I sure do).  Typographic manuals
suggest to limit columns to roughly 60 characters for exactly that
reason[*].  Four levels of indentation plus 60 characters of actual text
yields 76.

> Of course, gently prodding people to reconsider their line length if
> they are reliably approaching 75+ is another issue entirely... it just
> shouldn't reflect as non-success via checkpatch.

I think that's what Paolo's patch does.  Namely:

line shorter than the soft limit: fine
line between soft and hard limit: warn
line longer than the hard limit: error

He kept the soft limit at 80, and set the hard limit to 90.  Andreas
suggested to lower the soft limit a bit, and I suggested to lower it a
bit more.

[...]


[*] https://en.wikipedia.org/wiki/Column_(typography)#Typographic_style

Re: [Qemu-devel] [PATCH 2/8] qcow2: add dirty-bitmaps feature

[Stefan Hajnoczi]

On Fri, Aug 14, 2015 at 08:14:46PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> On 10.06.2015 17:30, Stefan Hajnoczi wrote:
> >On Mon, Jun 08, 2015 at 06:21:20PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> >>+ret = bdrv_pread(bs->file, bm->l1_table_offset, l1_table,
> >>+ bm->l1_size * sizeof(uint64_t));
> >>+if (ret < 0) {
> >>+goto fail;
> >>+}
> >>+
> >>+buf = g_malloc0(bm->l1_size * s->cluster_size);
> >What is the maximum l1_size value?  cluster_size and l1_size are 32-bit
> >so with 64 KB cluster_size this overflows if l1_size > 65535.  Do you
> >want to cast to size_t?
> 
> Hmm. What the maximum RAM space we'd like to spend on dirty bitmap? I think
> 4Gb is too much.. So here should be limited not the l1_size but number of
> bytes needed to store the bitmap. What is maximum disk size we are dealing
> with?

Modern file systems support up to exa- (XFS) or zetta- (ZFS) byte size.
If the disk image size is large, then the cluster size will probably
also be set larger than 64 KB (e.g. 1 MB).

Anyway, with 64 KB cluster size & bitmap granularity a 128 MB dirty
bitmap covers a 64 TB disk image.  So how about 256 MB or 512 MB max
dirty bitmap size?

Stefan

Re: [Qemu-devel] QEMU produces invalid JSON due to locale-dependent code

[Alberto Garcia]

On Tue 25 Aug 2015 09:54:42 AM CEST, Markus Armbruster wrote:

>> (D) Run in a controlled mixed locale
>>GTK runs completely in the locale determined by setlocale() (since it
>> never has to display raw JSON)
>>We fix our JSON output code to use thread-specific locale
>> manipulations to (temporarily) switch to C locale before printing JSON
>>
>> that way, GTK still shows full German formatting for any localized
>> message in the GUI that happens to print numbers, but the JSON output
>> (which is independent of the GUI) also behaves correctly as a C-only
>> entity.
>
> Switching back to C locale whenever some unwanted locale-dependency
> breaks the code is problematic, because it involves finding all the
> places that break, iteratively (euphemism for "we debug one breakage
> after the other, adding temporary locale switches as we go).

For some cases we probably don't even need to switch the locale. For the
JSON case cannot we just easily convert the QFloat into a string
ourselves without using printf's "%f" ?

That doesn't invalidate however your point.

> I'd feel much better about confining GTK in its own thread, and
> setting only that thread's locale.

I haven't digged much into that part of the QEMU code, but if my
assumptions are true I think we would need at least:

- A separate GMainContext with its own main loop
- Making sure that all the code that touches the UI runs from that
  thread.

This is in principle possible, but I fear that we might be opening a can
of worms.

Berto

Re: [Qemu-devel] [PATCH v2 RFC 0/8] block: persistent dirty bitmaps

[Stefan Hajnoczi]

On Wed, Aug 26, 2015 at 09:26:20AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> On 12.06.2015 13:36, Stefan Hajnoczi wrote:
> >On Fri, Jun 12, 2015 at 12:58:35PM +0300, Denis V. Lunev wrote:
> >>On 11/06/15 23:06, Stefan Hajnoczi wrote:
> >>>The load/store API is not scalable when bitmaps are 1 MB or larger.
> >>>
> >>>For example, a 500 GB disk image with 64 KB granularity requires a 1 MB
> >>>bitmap.  If a guest has several disk images of this size, then multiple
> >>>megabytes must be read to start the guest and written out to shut down
> >>>the guest.
> >>>
> >>>By comparison, the L1 table for the 500 GB disk image is less than 8 KB.
> >>>
> >>>I think something like qcow2-cache.c or metabitmaps should be used to
> >>>lazily read/write persistent bitmaps.  That way only small portions need
> >>>to be read/written at a time.
> >>>
> >>>Stefan
> >>for the first iteration we could open the image, start tracking,
> >>read bitmap as one entity in the background and or read
> >>and collected data.
> >>
> >>partial read could be done in the next step
> >Making bitmap load/store fully lazy will require changes to the
> >load/store API, so it's worth thinking about a little upfront.
> >Otherwise there will be a lot of code churn when the fully lazy patches
> >are posted.  As a reviewer it's in my interest to only spend time
> >reviewing the final version instead of code that gets thrown out :-),
> >but I understand.
> >
> >If you can make the read lazy to some extent that's a good start.
> That way we can improve load performance, but what about store?
> 
> I see two solutions:
> 1) meta bitmaps (already mentioned)
> 2) Always (optionally?) have two bitmaps instead one: backing, which should
> be equal to the bitmap, already stored to the image, and active delta. This
> can be used instead of meta bitmaps in migration too.
> 
> difference:
> with meta bitmaps we have double time overhead for writing to the bitmap
> (which is more often operations as I think),
> with second approach we have double overhead for read from the bitmap (but
> for backup, we can *or* these two bitmaps once, and it can be done fast,
> using the power of HBitmap). And of course double ram overhead..

Meta bitmaps seem like a good idea since they are already needed for
live migration.

Re: [Qemu-devel] [PATCH] e500 ATMU register reads broken

[Alexander Graf]

On 21.08.15 14:33, Rudolf Marek wrote:
> Hi all,
> 
> Ping?
> 
> Thanks
> Rudolf
> 
> Dne 14.8.2015 v 13:49 Rudolf Marek napsal(a):
>> Hi all,
>>
>> I noticed that ATMU register reads on E500 are broken. Due to the
>> wrong mask,
>> some registers cannot be read and instead some other registers are
>> read. Please
>> see attached patch which fixes the problem.

Sorry for the long delay. Please CC qemu-...@nongnu.org on the next
submission, so that more PPC people have the chance to review the patch ;).

Thanks, applied to ppc-next.

>>
>> I also noticed that if there was an intention to have 1:1 PCI/CPU
>> space mapping
>> for 0xC000_ for MPC8544DS without programming ATMUs - it does not
>> work,
>> unless ATMUs are programmed.

I've received a report about this on IRC as well. It works fine with
U-Boot because that writes the ATMUs, but not with direct -kernel boot.

Would you mind to follow up with a patch to the mpc8544ds machine file
that programs the ATMUs on init?


Thanks a lot!

Alex

Re: [Qemu-devel] [PATCH] target-s390x: Mask the SIGP order_code to 8bit.

[Alexander Graf]

On 20.08.15 19:16, Thomas Huth wrote:
> On 18/08/15 04:50, Philipp Kern wrote:
>> According to "CPU Signaling and Response", "Signal-Processor Orders",
>> the order field is bit position 56-63. Without this, the Linux
>> guest kernel is sometimes unable to stop emulation and enters
>> an infinite loop of "XXX unknown sigp: 0x0005".
>>
>> Signed-off-by: Philipp Kern 
>> ---
>>  target-s390x/misc_helper.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/target-s390x/misc_helper.c b/target-s390x/misc_helper.c
>> index 8eac0e1..0f0907c 100644
>> --- a/target-s390x/misc_helper.c
>> +++ b/target-s390x/misc_helper.c
>> @@ -500,7 +500,7 @@ uint32_t HELPER(sigp)(CPUS390XState *env, uint64_t 
>> order_code, uint32_t r1,
>>  /* Remember: Use "R1 or R1 + 1, whichever is the odd-numbered register"
>> as parameter (input). Status (output) is always R1. */
>>  
>> -switch (order_code) {
>> +switch (order_code & 0xff) {
>>  case SIGP_SET_ARCH:
>>  /* switch arch */
>>  break;
> 
> Reviewed-by: Thomas Huth 

Thanks, applied to s390-next.


Alex

Re: [Qemu-devel] QEMU produces invalid JSON due to locale-dependent code

[Daniel P. Berrange]

On Wed, Aug 26, 2015 at 11:13:00AM +0200, Alberto Garcia wrote:
> On Tue 25 Aug 2015 09:54:42 AM CEST, Markus Armbruster wrote:
> 
> >> (D) Run in a controlled mixed locale
> >>GTK runs completely in the locale determined by setlocale() (since it
> >> never has to display raw JSON)
> >>We fix our JSON output code to use thread-specific locale
> >> manipulations to (temporarily) switch to C locale before printing JSON
> >>
> >> that way, GTK still shows full German formatting for any localized
> >> message in the GUI that happens to print numbers, but the JSON output
> >> (which is independent of the GUI) also behaves correctly as a C-only
> >> entity.
> >
> > Switching back to C locale whenever some unwanted locale-dependency
> > breaks the code is problematic, because it involves finding all the
> > places that break, iteratively (euphemism for "we debug one breakage
> > after the other, adding temporary locale switches as we go).
> 
> For some cases we probably don't even need to switch the locale. For the
> JSON case cannot we just easily convert the QFloat into a string
> ourselves without using printf's "%f" ?

FWIW, libvirt had this exact same problem with formatting doubles for
JSON while non-C locales are in effect. We used the glibc thread-local
locale support, and fallback to some sick string munging in non-glibc
case:


/* In case thread-safe locales are available */
#if HAVE_NEWLOCALE

static locale_t virLocale;

static int
virLocaleOnceInit(void)
{
virLocale = newlocale(LC_ALL_MASK, "C", (locale_t)0);
if (!virLocale)
return -1;
return 0;
}

VIR_ONCE_GLOBAL_INIT(virLocale)
#endif

/**
 * virDoubleToStr
 *
 * converts double to string with C locale (thread-safe).
 *
 * Returns -1 on error, size of the string otherwise.
 */
int
virDoubleToStr(char **strp, double number)
{
int ret = -1;

#if HAVE_NEWLOCALE

locale_t old_loc;

if (virLocaleInitialize() < 0)
goto error;

old_loc = uselocale(virLocale);
ret = virAsprintf(strp, "%lf", number);
uselocale(old_loc);

#else

char *radix, *tmp;
struct lconv *lc;

if ((ret = virVasprintf(strp, "%lf", number) < 0)
goto error;

lc = localeconv();
radix = lc->decimal_point;
tmp = strstr(*strp, radix);
if (tmp) {
*tmp = '.';
if (strlen(radix) > 1)
memmove(tmp + 1, tmp + strlen(radix), strlen(*strp) - (tmp - str));
}

#endif /* HAVE_NEWLOCALE */
 error:
return ret;
}


> That doesn't invalidate however your point.
> 
> > I'd feel much better about confining GTK in its own thread, and
> > setting only that thread's locale.
> 
> I haven't digged much into that part of the QEMU code, but if my
> assumptions are true I think we would need at least:
> 
> - A separate GMainContext with its own main loop
> - Making sure that all the code that touches the UI runs from that
>   thread.
> 
> This is in principle possible, but I fear that we might be opening a can
> of worms.

Agreed, my experiance is that you should always run GTK in the main
thread and never try todo anything more clever with threads + GTK.
It has always ended in tears for me - even if you think you have it
working on your system, you'll inevitably find a different version
of GTK where it has broken. It just isn't worth the pain to try to
run anything GTK related in a non-main thread.

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

[Qemu-devel] [PATCH] sheepdog: add reopen support

[Liu Yuan]

From: Liu Yuan 

With reopen supported, block-commit (and offline commit) is now supported for
image files whose base image uses the Sheepdog protocol driver.

Cc: qemu-devel@nongnu.org
Cc: Jeff Cody 
Cc: Kevin Wolf 
Cc: Stefan Hajnoczi 
Signed-off-by: Liu Yuan 
---
 block/sheepdog.c | 72 
 1 file changed, 72 insertions(+)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index 9585beb..26d09e9 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -377,6 +377,11 @@ typedef struct BDRVSheepdogState {
 QLIST_HEAD(inflight_aiocb_head, SheepdogAIOCB) inflight_aiocb_head;
 } BDRVSheepdogState;
 
+typedef struct BDRVSheepdogReopenState {
+int fd;
+int cache_flags;
+} BDRVSheepdogReopenState;
+
 static const char * sd_strerror(int err)
 {
 int i;
@@ -1486,6 +1491,64 @@ out:
 return ret;
 }
 
+static int sd_reopen_prepare(BDRVReopenState *state, BlockReopenQueue *queue,
+ Error **errp)
+{
+BDRVSheepdogState *s = state->bs->opaque;
+BDRVSheepdogReopenState *re_s;
+int ret = 0;
+
+re_s = state->opaque = g_new0(BDRVSheepdogReopenState, 1);
+
+if (state->flags & BDRV_O_NOCACHE) {
+re_s->cache_flags = SD_FLAG_CMD_DIRECT;
+}
+
+re_s->fd = get_sheep_fd(s, errp);
+if (re_s->fd < 0) {
+ret = re_s->fd;
+return ret;
+}
+
+return ret;
+}
+
+static void sd_reopen_commit(BDRVReopenState *state)
+{
+BDRVSheepdogReopenState *re_s = state->opaque;
+BDRVSheepdogState *s = state->bs->opaque;
+
+if (s->fd) {
+closesocket(s->fd);
+}
+
+s->fd = re_s->fd;
+s->cache_flags = re_s->cache_flags;
+
+g_free(state->opaque);
+state->opaque = NULL;
+
+return;
+}
+
+static void sd_reopen_abort(BDRVReopenState *state)
+{
+BDRVSheepdogReopenState *re_s = state->opaque;
+
+if (re_s == NULL) {
+return;
+}
+
+if (re_s->fd) {
+closesocket(re_s->fd);
+}
+
+g_free(state->opaque);
+state->opaque = NULL;
+
+return;
+}
+
 static int do_sd_create(BDRVSheepdogState *s, uint32_t *vdi_id, int snapshot,
 Error **errp)
 {
@@ -2703,6 +2766,9 @@ static BlockDriver bdrv_sheepdog = {
 .instance_size  = sizeof(BDRVSheepdogState),
 .bdrv_needs_filename = true,
 .bdrv_file_open = sd_open,
+.bdrv_reopen_prepare= sd_reopen_prepare,
+.bdrv_reopen_commit = sd_reopen_commit,
+.bdrv_reopen_abort  = sd_reopen_abort,
 .bdrv_close = sd_close,
 .bdrv_create= sd_create,
 .bdrv_has_zero_init = bdrv_has_zero_init_1,
@@ -2736,6 +2802,9 @@ static BlockDriver bdrv_sheepdog_tcp = {
 .instance_size  = sizeof(BDRVSheepdogState),
 .bdrv_needs_filename = true,
 .bdrv_file_open = sd_open,
+.bdrv_reopen_prepare= sd_reopen_prepare,
+.bdrv_reopen_commit = sd_reopen_commit,
+.bdrv_reopen_abort  = sd_reopen_abort,
 .bdrv_close = sd_close,
 .bdrv_create= sd_create,
 .bdrv_has_zero_init = bdrv_has_zero_init_1,
@@ -2769,6 +2838,9 @@ static BlockDriver bdrv_sheepdog_unix = {
 .instance_size  = sizeof(BDRVSheepdogState),
 .bdrv_needs_filename = true,
 .bdrv_file_open = sd_open,
+.bdrv_reopen_prepare= sd_reopen_prepare,
+.bdrv_reopen_commit = sd_reopen_commit,
+.bdrv_reopen_abort  = sd_reopen_abort,
 .bdrv_close = sd_close,
 .bdrv_create= sd_create,
 .bdrv_has_zero_init = bdrv_has_zero_init_1,
-- 
1.9.1

Re: [Qemu-devel] [PATCH] qemu-ga: implement win32 guest-set-user-password

[Daniel P. Berrange]

On Tue, Jun 30, 2015 at 04:37:13PM +0200, Marc-André Lureau wrote:
> Use NetUserSetInfo() to set the user password.
> 
> This function is notoriously known to be problematic for users with EFS
> encrypted files. But the alternative, NetUserChangePassword() requires
> the old password. Nevertheless, The EFS file should be recovered by
> changing back to the old password.
> 
> Signed-off-by: Marc-André Lureau 
> ---
>  configure|  2 +-
>  qga/commands-win32.c | 77 
> ++--
>  2 files changed, 76 insertions(+), 3 deletions(-)

Reviewed-by: Daniel P. Berrange 


For reference, I compared with MS recommendations here

  https://support.microsoft.com/en-us/kb/151546

and this looks correct for way to administratively change passwords
without knowing the original password.

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

Re: [Qemu-devel] [PATCH v2 06/18] pc: implement NVDIMM device abstract

[Xiao Guangrong]

On 08/25/2015 10:57 PM, Stefan Hajnoczi wrote:

On Fri, Aug 14, 2015 at 10:51:59PM +0800, Xiao Guangrong wrote:

+static void set_file(Object *obj, const char *str, Error **errp)
+{
+PCNVDIMMDevice *nvdimm = PC_NVDIMM(obj);
+
+if (nvdimm->file) {
+g_free(nvdimm->file);
+}


g_free(NULL) is a nop so it's safe to replace the if with just
g_free(nvdimm->file).



Yeah, the man page says you're right. Will clean it up.

Re: [Qemu-devel] target-ppc: Fix SRR0 when taking unaligned exceptions

[Alexander Graf]

On 02.07.15 06:44, Anton Blanchard wrote:
> We are setting SRR0 to the instruction before the one causing the
> unaligned exception. A quick testcase:
> 
> . = 0x100
> .globl _start
> _start:
>   /* Cause a 0x600 */
>   li  3,0x1
>   stwcx.  3,0,3
> 1:b   1b
> 
> . = 0x600
> 1:b   1b
> 
> Built into something we can load as a BIOS image:
> 
> gcc -mbig -c test.S
> ld -EB -Ttext 0x0 -o test test.o
> objcopy -O binary test test.bin
> 
> Run with:
> 
> qemu-system-ppc64 -nographic -bios test.bin
> 
> Shows an incorrect SRR0 (points at the li):
> 
> SRR0 0100
> 
> With the patch we get the correct SRR0:
> 
> SRR0 0104
> 
> Signed-off-by: Anton Blanchard 

Thanks, applied to ppc-next.


Alex

Re: [Qemu-devel] [PATCH v2 07/18] nvdimm: reserve address range for NVDIMM

[Xiao Guangrong]

On 08/25/2015 11:12 PM, Stefan Hajnoczi wrote:

On Fri, Aug 14, 2015 at 10:52:00PM +0800, Xiao Guangrong wrote:

diff --git a/hw/mem/nvdimm/pc-nvdimm.c b/hw/mem/nvdimm/pc-nvdimm.c
index a53d235..7a270a8 100644
--- a/hw/mem/nvdimm/pc-nvdimm.c
+++ b/hw/mem/nvdimm/pc-nvdimm.c
@@ -24,6 +24,19 @@

  #include "hw/mem/pc-nvdimm.h"

+#define PAGE_SIZE  (1UL << 12)


This macro name is likely to collide with system headers or other code.

Could you use the existing TARGET_PAGE_SIZE constant instead?


Will follow your way in the next version. Thank you for pointing it out.

Re: [Qemu-devel] [PATCH v2 07/18] nvdimm: reserve address range for NVDIMM

[Xiao Guangrong]

On 08/25/2015 11:12 PM, Stefan Hajnoczi wrote:

On Fri, Aug 14, 2015 at 10:52:00PM +0800, Xiao Guangrong wrote:

diff --git a/hw/mem/nvdimm/pc-nvdimm.c b/hw/mem/nvdimm/pc-nvdimm.c
index a53d235..7a270a8 100644
--- a/hw/mem/nvdimm/pc-nvdimm.c
+++ b/hw/mem/nvdimm/pc-nvdimm.c
@@ -24,6 +24,19 @@

  #include "hw/mem/pc-nvdimm.h"

+#define PAGE_SIZE  (1UL << 12)


This macro name is likely to collide with system headers or other code.

Could you use the existing TARGET_PAGE_SIZE constant instead?



Will follow your way in the next version. Thank you for pointing it out.

Re: [Qemu-devel] [PATCH v3 3/4] qemu-ga: Created a separate component for each installed file in the MSI

[Leonid Bloch]

Thanks for pointing that out, Michael!

This indeed happens when configure is called out of tree.
But this bug was not introduced with this series of patches, so it is
irrelevant here.
We will issue another patch to address it shortly.

Thanks again,
Leonid.

On Wed, Aug 26, 2015 at 12:48 AM, Michael Roth
 wrote:
> Quoting Leonid Bloch (2015-08-03 12:54:23)
>> This is done to follow the recommendations given here: 
>> https://msdn.microsoft.com/en-us/library/aa368269%28VS.85%29.aspx
>>
>> Signed-off-by: Leonid Bloch 
>> ---
>>  qga/installer/qemu-ga.wxs | 47 
>> ---
>>  1 file changed, 36 insertions(+), 11 deletions(-)
>>
>> diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs
>> index dac1df0..2302745 100644
>> --- a/qga/installer/qemu-ga.wxs
>> +++ b/qga/installer/qemu-ga.wxs
>> @@ -71,16 +71,6 @@
>>  
>>> Guid="{908B7199-DE2A-4DC6-A8D0-27A5AE444FEA}">
>>  > Source="../../qemu-ga.exe" KeyPath="yes" DiskId="1"/>
>> -
>> -> Source="../vss-win32/qga-vss.dll" KeyPath="no" DiskId="1"/>
>> -> Source="../vss-win32/qga-vss.tlb" KeyPath="no" DiskId="1"/>
>> -
>> -> Source="$(var.Mingw_bin)/iconv.dll" KeyPath="no" DiskId="1"/>
>> -> Source="$(var.Mingw_bin)/$(var.ArchLib)" KeyPath="no" DiskId="1"/>
>> -> Source="$(var.Mingw_bin)/libglib-2.0-0.dll" KeyPath="no" DiskId="1"/>
>> -> Source="$(var.Mingw_bin)/libintl-8.dll" KeyPath="no" DiskId="1"/>
>> -> Source="$(var.Mingw_bin)/libssp-0.dll" KeyPath="no" DiskId="1"/>
>> -> Source="$(var.Mingw_bin)/libwinpthread-1.dll" KeyPath="no" DiskId="1"/>
>>  >Id="ServiceInstaller"
>>Type="ownProcess"
>> @@ -97,7 +87,32 @@
>>  
>>  > Remove="uninstall" Name="QEMU-GA" Wait="no" />
>>
>> -
>> +  
>> +  > Guid="{CB19C453-FABB-4BB1-ABAB-6B74F687BFBB}">
>> +> Source="../vss-win32/qga-vss.dll" KeyPath="yes" DiskId="1"/>
>
> Is there a reason these paths are seemingly relative to
> qga/installer/qemu-ga.wxs? I'd imagine that works for running wixl
> command within that directly, but top-level Makefile 'msi' ends up
> bombing if I do 'make msi':
>
> [mdroth@vm4 qemu-build-w64]$ ../w/qemu4.git/configure 
> --cross-prefix=x86_64-w64-mingw32- \
>   --with-vss-sdk=/home/mdroth/w/vss-win32/ --target-list=x86_64-softmmu
>   --extra-cflags=-Wall --enable-guest-agent --enable-guest-agent-msi
> ...
>
> [mdroth@vm4 qemu-build-w64]$ make -j4 qemu
> ...
>   CCtrace/generated-events.o
>   ARlibqemustub.a
>   CCqemu-img.o
>   CCqmp-marshal.o
>   ARlibqemuutil.a
>   LINK  qemu-ga.exe
>   LINK  qemu-img.exe
>   LINK  qemu-io.exe
> ...
>
> [mdroth@vm4 qemu-build-w64]$ make msi
>  LEX convert-dtsv0-lexer.lex.c
> make[1]: flex: Command not found
>  BISON dtc-parser.tab.c
> make[1]: bison: Command not found
>  LEX dtc-lexer.lex.c
> make[1]: flex: Command not found
>   WIXL  qemu-ga-x86_64.msi
> Couldn't find file ../../qemu-ga.exe
> make: *** [qemu-ga-x86_64.msi] Error 1
>
> The fix seems simple enough, but I want to make sure I'm not doing something
> stupid and have some idea of how people are using the current MSI installer
> code.
>

Re: [Qemu-devel] QEMU produces invalid JSON due to locale-dependent code

[Markus Armbruster]

"Daniel P. Berrange"  writes:

> On Wed, Aug 26, 2015 at 11:13:00AM +0200, Alberto Garcia wrote:
>> On Tue 25 Aug 2015 09:54:42 AM CEST, Markus Armbruster wrote:
>> 
>> >> (D) Run in a controlled mixed locale
>> >>GTK runs completely in the locale determined by setlocale() (since it
>> >> never has to display raw JSON)
>> >>We fix our JSON output code to use thread-specific locale
>> >> manipulations to (temporarily) switch to C locale before printing JSON
>> >>
>> >> that way, GTK still shows full German formatting for any localized
>> >> message in the GUI that happens to print numbers, but the JSON output
>> >> (which is independent of the GUI) also behaves correctly as a C-only
>> >> entity.
>> >
>> > Switching back to C locale whenever some unwanted locale-dependency
>> > breaks the code is problematic, because it involves finding all the
>> > places that break, iteratively (euphemism for "we debug one breakage
>> > after the other, adding temporary locale switches as we go).
>> 
>> For some cases we probably don't even need to switch the locale. For the
>> JSON case cannot we just easily convert the QFloat into a string
>> ourselves without using printf's "%f" ?
>
> FWIW, libvirt had this exact same problem with formatting doubles for
> JSON while non-C locales are in effect. We used the glibc thread-local
> locale support, and fallback to some sick string munging in non-glibc
> case:
[...]

For QEMU, I doubt sick string munging just to support GUI localization
on losing systems is worthwhile.

>> That doesn't invalidate however your point.
>> 
>> > I'd feel much better about confining GTK in its own thread, and
>> > setting only that thread's locale.
>> 
>> I haven't digged much into that part of the QEMU code, but if my
>> assumptions are true I think we would need at least:
>> 
>> - A separate GMainContext with its own main loop
>> - Making sure that all the code that touches the UI runs from that
>>   thread.
>> 
>> This is in principle possible, but I fear that we might be opening a can
>> of worms.
>
> Agreed, my experiance is that you should always run GTK in the main
> thread and never try todo anything more clever with threads + GTK.
> It has always ended in tears for me - even if you think you have it
> working on your system, you'll inevitably find a different version
> of GTK where it has broken. It just isn't worth the pain to try to
> run anything GTK related in a non-main thread.

If we can't move GTK out of the main thread, we need to move everything
else out.

One more option: instead of switching back to the C locale around
identified troublemakers (and then chase them down one by one, bug by
bug), switch away from it just around the GUI.  Thread-locally of
course.  Callbacks may require extra care.

We should've stayed out of the GUI business.

Re: [Qemu-devel] QEMU produces invalid JSON due to locale-dependent code

[Daniel P. Berrange]

On Wed, Aug 26, 2015 at 08:46:35AM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> > > It seems the only thing that we really care about being localized is
> > > the messages catalogue, so the GTK UI gets internationalization in
> > > its menus / dialogs / etc. As such I think that we should do the
> > > opposite of (C). ie run every LC_* in the C locale, except for
> > > LC_MESSAGES which we allow to be localized.
> > > 
> > > This avoids any unpredictable functional consequences (like number
> > > formatting) while still giving user decent localization in the UI
> > 
> > Except that the LC_MESSAGES catalog may include messages such as "blah
> > %d blah" that get translated for use as a printf argument, and the lack
> > of matching LC_NUMERIC will make the translation look wrong.
> > Translators often assume that their translation is being used with
> > everything else about the locale matching the locale of the translation.
> 
> I don't think this is a big issue in practice.  The menu item names are
> pretty much the only thing translated in the qemu gtk ui ...

Also we're talking about a tradeoff between functionally incorrect
formatting of JSON, vs "wrong looking" translations with no functional
impact, which probably won't even affect QEMU in reality.

In terms of minimizing hacks to QEMU, it seems like only localizing
LC_MESSAGES is a simple enough tradeoff to avoid functionally
incorrect behaviour with minimal downside and no code complexity
messing around with thread-locales, etc

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

Re: [Qemu-devel] [RFC PATCH v0 3/3] spapr: Memory hot-unplug support

[Bharata B Rao]

On Mon, Aug 24, 2015 at 09:39:31PM -0500, Michael Roth wrote:
> Quoting Bharata B Rao (2015-08-19 01:56:11)
> > Add support to hot remove pc-dimm memory devices.
> > 
> > Signed-off-by: Bharata B Rao 
> > ---
> >  hw/ppc/spapr.c | 114 
> > -
> >  hw/ppc/spapr_drc.c |  21 +
> >  include/hw/ppc/spapr.h |   2 +
> >  3 files changed, 136 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 06d000d..441012d 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -2110,6 +2110,109 @@ out:
> >  error_propagate(errp, local_err);
> >  }
> > 
> > +typedef struct sPAPRDIMMState {
> > +uint32_t nr_lmbs;
> > +} sPAPRDIMMState;
> > +
> > +/*
> > + * Called from spapr_drc.c: set_isolation_state().
> > + *
> > + * If the drc is being marked as ISOLATED, ensure that the corresponding
> > + * LMB is part of the DIMM device which is being deleted.
> > + */
> > +int spapr_lmb_in_removable_dimm(sPAPRDRConnector *drc,
> > +sPAPRDRIsolationState state)
> > +{
> > +DeviceState *dev = drc->dev;
> > +PCDIMMDevice *dimm = PC_DIMM(dev);
> > +
> > +if (state != SPAPR_DR_ISOLATION_STATE_ISOLATED) {
> > +return 0;
> > +}
> > +
> > +if (!dimm->delete_pending) {
> > +return -1;
> > +}
> > +
> > +return 0;
> > +}
> > +
> > +static void spapr_lmb_release(DeviceState *dev, void *opaque)
> > +{
> > +sPAPRDIMMState *ds = (sPAPRDIMMState *)opaque;
> > +HotplugHandler *hotplug_ctrl = NULL;
> > +Error *local_err = NULL;
> > +
> > +if (--ds->nr_lmbs) {
> > +return;
> > +}
> > +
> > +g_free(ds);
> > +
> > +/*
> > + * Now that all the LMBs have been removed by the guest, call the
> > + * pc-dimm unplug handler to cleanup up the pc-dimm device.
> > + */
> > +hotplug_ctrl = qdev_get_hotplug_handler(dev);
> > +hotplug_handler_unplug(hotplug_ctrl, dev, &local_err);
> > +}
> > +
> > +static void spapr_del_lmbs(DeviceState *dev, uint64_t addr, uint64_t size,
> > +   Error **errp)
> > +{
> > +sPAPRDRConnector *drc;
> > +sPAPRDRConnectorClass *drck;
> > +uint32_t nr_lmbs = size/SPAPR_MEMORY_BLOCK_SIZE;
> > +Error *local_err = NULL;
> > +int i;
> > +sPAPRDIMMState *ds = g_malloc0(sizeof(sPAPRDIMMState));
> > +
> > +ds->nr_lmbs = nr_lmbs;
> > +for (i = 0; i < nr_lmbs; i++) {
> > +drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_LMB,
> > +addr/SPAPR_MEMORY_BLOCK_SIZE);
> > +g_assert(drc);
> > +
> > +drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
> > +drck->detach(drc, dev, spapr_lmb_release, ds, &local_err);
> > +addr += SPAPR_MEMORY_BLOCK_SIZE;
> > +}
> > +spapr_hotplug_req_remove_by_count(SPAPR_DR_CONNECTOR_TYPE_LMB, 
> > nr_lmbs);
> > +}
> > +
> > +static void spapr_memory_unplug(HotplugHandler *hotplug_dev, DeviceState 
> > *dev,
> > +Error **errp)
> > +{
> > +sPAPRMachineState *ms = SPAPR_MACHINE(hotplug_dev);
> > +PCDIMMDevice *dimm = PC_DIMM(dev);
> > +PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
> > +MemoryRegion *mr = ddc->get_memory_region(dimm);
> > +
> > +pc_dimm_memory_unplug(dev, &ms->hotplug_memory, mr);
> > +object_unparent(OBJECT(dev));
> > +}
> 
> In the current code the unplug() and request_unplug() are mutually
> exclusive. Are the plans on making the unplug() do something in the
> prescence of request_unplug()? If so, I'd imagine it would've be a
> forced removal, except maybe as a fallback if the request is determined
> to fail somehow?

Like x86 memory hotremoval, our model too fits into async type of removal
where we first send removal notification to guest in ->unplug_request() and
when the guest indeed removes the memory, we cleanup the pc-dimm device
in ->unplug().

Since we implement both ->unplug() and ->unplug_request(), and given that
the removal works like above, I don't see why we would ever end up doing a
forced removal from ->unplug().

> > diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> > index 8cbcf4d..b9d7c71 100644
> > --- a/hw/ppc/spapr_drc.c
> > +++ b/hw/ppc/spapr_drc.c
> > @@ -11,6 +11,7 @@
> >   */
> > 
> >  #include "hw/ppc/spapr_drc.h"
> > +#include "hw/ppc/spapr.h"
> >  #include "qom/object.h"
> >  #include "hw/qdev.h"
> >  #include "qapi/visitor.h"
> > @@ -63,9 +64,29 @@ static int set_isolation_state(sPAPRDRConnector *drc,
> > sPAPRDRIsolationState state)
> >  {
> >  sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
> > +int ret;
> > 
> >  DPRINTFN("drc: %x, set_isolation_state: %x", get_index(drc), state);
> > 
> > +/*
> > + * Fail any requests to ISOLATE the LMB DRC if this LMB doesn't
> > + * belong to a DIMM device that is marked for removal.
> > + *
> > + * Currently the guest userspace tool dr

[Qemu-devel] [PATCH v8 04/11] netfilter: hook packets before net queue send

[Yang Hongyang]

Capture packets that will be sent.

Signed-off-by: Yang Hongyang 
---
v5: do not check ret against iov_size
pass sent_cb to filters
---
 net/net.c | 66 +++
 1 file changed, 66 insertions(+)

diff --git a/net/net.c b/net/net.c
index 74f3592..00cca83 100644
--- a/net/net.c
+++ b/net/net.c
@@ -562,6 +562,44 @@ int qemu_can_send_packet(NetClientState *sender)
 return 1;
 }
 
+static ssize_t filter_receive_iov(NetClientState *nc, int chain,
+  NetClientState *sender,
+  unsigned flags,
+  const struct iovec *iov,
+  int iovcnt,
+  NetPacketSent *sent_cb)
+{
+ssize_t ret = 0;
+NetFilterState *nf = NULL;
+
+QTAILQ_FOREACH(nf, &nc->filters, next) {
+if (nf->chain == chain || nf->chain == NET_FILTER_ALL) {
+ret = nf->info->receive_iov(nf, sender, flags,
+iov, iovcnt, sent_cb);
+if (ret) {
+return ret;
+}
+}
+}
+
+return ret;
+}
+
+static ssize_t filter_receive(NetClientState *nc, int chain,
+  NetClientState *sender,
+  unsigned flags,
+  const uint8_t *data,
+  size_t size,
+  NetPacketSent *sent_cb)
+{
+struct iovec iov = {
+.iov_base = (void *)data,
+.iov_len = size
+};
+
+return filter_receive_iov(nc, chain, sender, flags, &iov, 1, sent_cb);
+}
+
 ssize_t qemu_deliver_packet(NetClientState *sender,
 unsigned flags,
 const uint8_t *data,
@@ -633,6 +671,7 @@ static ssize_t 
qemu_send_packet_async_with_flags(NetClientState *sender,
  NetPacketSent *sent_cb)
 {
 NetQueue *queue;
+int ret;
 
 #ifdef DEBUG_NET
 printf("qemu_send_packet_async:\n");
@@ -643,6 +682,19 @@ static ssize_t 
qemu_send_packet_async_with_flags(NetClientState *sender,
 return size;
 }
 
+/* Let filters handle the packet first */
+ret = filter_receive(sender, NET_FILTER_OUT,
+ sender, flags, buf, size, sent_cb);
+if (ret) {
+return ret;
+}
+
+ret = filter_receive(sender->peer, NET_FILTER_IN,
+ sender, flags, buf, size, sent_cb);
+if (ret) {
+return ret;
+}
+
 queue = sender->peer->incoming_queue;
 
 return qemu_net_queue_send(queue, sender, flags, buf, size, sent_cb);
@@ -713,11 +765,25 @@ ssize_t qemu_sendv_packet_async(NetClientState *sender,
 NetPacketSent *sent_cb)
 {
 NetQueue *queue;
+int ret;
 
 if (sender->link_down || !sender->peer) {
 return iov_size(iov, iovcnt);
 }
 
+/* Let filters handle the packet first */
+ret = filter_receive_iov(sender, NET_FILTER_OUT, sender,
+ QEMU_NET_PACKET_FLAG_NONE, iov, iovcnt, sent_cb);
+if (ret) {
+return ret;
+}
+
+ret = filter_receive_iov(sender->peer, NET_FILTER_IN, sender,
+ QEMU_NET_PACKET_FLAG_NONE, iov, iovcnt, sent_cb);
+if (ret) {
+return ret;
+}
+
 queue = sender->peer->incoming_queue;
 
 return qemu_net_queue_send_iov(queue, sender,
-- 
1.9.1

[Qemu-devel] [PATCH v8 03/11] netfilter: add netfilter_{add|del} commands

[Yang Hongyang]

add netfilter_{add|del} commands
This is mostly the same with netdev_{add|del} commands.

When we delete the netdev, we also delete the netfilter object
attached to it, because if the netdev is removed, the filters
which attached to it is useless.

Signed-off-by: Yang Hongyang 
CC: Luiz Capitulino 
CC: Markus Armbruster 
CC: Eric Blake 
Reviewed-by: Thomas Huth 
---
v7: error msg fix
move qmp_opts_del() into qemu_del_net_filter()
v6: add multiqueue support (qemu_del_net_filter)
v5: squash "net: delete netfilter object when delete netdev"
---
 hmp-commands.hx  |  30 +++
 hmp.c|  29 +++
 hmp.h|   4 ++
 include/net/filter.h |   3 ++
 monitor.c|  33 +
 net/filter.c | 101 ++-
 net/net.c|   7 
 qapi-schema.json |  47 
 qmp-commands.hx  |  57 +
 9 files changed, 310 insertions(+), 1 deletion(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index d3b7932..902e2d1 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1253,6 +1253,36 @@ Remove host network device.
 ETEXI
 
 {
+.name   = "netfilter_add",
+.args_type  = "netfilter:O",
+.params = 
"[type],id=str,netdev=str[,chain=in|out|all,prop=value][,...]",
+.help   = "add netfilter",
+.mhandler.cmd = hmp_netfilter_add,
+.command_completion = netfilter_add_completion,
+},
+
+STEXI
+@item netfilter_add
+@findex netfilter_add
+Add netfilter.
+ETEXI
+
+{
+.name   = "netfilter_del",
+.args_type  = "id:s",
+.params = "id",
+.help   = "remove netfilter",
+.mhandler.cmd = hmp_netfilter_del,
+.command_completion = netfilter_del_completion,
+},
+
+STEXI
+@item netfilter_del
+@findex netfilter_del
+Remove netfilter.
+ETEXI
+
+{
 .name   = "object_add",
 .args_type  = "object:O",
 .params = "[qom-type=]type,id=str[,prop=value][,...]",
diff --git a/hmp.c b/hmp.c
index dcc66f1..09e3cda 100644
--- a/hmp.c
+++ b/hmp.c
@@ -15,6 +15,7 @@
 
 #include "hmp.h"
 #include "net/net.h"
+#include "net/filter.h"
 #include "net/eth.h"
 #include "sysemu/char.h"
 #include "sysemu/block-backend.h"
@@ -1599,6 +1600,34 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict)
 hmp_handle_error(mon, &err);
 }
 
+void hmp_netfilter_add(Monitor *mon, const QDict *qdict)
+{
+Error *err = NULL;
+QemuOpts *opts;
+
+opts = qemu_opts_from_qdict(qemu_find_opts("netfilter"), qdict, &err);
+if (err) {
+goto out;
+}
+
+netfilter_add(opts, &err);
+if (err) {
+qemu_opts_del(opts);
+}
+
+out:
+hmp_handle_error(mon, &err);
+}
+
+void hmp_netfilter_del(Monitor *mon, const QDict *qdict)
+{
+const char *id = qdict_get_str(qdict, "id");
+Error *err = NULL;
+
+qmp_netfilter_del(id, &err);
+hmp_handle_error(mon, &err);
+}
+
 void hmp_object_add(Monitor *mon, const QDict *qdict)
 {
 Error *err = NULL;
diff --git a/hmp.h b/hmp.h
index 0cf4f2a..a21dbbb 100644
--- a/hmp.h
+++ b/hmp.h
@@ -85,6 +85,8 @@ void hmp_device_del(Monitor *mon, const QDict *qdict);
 void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict);
 void hmp_netdev_add(Monitor *mon, const QDict *qdict);
 void hmp_netdev_del(Monitor *mon, const QDict *qdict);
+void hmp_netfilter_add(Monitor *mon, const QDict *qdict);
+void hmp_netfilter_del(Monitor *mon, const QDict *qdict);
 void hmp_getfd(Monitor *mon, const QDict *qdict);
 void hmp_closefd(Monitor *mon, const QDict *qdict);
 void hmp_sendkey(Monitor *mon, const QDict *qdict);
@@ -112,6 +114,8 @@ void chardev_add_completion(ReadLineState *rs, int nb_args, 
const char *str);
 void set_link_completion(ReadLineState *rs, int nb_args, const char *str);
 void netdev_add_completion(ReadLineState *rs, int nb_args, const char *str);
 void netdev_del_completion(ReadLineState *rs, int nb_args, const char *str);
+void netfilter_add_completion(ReadLineState *rs, int nb_args, const char *str);
+void netfilter_del_completion(ReadLineState *rs, int nb_args, const char *str);
 void ringbuf_write_completion(ReadLineState *rs, int nb_args, const char *str);
 void watchdog_action_completion(ReadLineState *rs, int nb_args,
 const char *str);
diff --git a/include/net/filter.h b/include/net/filter.h
index 7a858d8..f15d83d 100644
--- a/include/net/filter.h
+++ b/include/net/filter.h
@@ -53,5 +53,8 @@ NetFilterState *qemu_new_net_filter(NetFilterInfo *info,
 NetClientState *netdev,
 const char *name,
 int chain);
+void qemu_del_net_filter(NetFilterState *nf);
+void netfilter_add(QemuOpts *opts, Error **errp);
+void qmp_netfilter_add(QDict *qdict, QObject **ret, Error **errp);
 
 #endif /* QEMU_NET_FILTER_H */
diff --git a/mo

[Qemu-devel] [PATCH v8 01/11] net: add a new object netfilter

[Yang Hongyang]

Add the framework for a new netfilter object and a new
-netfilter CLI option as a basis for the following patches.

Signed-off-by: Yang Hongyang 
CC: Paolo Bonzini 
CC: Eric Blake 
Reviewed-by: Thomas Huth 
---
 include/net/filter.h| 15 +++
 include/sysemu/sysemu.h |  1 +
 net/Makefile.objs   |  1 +
 net/filter.c| 27 +++
 qemu-options.hx |  1 +
 vl.c| 13 +
 6 files changed, 58 insertions(+)
 create mode 100644 include/net/filter.h
 create mode 100644 net/filter.c

diff --git a/include/net/filter.h b/include/net/filter.h
new file mode 100644
index 000..4242ded
--- /dev/null
+++ b/include/net/filter.h
@@ -0,0 +1,15 @@
+/*
+ * Copyright (c) 2015 FUJITSU LIMITED
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later.  See the COPYING file in the top-level directory.
+ */
+
+#ifndef QEMU_NET_FILTER_H
+#define QEMU_NET_FILTER_H
+
+#include "qemu-common.h"
+
+int net_init_filters(void);
+
+#endif /* QEMU_NET_FILTER_H */
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 44570d1..15d6d00 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -212,6 +212,7 @@ extern QemuOptsList qemu_chardev_opts;
 extern QemuOptsList qemu_device_opts;
 extern QemuOptsList qemu_netdev_opts;
 extern QemuOptsList qemu_net_opts;
+extern QemuOptsList qemu_netfilter_opts;
 extern QemuOptsList qemu_global_opts;
 extern QemuOptsList qemu_mon_opts;
 
diff --git a/net/Makefile.objs b/net/Makefile.objs
index ec19cb3..914aec0 100644
--- a/net/Makefile.objs
+++ b/net/Makefile.objs
@@ -13,3 +13,4 @@ common-obj-$(CONFIG_HAIKU) += tap-haiku.o
 common-obj-$(CONFIG_SLIRP) += slirp.o
 common-obj-$(CONFIG_VDE) += vde.o
 common-obj-$(CONFIG_NETMAP) += netmap.o
+common-obj-y += filter.o
diff --git a/net/filter.c b/net/filter.c
new file mode 100644
index 000..4e40f08
--- /dev/null
+++ b/net/filter.c
@@ -0,0 +1,27 @@
+/*
+ * Copyright (c) 2015 FUJITSU LIMITED
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later.  See the COPYING file in the top-level directory.
+ */
+
+#include "qemu-common.h"
+#include "net/filter.h"
+
+int net_init_filters(void)
+{
+return 0;
+}
+
+QemuOptsList qemu_netfilter_opts = {
+.name = "netfilter",
+.implied_opt_name = "type",
+.head = QTAILQ_HEAD_INITIALIZER(qemu_netfilter_opts.head),
+.desc = {
+/*
+ * no elements => accept any params
+ * validation will happen later
+ */
+{ /* end of list */ }
+},
+};
diff --git a/qemu-options.hx b/qemu-options.hx
index 77f5853..0d52d02 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1575,6 +1575,7 @@ DEF("net", HAS_ARG, QEMU_OPTION_net,
 "socket][,vlan=n][,option][,option][,...]\n"
 "old way to initialize a host network interface\n"
 "(use the -netdev option if possible instead)\n", 
QEMU_ARCH_ALL)
+DEF("netfilter", HAS_ARG, QEMU_OPTION_netfilter, "", QEMU_ARCH_ALL)
 STEXI
 @item -net nic[,vlan=@var{n}][,macaddr=@var{mac}][,model=@var{type}] 
[,name=@var{name}][,addr=@var{addr}][,vectors=@var{v}]
 @findex -net
diff --git a/vl.c b/vl.c
index 584ca88..aee931a 100644
--- a/vl.c
+++ b/vl.c
@@ -75,6 +75,7 @@ int main(int argc, char **argv)
 #include "monitor/qdev.h"
 #include "sysemu/bt.h"
 #include "net/net.h"
+#include "net/filter.h"
 #include "net/slirp.h"
 #include "monitor/monitor.h"
 #include "ui/console.h"
@@ -2998,6 +2999,7 @@ int main(int argc, char **argv, char **envp)
 qemu_add_opts(&qemu_device_opts);
 qemu_add_opts(&qemu_netdev_opts);
 qemu_add_opts(&qemu_net_opts);
+qemu_add_opts(&qemu_netfilter_opts);
 qemu_add_opts(&qemu_rtc_opts);
 qemu_add_opts(&qemu_global_opts);
 qemu_add_opts(&qemu_mon_opts);
@@ -3284,6 +3286,13 @@ int main(int argc, char **argv, char **envp)
 exit(1);
 }
 break;
+case QEMU_OPTION_netfilter:
+opts = qemu_opts_parse_noisily(qemu_find_opts("netfilter"),
+   optarg, true);
+if (!opts) {
+exit(1);
+}
+break;
 #ifdef CONFIG_LIBISCSI
 case QEMU_OPTION_iscsi:
 opts = qemu_opts_parse_noisily(qemu_find_opts("iscsi"),
@@ -4413,6 +4422,10 @@ int main(int argc, char **argv, char **envp)
 exit(1);
 }
 
+if (net_init_filters() < 0) {
+exit(1);
+}
+
 #ifdef CONFIG_TPM
 if (tpm_init() < 0) {
 exit(1);
-- 
1.9.1

[Qemu-devel] [PATCH v8 08/11] net/queue: export qemu_net_queue_append_iov

[Yang Hongyang]

Signed-off-by: Yang Hongyang 
---
 include/net/queue.h |  7 +++
 net/queue.c | 12 ++--
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/include/net/queue.h b/include/net/queue.h
index 1d65e47..e139cc7 100644
--- a/include/net/queue.h
+++ b/include/net/queue.h
@@ -55,6 +55,13 @@ struct NetQueue {
 
 NetQueue *qemu_new_net_queue(void *opaque);
 
+void qemu_net_queue_append_iov(NetQueue *queue,
+   NetClientState *sender,
+   unsigned flags,
+   const struct iovec *iov,
+   int iovcnt,
+   NetPacketSent *sent_cb);
+
 void qemu_del_net_queue(NetQueue *queue);
 
 ssize_t qemu_net_queue_send(NetQueue *queue,
diff --git a/net/queue.c b/net/queue.c
index 1499479..428fdd6 100644
--- a/net/queue.c
+++ b/net/queue.c
@@ -91,12 +91,12 @@ static void qemu_net_queue_append(NetQueue *queue,
 QTAILQ_INSERT_TAIL(&queue->packets, packet, entry);
 }
 
-static void qemu_net_queue_append_iov(NetQueue *queue,
-  NetClientState *sender,
-  unsigned flags,
-  const struct iovec *iov,
-  int iovcnt,
-  NetPacketSent *sent_cb)
+void qemu_net_queue_append_iov(NetQueue *queue,
+   NetClientState *sender,
+   unsigned flags,
+   const struct iovec *iov,
+   int iovcnt,
+   NetPacketSent *sent_cb)
 {
 NetPacket *packet;
 size_t max_len = 0;
-- 
1.9.1

[Qemu-devel] [PATCH v8 02/11] init/cleanup of netfilter object

[Yang Hongyang]

QTAILQ_ENTRY global_list but used by filter layer, so that we can
manage all filters together.
QTAILQ_ENTRY next used by netdev, filter belongs to the specific netdev is
in this queue.
This is mostly the same with init/cleanup of netdev object.

Signed-off-by: Yang Hongyang 
---
v8: include vhost_net header
v7: add check for vhost
fix error propagate bug
v6: add multiqueue support (net_filter_init1)
v5: remove model from NetFilterState
add a sent_cb param to receive_iov API
---
 include/net/filter.h|  42 ++
 include/net/net.h   |   1 +
 include/qemu/typedefs.h |   1 +
 net/filter.c| 149 
 net/net.c   |   1 +
 qapi-schema.json|  37 
 6 files changed, 231 insertions(+)

diff --git a/include/net/filter.h b/include/net/filter.h
index 4242ded..7a858d8 100644
--- a/include/net/filter.h
+++ b/include/net/filter.h
@@ -9,7 +9,49 @@
 #define QEMU_NET_FILTER_H
 
 #include "qemu-common.h"
+#include "qemu/typedefs.h"
+#include "net/queue.h"
+
+/* the netfilter chain */
+enum {
+NET_FILTER_IN,
+NET_FILTER_OUT,
+NET_FILTER_ALL,
+};
+
+typedef void (FilterCleanup) (NetFilterState *);
+/*
+ * Return:
+ *   0: finished handling the packet, we should continue
+ *   size: filter stolen this packet, we stop pass this packet further
+ */
+typedef ssize_t (FilterReceiveIOV)(NetFilterState *nc,
+   NetClientState *sender,
+   unsigned flags,
+   const struct iovec *iov,
+   int iovcnt,
+   NetPacketSent *sent_cb);
+
+typedef struct NetFilterInfo {
+NetFilterOptionsKind type;
+size_t size;
+FilterCleanup *cleanup;
+FilterReceiveIOV *receive_iov;
+} NetFilterInfo;
+
+struct NetFilterState {
+NetFilterInfo *info;
+char *name;
+NetClientState *netdev;
+int chain;
+QTAILQ_ENTRY(NetFilterState) global_list;
+QTAILQ_ENTRY(NetFilterState) next;
+};
 
 int net_init_filters(void);
+NetFilterState *qemu_new_net_filter(NetFilterInfo *info,
+NetClientState *netdev,
+const char *name,
+int chain);
 
 #endif /* QEMU_NET_FILTER_H */
diff --git a/include/net/net.h b/include/net/net.h
index 6a6cbef..36e5fab 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -92,6 +92,7 @@ struct NetClientState {
 NetClientDestructor *destructor;
 unsigned int queue_index;
 unsigned rxfilter_notify_enabled:1;
+QTAILQ_HEAD(, NetFilterState) filters;
 };
 
 typedef struct NICState {
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index f8a9dd6..2c0648f 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -45,6 +45,7 @@ typedef struct Monitor Monitor;
 typedef struct MouseTransformInfo MouseTransformInfo;
 typedef struct MSIMessage MSIMessage;
 typedef struct NetClientState NetClientState;
+typedef struct NetFilterState NetFilterState;
 typedef struct NICInfo NICInfo;
 typedef struct PcGuestInfo PcGuestInfo;
 typedef struct PCIBridge PCIBridge;
diff --git a/net/filter.c b/net/filter.c
index 4e40f08..cb23384 100644
--- a/net/filter.c
+++ b/net/filter.c
@@ -6,10 +6,159 @@
  */
 
 #include "qemu-common.h"
+#include "qapi-visit.h"
+#include "qapi/qmp/qerror.h"
+#include "qemu/error-report.h"
+#include "qapi-visit.h"
+#include "qapi/opts-visitor.h"
+#include "qapi/dealloc-visitor.h"
+#include "qemu/config-file.h"
+
 #include "net/filter.h"
+#include "net/net.h"
+#include "net/vhost_net.h"
+
+static QTAILQ_HEAD(, NetFilterState) net_filters;
+
+NetFilterState *qemu_new_net_filter(NetFilterInfo *info,
+NetClientState *netdev,
+const char *name,
+int chain)
+{
+NetFilterState *nf;
+
+assert(info->size >= sizeof(NetFilterState));
+assert(info->receive_iov);
+
+nf = g_malloc0(info->size);
+nf->info = info;
+nf->name = g_strdup(name);
+nf->netdev = netdev;
+nf->chain = chain;
+QTAILQ_INSERT_TAIL(&net_filters, nf, global_list);
+QTAILQ_INSERT_TAIL(&netdev->filters, nf, next);
+
+return nf;
+}
+
+static inline void qemu_cleanup_net_filter(NetFilterState *nf)
+{
+QTAILQ_REMOVE(&nf->netdev->filters, nf, next);
+QTAILQ_REMOVE(&net_filters, nf, global_list);
+
+if (nf->info->cleanup) {
+nf->info->cleanup(nf);
+}
+
+g_free(nf->name);
+g_free(nf);
+}
+
+typedef int (NetFilterInit)(const NetFilterOptions *opts,
+const char *name, int chain,
+NetClientState *netdev, Error **errp);
+
+static
+NetFilterInit * const net_filter_init_fun[NET_FILTER_OPTIONS_KIND_MAX] = {
+};
+
+static int net_filter_init1(const NetFilter *netfilter, Error **errp)
+{
+NetClientState *ncs

[Qemu-devel] [PATCH v8 00/11] Add a netfilter object and netbuffer filter

[Yang Hongyang]

This patch add a new object netfilter, capture all network packets.
Also implement a netbuffer based on this object.
the "buffer" netfilter could be used by VM FT solutions like
MicroCheckpointing, to buffer/release packets. Or to simulate
packet delay.

You can also get the series from:
https://github.com/macrosheep/qemu/tree/netfilter-v8

Usage:
 -netdev tap,id=bn0
 -netfilter buffer,id=f0,netdev=bn0,chain=in,interval=1000
 -device e1000,netdev=bn0

dynamically add/remove netfilters:
 netfilter_add buffer,id=f0,netdev=bn0,chain=in,interval=1000
 netfilter_del f0

NOTE:
 interval's scale is microsecond.
 chain is optional, and is one of in|out|all, default is "all".
   "in" means this filter will receive packets sent to the @netdev
   "out" means this filter will receive packets sent from the @netdev
   "all" means this filter will receive packets both sent to/from
 the @netdev

TODO:
 - dump

v8:
 - some minor fixes according to Thomas's comments
 - rebased to the latest master branch

v7:
 - print filter info when execute 'info network'
 - addressed Jason's comments

v6:
 - add multiqueue support, please see individual patch for detail

v5:
 - add a sent_cb param to filter receive_iov api
 - squash the 4th patch into patch 3
 - remove dummy sent_cb (buffer filter)
 - addressed Jason's other comments, see individual patches for detail

v4:
 - get rid of struct Filter
 - squash the 4th patch into patch 2
 - fix qemu_netfilter_pass_to_next_iov
 - get rid of bh (buffer filter)
 - release the packet to next filter instead of to receiver (buffer filter)

v3:
 - add an api to pass the packet to next filter
 - remove netfilters when delete netdev
 - add qtest testcases for netfilter
 - addressed comments from Jason

v2:
 - add a chain option to netfilter object
 - move the hook place earlier, before net_queue_send
 - drop the unused api in buffer filter
 - squash buffer filter patches into one
 - remove receive() api from netfilter, only receive_iov() is enough
 - addressed comments from Jason&Thomas

v1:
 initial patch.

Yang Hongyang (11):
  net: add a new object netfilter
  init/cleanup of netfilter object
  netfilter: add netfilter_{add|del} commands
  netfilter: hook packets before net queue send
  move out net queue structs define
  netfilter: add an API to pass the packet to next filter
  netfilter: print filter info associate with the netdev
  net/queue: export qemu_net_queue_append_iov
  netfilter: add a netbuffer filter
  filter/buffer: update command description and help
  tests: add test cases for netfilter object

 hmp-commands.hx |  30 +
 hmp.c   |  29 +
 hmp.h   |   4 +
 include/net/filter.h|  64 ++
 include/net/net.h   |   1 +
 include/net/queue.h |  26 
 include/qemu/typedefs.h |   1 +
 include/sysemu/sysemu.h |   1 +
 monitor.c   |  33 +
 net/Makefile.objs   |   2 +
 net/filter-buffer.c | 125 ++
 net/filter.c| 332 
 net/filters.h   |  17 +++
 net/net.c   |  85 +
 net/queue.c |  31 +
 qapi-schema.json| 100 +++
 qemu-options.hx |  17 +++
 qmp-commands.hx |  57 +
 tests/.gitignore|   1 +
 tests/Makefile  |   2 +
 tests/test-netfilter.c  | 194 
 vl.c|  13 ++
 22 files changed, 1140 insertions(+), 25 deletions(-)
 create mode 100644 include/net/filter.h
 create mode 100644 net/filter-buffer.c
 create mode 100644 net/filter.c
 create mode 100644 net/filters.h
 create mode 100644 tests/test-netfilter.c

-- 
1.9.1

[Qemu-devel] [PATCH v8 07/11] netfilter: print filter info associate with the netdev

[Yang Hongyang]

From: Yang Hongyang 

When execute "info network", print filter info also.
current info printed is simple, can add more info later.

Signed-off-by: Yang Hongyang 
---
v7: initial patch
---
 include/net/filter.h |  1 +
 net/filter.c | 22 ++
 net/net.c| 11 +++
 3 files changed, 34 insertions(+)

diff --git a/include/net/filter.h b/include/net/filter.h
index 9278d40..188ecb1 100644
--- a/include/net/filter.h
+++ b/include/net/filter.h
@@ -56,6 +56,7 @@ NetFilterState *qemu_new_net_filter(NetFilterInfo *info,
 void qemu_del_net_filter(NetFilterState *nf);
 void netfilter_add(QemuOpts *opts, Error **errp);
 void qmp_netfilter_add(QDict *qdict, QObject **ret, Error **errp);
+const char *qemu_netfilter_get_chain_str(int chain);
 
 /* pass the packet to the next filter */
 ssize_t qemu_netfilter_pass_to_next(NetFilterState *nf, NetPacket *packet);
diff --git a/net/filter.c b/net/filter.c
index 44c17b0..76e12ea 100644
--- a/net/filter.c
+++ b/net/filter.c
@@ -23,6 +23,28 @@
 
 static QTAILQ_HEAD(, NetFilterState) net_filters;
 
+const char *qemu_netfilter_get_chain_str(int chain)
+{
+const char *str = NULL;
+
+switch (chain) {
+case NET_FILTER_IN:
+str = "in";
+break;
+case NET_FILTER_OUT:
+str = "out";
+break;
+case NET_FILTER_ALL:
+str = "all";
+break;
+default:
+str = "unknown";
+break;
+}
+
+return str;
+}
+
 NetFilterState *qemu_new_net_filter(NetFilterInfo *info,
 NetClientState *netdev,
 const char *name,
diff --git a/net/net.c b/net/net.c
index 00cca83..99f0e87 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1199,10 +1199,21 @@ void qmp_netdev_del(const char *id, Error **errp)
 
 void print_net_client(Monitor *mon, NetClientState *nc)
 {
+NetFilterState *nf;
+
 monitor_printf(mon, "%s: index=%d,type=%s,%s\n", nc->name,
nc->queue_index,
NetClientOptionsKind_lookup[nc->info->type],
nc->info_str);
+if (!QTAILQ_EMPTY(&nc->filters)) {
+monitor_printf(mon, "filters:\n");
+}
+QTAILQ_FOREACH(nf, &nc->filters, next) {
+monitor_printf(mon, "  - %s: type=%s,netdev=%s,chain=%s\n", nf->name,
+   NetFilterOptionsKind_lookup[nf->info->type],
+   nf->netdev->name,
+   qemu_netfilter_get_chain_str(nf->chain));
+}
 }
 
 RxFilterInfoList *qmp_query_rx_filter(bool has_name, const char *name,
-- 
1.9.1

[Qemu-devel] [PATCH v8 05/11] move out net queue structs define

[Yang Hongyang]

Signed-off-by: Yang Hongyang 
---
 include/net/queue.h | 19 +++
 net/queue.c | 19 ---
 2 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/include/net/queue.h b/include/net/queue.h
index fc02b33..1d65e47 100644
--- a/include/net/queue.h
+++ b/include/net/queue.h
@@ -31,6 +31,25 @@ typedef struct NetQueue NetQueue;
 
 typedef void (NetPacketSent) (NetClientState *sender, ssize_t ret);
 
+struct NetPacket {
+QTAILQ_ENTRY(NetPacket) entry;
+NetClientState *sender;
+unsigned flags;
+int size;
+NetPacketSent *sent_cb;
+uint8_t data[0];
+};
+
+struct NetQueue {
+void *opaque;
+uint32_t nq_maxlen;
+uint32_t nq_count;
+
+QTAILQ_HEAD(packets, NetPacket) packets;
+
+unsigned delivering:1;
+};
+
 #define QEMU_NET_PACKET_FLAG_NONE  0
 #define QEMU_NET_PACKET_FLAG_RAW  (1<<0)
 
diff --git a/net/queue.c b/net/queue.c
index ebbe2bb..1499479 100644
--- a/net/queue.c
+++ b/net/queue.c
@@ -39,25 +39,6 @@
  * unbounded queueing.
  */
 
-struct NetPacket {
-QTAILQ_ENTRY(NetPacket) entry;
-NetClientState *sender;
-unsigned flags;
-int size;
-NetPacketSent *sent_cb;
-uint8_t data[0];
-};
-
-struct NetQueue {
-void *opaque;
-uint32_t nq_maxlen;
-uint32_t nq_count;
-
-QTAILQ_HEAD(packets, NetPacket) packets;
-
-unsigned delivering : 1;
-};
-
 NetQueue *qemu_new_net_queue(void *opaque)
 {
 NetQueue *queue;
-- 
1.9.1

[Qemu-devel] [PATCH v8 09/11] netfilter: add a netbuffer filter

[Yang Hongyang]

This filter is to buffer/release packets, this feature can be used
when using MicroCheckpointing, or other Remus like VM FT solutions, you
can also use it to simulate the network delay.
It has an interval option, if supplied, this filter will release
packets by interval.

Usage:
 -netdev tap,id=bn0
 -netfilter buffer,id=f0,netdev=bn0,chain=in,interval=1000

NOTE:
 the scale of interval is microsecond.

Signed-off-by: Yang Hongyang 
---
v7: use QTAILQ_FOREACH_SAFE() when flush packets
v6: move the interval check earlier and some comment adjust
v5: remove dummy sent_cb
change interval type from int64 to uint32
check interval!=0 when initialise
rename FILTERBUFFERState to FilterBufferState
v4: remove bh
pass the packet to next filter instead of receiver
v3: check packet's sender and sender->peer when flush it

fix for netbuffer
---
 net/Makefile.objs   |   1 +
 net/filter-buffer.c | 125 
 net/filter.c|   2 +
 net/filters.h   |  17 +++
 qapi-schema.json|  18 +++-
 5 files changed, 162 insertions(+), 1 deletion(-)
 create mode 100644 net/filter-buffer.c
 create mode 100644 net/filters.h

diff --git a/net/Makefile.objs b/net/Makefile.objs
index 914aec0..5fa2f97 100644
--- a/net/Makefile.objs
+++ b/net/Makefile.objs
@@ -14,3 +14,4 @@ common-obj-$(CONFIG_SLIRP) += slirp.o
 common-obj-$(CONFIG_VDE) += vde.o
 common-obj-$(CONFIG_NETMAP) += netmap.o
 common-obj-y += filter.o
+common-obj-y += filter-buffer.o
diff --git a/net/filter-buffer.c b/net/filter-buffer.c
new file mode 100644
index 000..622ac54
--- /dev/null
+++ b/net/filter-buffer.c
@@ -0,0 +1,125 @@
+/*
+ * Copyright (c) 2015 FUJITSU LIMITED
+ * Author: Yang Hongyang 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later.  See the COPYING file in the top-level directory.
+ */
+
+#include "net/filter.h"
+#include "net/queue.h"
+#include "filters.h"
+#include "qemu-common.h"
+#include "qemu/timer.h"
+#include "qemu/iov.h"
+#include "qapi/qmp/qerror.h"
+
+typedef struct FilterBufferState {
+NetFilterState nf;
+NetQueue *incoming_queue;
+uint32_t interval;
+QEMUTimer release_timer;
+} FilterBufferState;
+
+static void filter_buffer_flush(NetFilterState *nf)
+{
+FilterBufferState *s = DO_UPCAST(FilterBufferState, nf, nf);
+NetQueue *queue = s->incoming_queue;
+NetPacket *packet, *next;
+
+QTAILQ_FOREACH_SAFE(packet, &queue->packets, entry, next) {
+QTAILQ_REMOVE(&queue->packets, packet, entry);
+queue->nq_count--;
+
+if (packet->sender && packet->sender->peer) {
+qemu_netfilter_pass_to_next(nf, packet);
+}
+
+/*
+ * now that we have passed the packet to next filter (or there's
+ * no receiver). If it's queued by receiver's incoming_queue, there
+ * will be a copy of the packet->data, so simply free this packet
+ * now.
+ */
+g_free(packet);
+}
+}
+
+static void filter_buffer_release_timer(void *opaque)
+{
+FilterBufferState *s = opaque;
+filter_buffer_flush(&s->nf);
+timer_mod(&s->release_timer,
+  qemu_clock_get_us(QEMU_CLOCK_VIRTUAL) + s->interval);
+}
+
+/* filter APIs */
+static ssize_t filter_buffer_receive_iov(NetFilterState *nf,
+ NetClientState *sender,
+ unsigned flags,
+ const struct iovec *iov,
+ int iovcnt,
+ NetPacketSent *sent_cb)
+{
+FilterBufferState *s = DO_UPCAST(FilterBufferState, nf, nf);
+NetQueue *queue = s->incoming_queue;
+
+qemu_net_queue_append_iov(queue, sender, flags, iov, iovcnt, sent_cb);
+return iov_size(iov, iovcnt);
+}
+
+static void filter_buffer_cleanup(NetFilterState *nf)
+{
+FilterBufferState *s = DO_UPCAST(FilterBufferState, nf, nf);
+
+if (s->interval) {
+timer_del(&s->release_timer);
+}
+
+/* flush packets */
+filter_buffer_flush(nf);
+g_free(s->incoming_queue);
+return;
+}
+
+static NetFilterInfo net_filter_buffer_info = {
+.type = NET_FILTER_OPTIONS_KIND_BUFFER,
+.size = sizeof(FilterBufferState),
+.receive_iov = filter_buffer_receive_iov,
+.cleanup = filter_buffer_cleanup,
+};
+
+int net_init_filter_buffer(const NetFilterOptions *opts, const char *name,
+   int chain, NetClientState *netdev, Error **errp)
+{
+NetFilterState *nf;
+FilterBufferState *s;
+const NetFilterBufferOptions *bufferopt;
+int interval;
+
+assert(opts->kind == NET_FILTER_OPTIONS_KIND_BUFFER);
+bufferopt = opts->buffer;
+/*
+ * this check will be dropped when there're VM FT solutions like MC
+ * or COLO use this filter to release packets on demand.
+ */
+interval = bufferopt->has_interval ? bufferopt->interval : 0;
+if (!interval) {
+   

[Qemu-devel] [PATCH v8 11/11] tests: add test cases for netfilter object

[Yang Hongyang]

Using qtest qmp interface to implement following cases:
1) add/remove netfilter
2) add a netfilter then delete the netdev
3) add/remove more than one netfilters
4) add more than one netfilters and then delete the netdev

Signed-off-by: Yang Hongyang 
---
 tests/.gitignore   |   1 +
 tests/Makefile |   2 +
 tests/test-netfilter.c | 194 +
 3 files changed, 197 insertions(+)
 create mode 100644 tests/test-netfilter.c

diff --git a/tests/.gitignore b/tests/.gitignore
index ccc92e4..395962b 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -41,5 +41,6 @@ test-vmstate
 test-write-threshold
 test-x86-cpuid
 test-xbzrle
+test-netfilter
 *-test
 qapi-schema/*.test.*
diff --git a/tests/Makefile b/tests/Makefile
index 5271123..bc59ad8 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -185,6 +185,7 @@ check-qtest-i386-y += tests/pc-cpu-test$(EXESUF)
 check-qtest-i386-y += tests/q35-test$(EXESUF)
 gcov-files-i386-y += hw/pci-host/q35.c
 check-qtest-i386-$(CONFIG_LINUX) += tests/vhost-user-test$(EXESUF)
+check-qtest-i386-y += tests/test-netfilter$(EXESUF)
 check-qtest-x86_64-y = $(check-qtest-i386-y)
 gcov-files-i386-y += i386-softmmu/hw/timer/mc146818rtc.c
 gcov-files-x86_64-y = $(subst 
i386-softmmu/,x86_64-softmmu/,$(gcov-files-i386-y))
@@ -409,6 +410,7 @@ tests/vhost-user-test$(EXESUF): tests/vhost-user-test.o 
qemu-char.o qemu-timer.o
 tests/qemu-iotests/socket_scm_helper$(EXESUF): 
tests/qemu-iotests/socket_scm_helper.o
 tests/test-qemu-opts$(EXESUF): tests/test-qemu-opts.o libqemuutil.a 
libqemustub.a
 tests/test-write-threshold$(EXESUF): tests/test-write-threshold.o 
$(block-obj-y) libqemuutil.a libqemustub.a
+tests/test-netfilter$(EXESUF): tests/test-netfilter.o $(qtest-obj-y)
 
 ifeq ($(CONFIG_POSIX),y)
 LIBS += -lutil
diff --git a/tests/test-netfilter.c b/tests/test-netfilter.c
new file mode 100644
index 000..acbea4c
--- /dev/null
+++ b/tests/test-netfilter.c
@@ -0,0 +1,194 @@
+/*
+ * QTest testcase for netfilter
+ *
+ * Copyright (c) 2015 FUJITSU LIMITED
+ * Author: Yang Hongyang 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later.  See the COPYING file in the top-level directory.
+ */
+
+#include 
+#include "libqtest.h"
+
+/* add a netfilter to a netdev and then remove it */
+static void add_one_netfilter(void)
+{
+QDict *response;
+
+response = qmp("{\"execute\": \"netfilter_add\","
+   " \"arguments\": {"
+   "   \"type\": \"buffer\","
+   "   \"id\": \"qtest-f0\","
+   "   \"netdev\": \"qtest-bn0\","
+   "   \"chain\": \"in\","
+   "   \"interval\": \"1000\""
+   "}}");
+
+g_assert(response);
+g_assert(!qdict_haskey(response, "error"));
+QDECREF(response);
+
+response = qmp("{\"execute\": \"netfilter_del\","
+   " \"arguments\": {"
+   "   \"id\": \"qtest-f0\""
+   "}}");
+g_assert(response);
+g_assert(!qdict_haskey(response, "error"));
+QDECREF(response);
+}
+
+/* add a netfilter to a netdev and then remove the netdev */
+static void remove_netdev_with_one_netfilter(void)
+{
+QDict *response;
+
+response = qmp("{\"execute\": \"netfilter_add\","
+   " \"arguments\": {"
+   "   \"type\": \"buffer\","
+   "   \"id\": \"qtest-f0\","
+   "   \"netdev\": \"qtest-bn0\","
+   "   \"chain\": \"in\","
+   "   \"interval\": \"1000\""
+   "}}");
+
+g_assert(response);
+g_assert(!qdict_haskey(response, "error"));
+QDECREF(response);
+
+response = qmp("{\"execute\": \"netdev_del\","
+   " \"arguments\": {"
+   "   \"id\": \"qtest-bn0\""
+   "}}");
+g_assert(response);
+g_assert(!qdict_haskey(response, "error"));
+QDECREF(response);
+
+/* add back the netdev */
+response = qmp("{\"execute\": \"netdev_add\","
+   " \"arguments\": {"
+   "   \"type\": \"user\","
+   "   \"id\": \"qtest-bn0\""
+   "}}");
+g_assert(response);
+g_assert(!qdict_haskey(response, "error"));
+QDECREF(response);
+}
+
+/* add multi(2) netfilters to a netdev and then remove them */
+static void add_multi_netfilter(void)
+{
+QDict *response;
+
+response = qmp("{\"execute\": \"netfilter_add\","
+   " \"arguments\": {"
+   "   \"type\": \"buffer\","
+   "   \"id\": \"qtest-f0\","
+   "   \"netdev\": \"qtest-bn0\","
+   "   \"chain\": \"in\","
+   "   \"interval\": \"1000\""
+   "}}");
+
+g_assert(response);
+g_assert(!qdict_haskey(response, "error"));
+QDECREF(response);
+
+response = qmp("{\"execute\": \"netfilter_add\","
+   " 

[Qemu-devel] [PATCH v3 00/12] qemu-ga: add a configuration file

[marcandre . lureau]

From: Marc-André Lureau 

Hi,

The following patches for the qemu agent add support for an optionnal
configuration file, and a man page.

Since v1:
- spelling fixes
- change device_path to channel_path
- moving config to GAConfig struct
- do check_is_frozen() during main
- use g_key_file_to_data() for the dump
Since v2:
- fix compilation in intermediate patch
- remove some extra space in intermediate patch
- add some missing Reviewed-by tags

This is related to this RFE:
https://bugzilla.redhat.com/show_bug.cgi?id=1101556

Marc-André Lureau (12):
  qga: misc spelling
  qga: use exit() when parsing options
  qga: move string split in separate function
  qga: rename 'path' to 'channel_path'
  qga: copy argument strings
  qga: move option parsing to separate function
  qga: fill default options in main()
  qga: move agent run in a separate function
  qga: free a bit more
  qga: add an optionnal qemu-ga.conf system configuration
  qga: add --dump-conf option
  qga: start a man page

 Makefile |  14 +-
 qemu-doc.texi|   6 +
 qemu-ga.texi | 136 +++
 qga/main.c   | 464 ---
 qga/qapi-schema.json |   2 +-
 5 files changed, 481 insertions(+), 141 deletions(-)
 create mode 100644 qemu-ga.texi

-- 
2.4.3

[Qemu-devel] [PATCH v8 06/11] netfilter: add an API to pass the packet to next filter

[Yang Hongyang]

add an API qemu_netfilter_pass_to_next() to pass the packet
to next filter.

Signed-off-by: Yang Hongyang 

---
v5: fold params to NetPacket struct
---
 include/net/filter.h |  3 +++
 net/filter.c | 33 +
 2 files changed, 36 insertions(+)

diff --git a/include/net/filter.h b/include/net/filter.h
index f15d83d..9278d40 100644
--- a/include/net/filter.h
+++ b/include/net/filter.h
@@ -57,4 +57,7 @@ void qemu_del_net_filter(NetFilterState *nf);
 void netfilter_add(QemuOpts *opts, Error **errp);
 void qmp_netfilter_add(QDict *qdict, QObject **ret, Error **errp);
 
+/* pass the packet to the next filter */
+ssize_t qemu_netfilter_pass_to_next(NetFilterState *nf, NetPacket *packet);
+
 #endif /* QEMU_NET_FILTER_H */
diff --git a/net/filter.c b/net/filter.c
index dcb1891..44c17b0 100644
--- a/net/filter.c
+++ b/net/filter.c
@@ -14,10 +14,12 @@
 #include "qapi/dealloc-visitor.h"
 #include "qemu/config-file.h"
 #include "qmp-commands.h"
+#include "qemu/iov.h"
 
 #include "net/filter.h"
 #include "net/net.h"
 #include "net/vhost_net.h"
+#include "net/queue.h"
 
 static QTAILQ_HEAD(, NetFilterState) net_filters;
 
@@ -153,6 +155,37 @@ void qmp_netfilter_del(const char *id, Error **errp)
 qemu_del_net_filter(nf);
 }
 
+ssize_t qemu_netfilter_pass_to_next(NetFilterState *nf, NetPacket *packet)
+{
+int ret = 0;
+NetFilterState *next = QTAILQ_NEXT(nf, next);
+struct iovec iov = {
+.iov_base = (void *)packet->data,
+.iov_len = packet->size
+};
+
+while (next) {
+if (next->chain == nf->chain || next->chain == NET_FILTER_ALL) {
+ret = next->info->receive_iov(next, packet->sender, packet->flags,
+  &iov, 1, packet->sent_cb);
+if (ret) {
+return ret;
+}
+}
+next = QTAILQ_NEXT(next, next);
+}
+
+/* we have gone through all filters, pass it to receiver */
+if (packet->sender && packet->sender->peer) {
+return qemu_net_queue_send_iov(packet->sender->peer->incoming_queue,
+   packet->sender, packet->flags,
+   &iov, 1, packet->sent_cb);
+}
+
+/* no receiver, or sender been deleted */
+return packet->size;
+}
+
 typedef int (NetFilterInit)(const NetFilterOptions *opts,
 const char *name, int chain,
 NetClientState *netdev, Error **errp);
-- 
1.9.1

[Qemu-devel] [PATCH v8 10/11] filter/buffer: update command description and help

[Yang Hongyang]

now that we have a buffer netfilter, update the command
description and help.

Signed-off-by: Yang Hongyang 
CC: Luiz Capitulino 
CC: Markus Armbruster 
---
v8: add more description for the filter to the TEXI section
---
 hmp-commands.hx |  2 +-
 qemu-options.hx | 18 +-
 qmp-commands.hx |  2 +-
 3 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 902e2d1..63177a8 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1255,7 +1255,7 @@ ETEXI
 {
 .name   = "netfilter_add",
 .args_type  = "netfilter:O",
-.params = 
"[type],id=str,netdev=str[,chain=in|out|all,prop=value][,...]",
+.params = 
"[buffer],id=str,netdev=str[,chain=in|out|all,prop=value][,...]",
 .help   = "add netfilter",
 .mhandler.cmd = hmp_netfilter_add,
 .command_completion = netfilter_add_completion,
diff --git a/qemu-options.hx b/qemu-options.hx
index 0d52d02..390e055 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1575,7 +1575,10 @@ DEF("net", HAS_ARG, QEMU_OPTION_net,
 "socket][,vlan=n][,option][,option][,...]\n"
 "old way to initialize a host network interface\n"
 "(use the -netdev option if possible instead)\n", 
QEMU_ARCH_ALL)
-DEF("netfilter", HAS_ARG, QEMU_OPTION_netfilter, "", QEMU_ARCH_ALL)
+DEF("netfilter", HAS_ARG, QEMU_OPTION_netfilter,
+"-netfilter buffer,id=str,netdev=str[,chain=in|out|all,interval=n]\n"
+"buffer netdev in/out packets. if interval provided, will 
release\n"
+"packets by interval. interval scale: microsecond\n", 
QEMU_ARCH_ALL)
 STEXI
 @item -net nic[,vlan=@var{n}][,macaddr=@var{mac}][,model=@var{type}] 
[,name=@var{name}][,addr=@var{addr}][,vectors=@var{v}]
 @findex -net
@@ -1990,6 +1993,19 @@ libpcap, so it can be analyzed with tools such as 
tcpdump or Wireshark.
 Indicate that no network devices should be configured. It is used to
 override the default configuration (@option{-net nic -net user}) which
 is activated if no @option{-net} options are provided.
+
+@item -netfilter 
buffer,id=@var{id},netdev=@var{netdevid}[,chain=@var{in/out/all}][,interval=@var{n}]
+Buffer netdev @var{netdevid} input or output packets. if interval @var{n}
+provided, will release packets by interval. interval scale: microsecond.
+
+chain @var{in/out/all} is an option that can be applied to any netfilters, if
+not provided, default is @option{all}.
+
+@option{in} means this filter will receive packets sent to the netdev
+
+@option{out} means this filter will receive packets sent from the netdev
+
+@option{all} means this filter will receive packets both sent to/from the 
netdev
 ETEXI
 
 STEXI
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 4f0dc98..9419a6f 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -947,7 +947,7 @@ Arguments:
 Example:
 
 -> { "execute": "netfilter_add",
-"arguments": { "type": "type", "id": "nf0",
+"arguments": { "type": "buffer", "id": "nf0",
"netdev": "bn",
"chain": "in" } }
 <- { "return": {} }
-- 
1.9.1

[Qemu-devel] [PATCH 1/2] memory: allow zero size for adjust_endianness()

[Jason Wang]

Wildcard mmio eventfd use zero size, but it will lead abort() since it
was illegal in adjust_endianness(). Fix this by allowing zero size.

Cc: Greg Kurz 
Cc: Paolo Bonzini 
Signed-off-by: Jason Wang 
---
 memory.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/memory.c b/memory.c
index 4eb138a..134aa57 100644
--- a/memory.c
+++ b/memory.c
@@ -353,6 +353,7 @@ static void adjust_endianness(MemoryRegion *mr, uint64_t 
*data, unsigned size)
 {
 if (memory_region_wrong_endianness(mr)) {
 switch (size) {
+case 0:
 case 1:
 break;
 case 2:
-- 
2.1.4

[Qemu-devel] [PATCH v3 01/12] qga: misc spelling

[marcandre . lureau]

From: Marc-André Lureau 

Signed-off-by: Marc-André Lureau 
Reviewed-by: Eric Blake 
Reviewed-by: Denis V. Lunev 
Reviewed-by: Michael Roth 
---
 qga/qapi-schema.json | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 18e3cc3..6b0bd16 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -793,7 +793,7 @@
 # scheme. Refer to the documentation of the guest operating system
 # in question to determine what is supported.
 #
-# Note all guest operating systems will support use of the
+# Not all guest operating systems will support use of the
 # @crypted flag, as they may require the clear-text password
 #
 # The @password parameter must always be base64 encoded before
-- 
2.4.3

[Qemu-devel] [PATCH 2/2] pci: test-dev: try to test fast mmio bus for wildcard mmio event

[Jason Wang]

Test fast mmio by using zero size eventfd.

Signed-off-by: Jason Wang 
---
 hw/misc/pci-testdev.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/misc/pci-testdev.c b/hw/misc/pci-testdev.c
index 26b9b86..7bf67ed 100644
--- a/hw/misc/pci-testdev.c
+++ b/hw/misc/pci-testdev.c
@@ -261,8 +261,12 @@ static void pci_testdev_realize(PCIDevice *pci_dev, Error 
**errp)
 memcpy(test->hdr->name, name, strlen(name) + 1);
 g_free(name);
 test->hdr->offset = cpu_to_le32(IOTEST_SIZE(i) + i * 
IOTEST_ACCESS_WIDTH);
-test->size = IOTEST_ACCESS_WIDTH;
 test->match_data = strcmp(IOTEST_TEST(i), "wildcard-eventfd");
+if (!test->match_data && !strcmp(IOTEST_TYPE(i), "mmio")) {
+test->size = 0;
+} else {
+test->size = IOTEST_ACCESS_WIDTH;
+}
 test->hdr->test = i;
 test->hdr->data = test->match_data ? IOTEST_DATAMATCH : IOTEST_NOMATCH;
 test->hdr->width = IOTEST_ACCESS_WIDTH;
-- 
2.1.4

[Qemu-devel] [PATCH v3 05/12] qga: copy argument strings

[marcandre . lureau]

From: Marc-André Lureau 

A following patch will return allocated string.

Signed-off-by: Marc-André Lureau 
Reviewed-by: Michael Roth 
---
 qga/main.c | 57 +++--
 1 file changed, 31 insertions(+), 26 deletions(-)

diff --git a/qga/main.c b/qga/main.c
index ede5306..83b7804 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -944,13 +944,13 @@ static GList *split_list(gchar *str, const gchar 
separator)
 int main(int argc, char **argv)
 {
 const char *sopt = "hVvdm:p:l:f:F::b:s:t:";
-const char *method = NULL, *channel_path = NULL;
-const char *log_filepath = NULL;
-const char *pid_filepath;
+char *method = NULL, *channel_path = NULL;
+char *log_filepath = NULL;
+char *pid_filepath = NULL;
 #ifdef CONFIG_FSFREEZE
-const char *fsfreeze_hook = NULL;
+char *fsfreeze_hook = NULL;
 #endif
-const char *state_dir;
+char *state_dir = NULL;
 #ifdef _WIN32
 const char *service = NULL;
 #endif
@@ -981,31 +981,28 @@ int main(int argc, char **argv)
 module_call_init(MODULE_INIT_QAPI);
 
 init_dfl_pathnames();
-pid_filepath = dfl_pathnames.pidfile;
-state_dir = dfl_pathnames.state_dir;
-
 while ((ch = getopt_long(argc, argv, sopt, lopt, &opt_ind)) != -1) {
 switch (ch) {
 case 'm':
-method = optarg;
+method = g_strdup(optarg);
 break;
 case 'p':
-channel_path = optarg;
+channel_path = g_strdup(optarg);
 break;
 case 'l':
-log_filepath = optarg;
+log_filepath = g_strdup(optarg);
 break;
 case 'f':
-pid_filepath = optarg;
+pid_filepath = g_strdup(optarg);
 break;
 #ifdef CONFIG_FSFREEZE
 case 'F':
-fsfreeze_hook = optarg ? optarg : QGA_FSFREEZE_HOOK_DEFAULT;
+fsfreeze_hook = g_strdup(optarg ?: QGA_FSFREEZE_HOOK_DEFAULT);
 break;
 #endif
 case 't':
- state_dir = optarg;
- break;
+state_dir = g_strdup(optarg);
+break;
 case 'v':
 /* enable all log levels */
 log_level = G_LOG_LEVEL_MASK;
@@ -1028,20 +1025,10 @@ int main(int argc, char **argv)
 case 's':
 service = optarg;
 if (strcmp(service, "install") == 0) {
-const char *fixed_state_dir;
-
-/* If the user passed the "-t" option, we save that state dir
- * in the service. Otherwise we let the service fetch the state
- * dir from the environment when it starts.
- */
-fixed_state_dir = (state_dir == dfl_pathnames.state_dir) ?
-  NULL :
-  state_dir;
 if (ga_install_vss_provider()) {
 exit(EXIT_FAILURE);
 }
-if (ga_install_service(channel_path, log_filepath,
-   fixed_state_dir)) {
+if (ga_install_service(channel_path, log_filepath, state_dir)) 
{
 exit(EXIT_FAILURE);
 }
 exit(EXIT_SUCCESS);
@@ -1072,6 +1059,14 @@ int main(int argc, char **argv)
 }
 }
 
+if (pid_filepath == NULL) {
+pid_filepath = g_strdup(dfl_pathnames.pidfile);
+}
+
+if (state_dir == NULL) {
+state_dir = g_strdup(dfl_pathnames.state_dir);
+}
+
 #ifdef _WIN32
 /* On win32 the state directory is application specific (be it the default
  * or a user override). We got past the command line parsing; let's create
@@ -1214,5 +1209,15 @@ out_bad:
 if (daemonize) {
 unlink(pid_filepath);
 }
+
+g_free(method);
+g_free(log_filepath);
+g_free(pid_filepath);
+g_free(state_dir);
+g_free(channel_path);
+#ifdef CONFIG_FSFREEZE
+g_free(fsfreeze_hook);
+#endif
+
 return EXIT_FAILURE;
 }
-- 
2.4.3

[Qemu-devel] [PATCH v3 04/12] qga: rename 'path' to 'channel_path'

[marcandre . lureau]

From: Marc-André Lureau 

'path' is already a global function, rename the variable since it's
going to be in global scope in a later patch.

Signed-off-by: Marc-André Lureau 
Reviewed-by: Michael Roth 
---
 qga/main.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/qga/main.c b/qga/main.c
index e75022c..ede5306 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -944,7 +944,7 @@ static GList *split_list(gchar *str, const gchar separator)
 int main(int argc, char **argv)
 {
 const char *sopt = "hVvdm:p:l:f:F::b:s:t:";
-const char *method = NULL, *path = NULL;
+const char *method = NULL, *channel_path = NULL;
 const char *log_filepath = NULL;
 const char *pid_filepath;
 #ifdef CONFIG_FSFREEZE
@@ -990,7 +990,7 @@ int main(int argc, char **argv)
 method = optarg;
 break;
 case 'p':
-path = optarg;
+channel_path = optarg;
 break;
 case 'l':
 log_filepath = optarg;
@@ -1040,7 +1040,8 @@ int main(int argc, char **argv)
 if (ga_install_vss_provider()) {
 exit(EXIT_FAILURE);
 }
-if (ga_install_service(path, log_filepath, fixed_state_dir)) {
+if (ga_install_service(channel_path, log_filepath,
+   fixed_state_dir)) {
 exit(EXIT_FAILURE);
 }
 exit(EXIT_SUCCESS);
@@ -1185,7 +1186,7 @@ int main(int argc, char **argv)
 #endif
 
 s->main_loop = g_main_loop_new(NULL, false);
-if (!channel_init(ga_state, method, path)) {
+if (!channel_init(ga_state, method, channel_path)) {
 g_critical("failed to initialize guest agent channel");
 goto out_bad;
 }
-- 
2.4.3

[Qemu-devel] [PATCH v3 02/12] qga: use exit() when parsing options

[marcandre . lureau]

From: Marc-André Lureau 

The option parsing is going to be moved to a separate function,
use exit() consistently.

Signed-off-by: Marc-André Lureau 
Reviewed-by: Denis V. Lunev 
Reviewed-by: Eric Blake 
Reviewed-by: Michael Roth 
---
 qga/main.c | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/qga/main.c b/qga/main.c
index 791982e..10bb2f7 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -992,14 +992,14 @@ int main(int argc, char **argv)
 break;
 case 'V':
 printf("QEMU Guest Agent %s\n", QEMU_VERSION);
-return 0;
+exit(EXIT_SUCCESS);
 case 'd':
 daemonize = 1;
 break;
 case 'b': {
 if (is_help_option(optarg)) {
 qmp_for_each_command(ga_print_cmd, NULL);
-return 0;
+exit(EXIT_SUCCESS);
 }
 for (j = 0, i = 0, len = strlen(optarg); i < len; i++) {
 if (optarg[i] == ',') {
@@ -1027,36 +1027,36 @@ int main(int argc, char **argv)
   NULL :
   state_dir;
 if (ga_install_vss_provider()) {
-return EXIT_FAILURE;
+exit(EXIT_FAILURE);
 }
 if (ga_install_service(path, log_filepath, fixed_state_dir)) {
-return EXIT_FAILURE;
+exit(EXIT_FAILURE);
 }
-return 0;
+exit(EXIT_SUCCESS);
 } else if (strcmp(service, "uninstall") == 0) {
 ga_uninstall_vss_provider();
-return ga_uninstall_service();
+exit(ga_uninstall_service());
 } else if (strcmp(service, "vss-install") == 0) {
 if (ga_install_vss_provider()) {
-return EXIT_FAILURE;
+exit(EXIT_FAILURE);
 }
-return EXIT_SUCCESS;
+exit(EXIT_SUCCESS);
 } else if (strcmp(service, "vss-uninstall") == 0) {
 ga_uninstall_vss_provider();
-return EXIT_SUCCESS;
+exit(EXIT_SUCCESS);
 } else {
 printf("Unknown service command.\n");
-return EXIT_FAILURE;
+exit(EXIT_FAILURE);
 }
 break;
 #endif
 case 'h':
 usage(argv[0]);
-return 0;
+exit(EXIT_SUCCESS);
 case '?':
 g_print("Unknown option, try '%s --help' for more information.\n",
 argv[0]);
-return EXIT_FAILURE;
+exit(EXIT_FAILURE);
 }
 }
 
-- 
2.4.3

[Qemu-devel] [PATCH v3 03/12] qga: move string split in separate function

[marcandre . lureau]

From: Marc-André Lureau 

The function is going to be reused in a later patch.

Signed-off-by: Marc-André Lureau 
Reviewed-by: Michael Roth 
---
 qga/main.c | 33 ++---
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/qga/main.c b/qga/main.c
index 10bb2f7..e75022c 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -921,6 +921,26 @@ static void ga_print_cmd(QmpCommand *cmd, void *opaque)
 printf("%s\n", qmp_command_name(cmd));
 }
 
+static GList *split_list(gchar *str, const gchar separator)
+{
+GList *list = NULL;
+int i, j, len;
+
+for (j = 0, i = 0, len = strlen(str); i < len; i++) {
+if (str[i] == separator) {
+str[i] = 0;
+list = g_list_append(list, &str[j]);
+j = i + 1;
+}
+}
+
+if (j < i) {
+list = g_list_append(list, &str[j]);
+}
+
+return list;
+}
+
 int main(int argc, char **argv)
 {
 const char *sopt = "hVvdm:p:l:f:F::b:s:t:";
@@ -953,7 +973,7 @@ int main(int argc, char **argv)
 { "statedir", 1, NULL, 't' },
 { NULL, 0, NULL, 0 }
 };
-int opt_ind = 0, ch, daemonize = 0, i, j, len;
+int opt_ind = 0, ch, daemonize = 0;
 GLogLevelFlags log_level = G_LOG_LEVEL_ERROR | G_LOG_LEVEL_CRITICAL;
 GList *blacklist = NULL;
 GAState *s;
@@ -1001,16 +1021,7 @@ int main(int argc, char **argv)
 qmp_for_each_command(ga_print_cmd, NULL);
 exit(EXIT_SUCCESS);
 }
-for (j = 0, i = 0, len = strlen(optarg); i < len; i++) {
-if (optarg[i] == ',') {
-optarg[i] = 0;
-blacklist = g_list_append(blacklist, &optarg[j]);
-j = i + 1;
-}
-}
-if (j < i) {
-blacklist = g_list_append(blacklist, &optarg[j]);
-}
+blacklist = g_list_concat(blacklist, split_list(optarg, ','));
 break;
 }
 #ifdef _WIN32
-- 
2.4.3

[Qemu-devel] [PATCH v3 08/12] qga: move agent run in a separate function

[marcandre . lureau]

From: Marc-André Lureau 

Once the options are populated, move the running state to
a run_agent() function.

Signed-off-by: Marc-André Lureau 
Reviewed-by: Michael Roth 
---
 qga/main.c | 164 +
 1 file changed, 89 insertions(+), 75 deletions(-)

diff --git a/qga/main.c b/qga/main.c
index 80f51fe..118847c 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -1064,70 +1064,8 @@ static void config_free(GAConfig *config)
 g_free(config);
 }
 
-int main(int argc, char **argv)
+static bool check_is_frozen(GAState *s)
 {
-GAState *s;
-GAConfig *config;
-
-module_call_init(MODULE_INIT_QAPI);
-
-init_dfl_pathnames();
-
-config = config_parse(argc, argv);
-
-if (config->pid_filepath == NULL) {
-config->pid_filepath = g_strdup(dfl_pathnames.pidfile);
-}
-
-if (config->state_dir == NULL) {
-config->state_dir = g_strdup(dfl_pathnames.state_dir);
-}
-
-if (config->method == NULL) {
-config->method = g_strdup("virtio-serial");
-}
-
-if (config->channel_path == NULL) {
-if (strcmp(config->method, "virtio-serial") == 0) {
-/* try the default path for the virtio-serial port */
-config->channel_path = g_strdup(QGA_VIRTIO_PATH_DEFAULT);
-} else if (strcmp(config->method, "isa-serial") == 0) {
-/* try the default path for the serial port - COM1 */
-config->channel_path = g_strdup(QGA_SERIAL_PATH_DEFAULT);
-} else {
-g_critical("must specify a path for this channel");
-goto out_bad;
-}
-}
-
-#ifdef _WIN32
-/* On win32 the state directory is application specific (be it the default
- * or a user override). We got past the command line parsing; let's create
- * the directory (with any intermediate directories). If we run into an
- * error later on, we won't try to clean up the directory, it is considered
- * persistent.
- */
-if (g_mkdir_with_parents(config->state_dir, S_IRWXU) == -1) {
-g_critical("unable to create (an ancestor of) the state directory"
-   " '%s': %s", config->state_dir, strerror(errno));
-return EXIT_FAILURE;
-}
-#endif
-
-s = g_malloc0(sizeof(GAState));
-s->log_level = config->log_level;
-s->log_file = stderr;
-#ifdef CONFIG_FSFREEZE
-s->fsfreeze_hook = config->fsfreeze_hook;
-#endif
-g_log_set_default_handler(ga_log, s);
-g_log_set_fatal_mask(NULL, G_LOG_LEVEL_ERROR);
-ga_enable_logging(s);
-s->state_filepath_isfrozen = g_strdup_printf("%s/qga.state.isfrozen",
- config->state_dir);
-s->pstate_filepath = g_strdup_printf("%s/qga.state", config->state_dir);
-s->frozen = false;
-
 #ifndef _WIN32
 /* check if a previous instance of qemu-ga exited with filesystems' state
  * marked as frozen. this could be a stale value (a non-qemu-ga process
@@ -1153,7 +1091,31 @@ int main(int argc, char **argv)
   " guest-fsfreeze-thaw is issued, or filesystems are"
   " manually unfrozen and the file %s is removed",
   s->state_filepath_isfrozen);
-s->frozen = true;
+return true;
+}
+#endif
+return false;
+}
+
+static int run_agent(GAState *s, GAConfig *config)
+{
+ga_state = s;
+
+g_log_set_default_handler(ga_log, s);
+g_log_set_fatal_mask(NULL, G_LOG_LEVEL_ERROR);
+ga_enable_logging(s);
+
+#ifdef _WIN32
+/* On win32 the state directory is application specific (be it the default
+ * or a user override). We got past the command line parsing; let's create
+ * the directory (with any intermediate directories). If we run into an
+ * error later on, we won't try to clean up the directory, it is considered
+ * persistent.
+ */
+if (g_mkdir_with_parents(config->state_dir, S_IRWXU) == -1) {
+g_critical("unable to create (an ancestor of) the state directory"
+   " '%s': %s", config->state_dir, strerror(errno));
+return EXIT_FAILURE;
 }
 #endif
 
@@ -1178,7 +1140,7 @@ int main(int argc, char **argv)
 if (!log_file) {
 g_critical("unable to open specified log file: %s",
strerror(errno));
-goto out_bad;
+return EXIT_FAILURE;
 }
 s->log_file = log_file;
 }
@@ -1189,7 +1151,7 @@ int main(int argc, char **argv)
s->pstate_filepath,
ga_is_frozen(s))) {
 g_critical("failed to load persistent state");
-goto out_bad;
+return EXIT_FAILURE;
 }
 
 config->blacklist = ga_command_blacklist_init(config->blacklist);
@@ -1210,14 +1172,14 @@ int main(int argc, char **argv)
 #ifndef _WIN32
 if (!register_signal_handlers()) {
 g_critical("failed to register signal handlers");
-goto out_bad;

[Qemu-devel] [PATCH v3 09/12] qga: free a bit more

[marcandre . lureau]

From: Marc-André Lureau 

Now that main() has a single exit point, we can free a few
more allocations.

Signed-off-by: Marc-André Lureau 
---
 qga/main.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/qga/main.c b/qga/main.c
index 118847c..58f2fc7 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -82,7 +82,7 @@ struct GAState {
 bool delimit_response;
 bool frozen;
 GList *blacklist;
-const char *state_filepath_isfrozen;
+char *state_filepath_isfrozen;
 struct {
 const char *log_filepath;
 const char *pid_filepath;
@@ -90,7 +90,7 @@ struct GAState {
 #ifdef CONFIG_FSFREEZE
 const char *fsfreeze_hook;
 #endif
-const gchar *pstate_filepath;
+gchar *pstate_filepath;
 GAPersistentState pstate;
 };
 
@@ -1253,6 +1253,8 @@ end:
 if (s->channel) {
 ga_channel_free(s->channel);
 }
+g_free(s->pstate_filepath);
+g_free(s->state_filepath_isfrozen);
 
 if (config->daemonize) {
 unlink(config->pid_filepath);
-- 
2.4.3

[Qemu-devel] [PATCH v3 06/12] qga: move option parsing to separate function

[marcandre . lureau]

From: Marc-André Lureau 

Move option parsing out of giant main().

Signed-off-by: Marc-André Lureau 
Reviewed-by: Michael Roth 
---
 qga/main.c | 165 +++--
 1 file changed, 96 insertions(+), 69 deletions(-)

diff --git a/qga/main.c b/qga/main.c
index 83b7804..8c4a075 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -941,19 +941,28 @@ static GList *split_list(gchar *str, const gchar 
separator)
 return list;
 }
 
-int main(int argc, char **argv)
-{
-const char *sopt = "hVvdm:p:l:f:F::b:s:t:";
-char *method = NULL, *channel_path = NULL;
-char *log_filepath = NULL;
-char *pid_filepath = NULL;
+typedef struct GAConfig {
+char *channel_path;
+char *method;
+char *log_filepath;
+char *pid_filepath;
 #ifdef CONFIG_FSFREEZE
-char *fsfreeze_hook = NULL;
+char *fsfreeze_hook;
 #endif
-char *state_dir = NULL;
+char *state_dir;
 #ifdef _WIN32
-const char *service = NULL;
+const char *service;
 #endif
+GList *blacklist;
+int daemonize;
+GLogLevelFlags log_level;
+} GAConfig;
+
+static GAConfig *config_parse(int argc, char **argv)
+{
+GAConfig *config = g_new0(GAConfig, 1);
+const char *sopt = "hVvdm:p:l:f:F::b:s:t:D";
+int opt_ind = 0, ch;
 const struct option lopt[] = {
 { "help", 0, NULL, 'h' },
 { "version", 0, NULL, 'V' },
@@ -973,74 +982,71 @@ int main(int argc, char **argv)
 { "statedir", 1, NULL, 't' },
 { NULL, 0, NULL, 0 }
 };
-int opt_ind = 0, ch, daemonize = 0;
-GLogLevelFlags log_level = G_LOG_LEVEL_ERROR | G_LOG_LEVEL_CRITICAL;
-GList *blacklist = NULL;
-GAState *s;
 
-module_call_init(MODULE_INIT_QAPI);
+config->log_level = G_LOG_LEVEL_ERROR | G_LOG_LEVEL_CRITICAL;
 
-init_dfl_pathnames();
 while ((ch = getopt_long(argc, argv, sopt, lopt, &opt_ind)) != -1) {
 switch (ch) {
 case 'm':
-method = g_strdup(optarg);
+config->method = g_strdup(optarg);
 break;
 case 'p':
-channel_path = g_strdup(optarg);
+config->channel_path = g_strdup(optarg);
 break;
 case 'l':
-log_filepath = g_strdup(optarg);
+config->log_filepath = g_strdup(optarg);
 break;
 case 'f':
-pid_filepath = g_strdup(optarg);
+config->pid_filepath = g_strdup(optarg);
 break;
 #ifdef CONFIG_FSFREEZE
 case 'F':
-fsfreeze_hook = g_strdup(optarg ?: QGA_FSFREEZE_HOOK_DEFAULT);
+config->fsfreeze_hook = g_strdup(optarg ?: 
QGA_FSFREEZE_HOOK_DEFAULT);
 break;
 #endif
 case 't':
-state_dir = g_strdup(optarg);
+config->state_dir = g_strdup(optarg);
 break;
 case 'v':
 /* enable all log levels */
-log_level = G_LOG_LEVEL_MASK;
+config->log_level = G_LOG_LEVEL_MASK;
 break;
 case 'V':
 printf("QEMU Guest Agent %s\n", QEMU_VERSION);
 exit(EXIT_SUCCESS);
 case 'd':
-daemonize = 1;
+config->daemonize = 1;
 break;
 case 'b': {
 if (is_help_option(optarg)) {
 qmp_for_each_command(ga_print_cmd, NULL);
 exit(EXIT_SUCCESS);
 }
-blacklist = g_list_concat(blacklist, split_list(optarg, ','));
+config->blacklist = g_list_concat(config->blacklist,
+ split_list(optarg, ','));
 break;
 }
 #ifdef _WIN32
 case 's':
-service = optarg;
-if (strcmp(service, "install") == 0) {
+config->service = optarg;
+if (strcmp(config->service, "install") == 0) {
 if (ga_install_vss_provider()) {
 exit(EXIT_FAILURE);
 }
-if (ga_install_service(channel_path, log_filepath, state_dir)) 
{
+if (ga_install_service(config->channel_path,
+   config->log_filepath, 
config->state_dir)) {
 exit(EXIT_FAILURE);
 }
 exit(EXIT_SUCCESS);
-} else if (strcmp(service, "uninstall") == 0) {
+} else if (strcmp(config->service, "uninstall") == 0) {
 ga_uninstall_vss_provider();
 exit(ga_uninstall_service());
-} else if (strcmp(service, "vss-install") == 0) {
+} else if (strcmp(config->service, "vss-install") == 0) {
 if (ga_install_vss_provider()) {
 exit(EXIT_FAILURE);
 }
 exit(EXIT_SUCCESS);
-} else if (strcmp(service, "vss-uninstall") == 0) {
+} else if (strcmp(config->service, "vss-uninstall") == 0) {
 ga_uninstall_vss_provider();
 exit

[Qemu-devel] [PATCH v3 10/12] qga: add an optionnal qemu-ga.conf system configuration

[marcandre . lureau]

From: Marc-André Lureau 

Learn to configure the agent with a system configuration.

This may simplify command-line handling, especially when the blacklist
is long.

Among the other benefits, this may standardize the configuration of a
init service (instead of distro-specific init keys/files)

Signed-off-by: Marc-André Lureau 
Reviewed-by: Michael Roth 
---
 qga/main.c | 80 --
 1 file changed, 73 insertions(+), 7 deletions(-)

diff --git a/qga/main.c b/qga/main.c
index 58f2fc7..9193043 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -56,6 +56,7 @@
 #define QGA_FSFREEZE_HOOK_DEFAULT CONFIG_QEMU_CONFDIR "/fsfreeze-hook"
 #endif
 #define QGA_SENTINEL_BYTE 0xFF
+#define QGA_CONF_DEFAULT CONFIG_QEMU_CONFDIR G_DIR_SEPARATOR_S "qemu-ga.conf"
 
 static struct {
 const char *state_dir;
@@ -936,14 +937,80 @@ typedef struct GAConfig {
 #ifdef _WIN32
 const char *service;
 #endif
+gchar *bliststr; /* blacklist may point to this string */
 GList *blacklist;
 int daemonize;
 GLogLevelFlags log_level;
 } GAConfig;
 
-static GAConfig *config_parse(int argc, char **argv)
+static void config_load(GAConfig *config)
+{
+GError *gerr = NULL;
+GKeyFile *keyfile;
+
+/* read system config */
+keyfile = g_key_file_new();
+if (!g_key_file_load_from_file(keyfile, QGA_CONF_DEFAULT, 0, &gerr)) {
+goto end;
+}
+if (g_key_file_has_key(keyfile, "general", "daemon", NULL)) {
+config->daemonize =
+g_key_file_get_boolean(keyfile, "general", "daemon", &gerr);
+}
+if (g_key_file_has_key(keyfile, "general", "method", NULL)) {
+config->method =
+g_key_file_get_string(keyfile, "general", "method", &gerr);
+}
+if (g_key_file_has_key(keyfile, "general", "path", NULL)) {
+config->channel_path =
+g_key_file_get_string(keyfile, "general", "path", &gerr);
+}
+if (g_key_file_has_key(keyfile, "general", "logfile", NULL)) {
+config->log_filepath =
+g_key_file_get_string(keyfile, "general", "logfile", &gerr);
+}
+if (g_key_file_has_key(keyfile, "general", "pidfile", NULL)) {
+config->pid_filepath =
+g_key_file_get_string(keyfile, "general", "pidfile", &gerr);
+}
+#ifdef CONFIG_FSFREEZE
+if (g_key_file_has_key(keyfile, "general", "fsfreeze-hook", NULL)) {
+config->fsfreeze_hook =
+g_key_file_get_string(keyfile,
+  "general", "fsfreeze-hook", &gerr);
+}
+#endif
+if (g_key_file_has_key(keyfile, "general", "statedir", NULL)) {
+config->state_dir =
+g_key_file_get_string(keyfile, "general", "statedir", &gerr);
+}
+if (g_key_file_has_key(keyfile, "general", "verbose", NULL) &&
+g_key_file_get_boolean(keyfile, "general", "verbose", &gerr)) {
+/* enable all log levels */
+config->log_level = G_LOG_LEVEL_MASK;
+}
+if (g_key_file_has_key(keyfile, "general", "blacklist", NULL)) {
+config->bliststr =
+g_key_file_get_string(keyfile, "general", "blacklist", &gerr);
+config->blacklist = g_list_concat(config->blacklist,
+  split_list(config->bliststr, ','));
+}
+
+end:
+if (keyfile) {
+g_key_file_free(keyfile);
+}
+if (gerr &&
+!(gerr->domain == G_FILE_ERROR && gerr->code == G_FILE_ERROR_NOENT)) {
+g_critical("error loading configuration from path: %s, %s",
+   QGA_CONF_DEFAULT, gerr->message);
+exit(EXIT_FAILURE);
+}
+g_clear_error(&gerr);
+}
+
+static void config_parse(GAConfig *config, int argc, char **argv)
 {
-GAConfig *config = g_new0(GAConfig, 1);
 const char *sopt = "hVvdm:p:l:f:F::b:s:t:D";
 int opt_ind = 0, ch;
 const struct option lopt[] = {
@@ -1047,8 +1114,6 @@ static GAConfig *config_parse(int argc, char **argv)
 exit(EXIT_FAILURE);
 }
 }
-
-return config;
 }
 
 static void config_free(GAConfig *config)
@@ -1058,6 +1123,7 @@ static void config_free(GAConfig *config)
 g_free(config->pid_filepath);
 g_free(config->state_dir);
 g_free(config->channel_path);
+g_free(config->bliststr);
 #ifdef CONFIG_FSFREEZE
 g_free(config->fsfreeze_hook);
 #endif
@@ -1200,13 +1266,13 @@ int main(int argc, char **argv)
 {
 int ret = EXIT_SUCCESS;
 GAState *s = g_new0(GAState, 1);
-GAConfig *config;
+GAConfig *config = g_new0(GAConfig, 1);
 
 module_call_init(MODULE_INIT_QAPI);
 
 init_dfl_pathnames();
-
-config = config_parse(argc, argv);
+config_load(config);
+config_parse(config, argc, argv);
 
 if (config->pid_filepath == NULL) {
 config->pid_filepath = g_strdup(dfl_pathnames.pidfile);
-- 
2.4.3

[Qemu-devel] [PATCH v3 07/12] qga: fill default options in main()

[marcandre . lureau]

From: Marc-André Lureau 

Fill all default options during main(). This is a preparation patch
to allow to dump the configuration.

Signed-off-by: Marc-André Lureau 
Reviewed-by: Michael Roth 
---
 qga/main.c | 34 +-
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/qga/main.c b/qga/main.c
index 8c4a075..80f51fe 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -658,23 +658,6 @@ static gboolean channel_init(GAState *s, const gchar 
*method, const gchar *path)
 {
 GAChannelMethod channel_method;
 
-if (method == NULL) {
-method = "virtio-serial";
-}
-
-if (path == NULL) {
-if (strcmp(method, "virtio-serial") == 0 ) {
-/* try the default path for the virtio-serial port */
-path = QGA_VIRTIO_PATH_DEFAULT;
-} else if (strcmp(method, "isa-serial") == 0){
-/* try the default path for the serial port - COM1 */
-path = QGA_SERIAL_PATH_DEFAULT;
-} else {
-g_critical("must specify a path for this channel");
-return false;
-}
-}
-
 if (strcmp(method, "virtio-serial") == 0) {
 s->virtio = true; /* virtio requires special handling in some cases */
 channel_method = GA_CHANNEL_VIRTIO_SERIAL;
@@ -1100,6 +1083,23 @@ int main(int argc, char **argv)
 config->state_dir = g_strdup(dfl_pathnames.state_dir);
 }
 
+if (config->method == NULL) {
+config->method = g_strdup("virtio-serial");
+}
+
+if (config->channel_path == NULL) {
+if (strcmp(config->method, "virtio-serial") == 0) {
+/* try the default path for the virtio-serial port */
+config->channel_path = g_strdup(QGA_VIRTIO_PATH_DEFAULT);
+} else if (strcmp(config->method, "isa-serial") == 0) {
+/* try the default path for the serial port - COM1 */
+config->channel_path = g_strdup(QGA_SERIAL_PATH_DEFAULT);
+} else {
+g_critical("must specify a path for this channel");
+goto out_bad;
+}
+}
+
 #ifdef _WIN32
 /* On win32 the state directory is application specific (be it the default
  * or a user override). We got past the command line parsing; let's create
-- 
2.4.3

[Qemu-devel] [PATCH v3 12/12] qga: start a man page

[marcandre . lureau]

From: Marc-André Lureau 

Add a simple man page for the qemu agent.

Signed-off-by: Marc-André Lureau 
Reviewed-by: Michael Roth 
---
 Makefile  |  14 +-
 qemu-doc.texi |   6 +++
 qemu-ga.texi  | 136 ++
 3 files changed, 154 insertions(+), 2 deletions(-)
 create mode 100644 qemu-ga.texi

diff --git a/Makefile b/Makefile
index 340d9c8..67d44b8 100644
--- a/Makefile
+++ b/Makefile
@@ -88,7 +88,8 @@ LIBS+=-lz $(LIBS_TOOLS)
 HELPERS-$(CONFIG_LINUX) = qemu-bridge-helper$(EXESUF)
 
 ifdef BUILD_DOCS
-DOCS=qemu-doc.html qemu-tech.html qemu.1 qemu-img.1 qemu-nbd.8 qmp-commands.txt
+DOCS=qemu-doc.html qemu-tech.html qemu.1 qemu-img.1 qemu-nbd.8 qemu-ga.8
+DOCS+=qmp-commands.txt
 ifdef CONFIG_LINUX
 DOCS+=kvm_stat.1
 endif
@@ -400,6 +401,9 @@ ifneq ($(TOOLS),)
$(INSTALL_DIR) "$(DESTDIR)$(mandir)/man8"
$(INSTALL_DATA) qemu-nbd.8 "$(DESTDIR)$(mandir)/man8"
 endif
+ifneq (,$(findstring qemu-ga,$(TOOLS)))
+   $(INSTALL_DATA) qemu-ga.8 "$(DESTDIR)$(mandir)/man8"
+endif
 endif
 ifdef CONFIG_VIRTFS
$(INSTALL_DIR) "$(DESTDIR)$(mandir)/man1"
@@ -538,6 +542,12 @@ qemu-nbd.8: qemu-nbd.texi
  $(POD2MAN) --section=8 --center=" " --release=" " qemu-nbd.pod > $@, \
  "  GEN   $@")
 
+qemu-ga.8: qemu-ga.texi
+   $(call quiet-command, \
+ perl -Ww -- $(SRC_PATH)/scripts/texi2pod.pl $< qemu-ga.pod && \
+ $(POD2MAN) --section=8 --center=" " --release=" " qemu-ga.pod > $@, \
+ "  GEN   $@")
+
 kvm_stat.1: scripts/kvm/kvm_stat.texi
$(call quiet-command, \
  perl -Ww -- $(SRC_PATH)/scripts/texi2pod.pl $< kvm_stat.pod && \
@@ -551,7 +561,7 @@ pdf: qemu-doc.pdf qemu-tech.pdf
 
 qemu-doc.dvi qemu-doc.html qemu-doc.info qemu-doc.pdf: \
qemu-img.texi qemu-nbd.texi qemu-options.texi \
-   qemu-monitor.texi qemu-img-cmds.texi
+   qemu-monitor.texi qemu-img-cmds.texi qemu-ga.texi
 
 ifdef CONFIG_WIN32
 
diff --git a/qemu-doc.texi b/qemu-doc.texi
index 94af8c0..84d17d1 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -412,6 +412,7 @@ snapshots.
 * vm_snapshots::  VM snapshots
 * qemu_img_invocation::   qemu-img Invocation
 * qemu_nbd_invocation::   qemu-nbd Invocation
+* qemu_ga_invocation::qemu-ga Invocation
 * disk_images_formats::   Disk image file formats
 * host_drives::   Using host drives
 * disk_images_fat_images::Virtual FAT disk images
@@ -505,6 +506,11 @@ state is not saved or restored properly (in particular 
USB).
 
 @include qemu-nbd.texi
 
+@node qemu_ga_invocation
+@subsection @code{qemu-ga} Invocation
+
+@include qemu-ga.texi
+
 @node disk_images_formats
 @subsection Disk image file formats
 
diff --git a/qemu-ga.texi b/qemu-ga.texi
new file mode 100644
index 000..7d4a628
--- /dev/null
+++ b/qemu-ga.texi
@@ -0,0 +1,136 @@
+@example
+@c man begin SYNOPSIS
+usage: qemu-ga [-m  -p ] [OPTION]...
+@c man end
+@end example
+
+@c man begin DESCRIPTION
+
+The QEMU Guest Agent is a deamon that allows the host to perform
+various operations in the guest, such as:
+
+@itemize
+@item
+get information from the guest
+@item
+set the guest's system time
+@item
+read/write a file
+@item
+sync and freeze the filesystems
+@item
+suspend the guest
+@item
+reconfigure guest local processors
+@item
+set user's password
+@item
+...
+@end itemize
+
+qemu-ga will read a system configuration file on startup (located at
+q@file{/etc/qemu/qemu-ga.conf} by default), then parse remaining
+configuration options on the command line. For the same key, the last
+option wins, but the lists accumulate (see below for configuration
+file format).
+
+@c man end
+
+@c man begin OPTIONS
+@table @option
+@item -m, --method=@var{method}
+  Transport method: one of @samp{unix-listen}, @samp{virtio-serial}, or
+  @samp{isa-serial} (@samp{virtio-serial} is the default).
+
+@item -p, --path=@var{path}
+  Device/socket path (the default for virtio-serial is:
+  @samp{/dev/virtio-ports/org.qemu.guest_agent.0},
+  the default for isa-serial is: @samp{/dev/ttyS0})
+
+@item -l, --logfile=@var{path}
+  Set log file path, logs to stderr by default.
+
+@item -f, --pidfile=@var{path}
+  Specify pid file (default is @samp{/var/run/qemu-ga.pid}).
+
+@item -F, --fsfreeze-hook=@var{path}
+  Enable fsfreeze hook. Accepts an optional argument that specifies
+  script to run on freeze/thaw. Script will be called with
+  'freeze'/'thaw' arguments accordingly.  (default is
+  @samp{/etc/qemu/fsfreeze-hook}) If using -F with an argument, do
+  not follow -F with a space. (for example:
+  @samp{-F/var/run/fsfreezehook.sh})
+
+@item -t, --statedir=@var{path}
+  Specify the directory to store state information (absolute paths only,
+  default is @samp{/var/run}).
+
+@item -v, --verbose
+  Log extra debugging information.
+
+@item -V, --version
+  Print version information and exit.
+
+@item -d, --daemon
+  Daemonize after startup (detach from terminal).
+
+@item -b, --blacklist=@

[Qemu-devel] [PATCH v3 11/12] qga: add --dump-conf option

[marcandre . lureau]

From: Marc-André Lureau 

This new option allows to review the agent configuration,
and ease the task of writing a configuration file.

Signed-off-by: Marc-André Lureau 
Reviewed-by: Michael Roth 
---
 qga/main.c | 62 ++
 1 file changed, 62 insertions(+)

diff --git a/qga/main.c b/qga/main.c
index 9193043..72dc366 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -941,6 +941,7 @@ typedef struct GAConfig {
 GList *blacklist;
 int daemonize;
 GLogLevelFlags log_level;
+int dumpconf;
 } GAConfig;
 
 static void config_load(GAConfig *config)
@@ -1009,6 +1010,58 @@ end:
 g_clear_error(&gerr);
 }
 
+static gchar *list_join(GList *list, const gchar separator)
+{
+GString *str = g_string_new("");
+
+while (list) {
+str = g_string_append(str, (gchar *)list->data);
+list = g_list_next(list);
+if (list) {
+str = g_string_append_c(str, separator);
+}
+}
+
+return g_string_free(str, FALSE);
+}
+
+static void config_dump(GAConfig *config)
+{
+GError *error = NULL;
+GKeyFile *keyfile;
+gchar *tmp;
+
+keyfile = g_key_file_new();
+g_key_file_set_boolean(keyfile, "general", "daemon", config->daemonize);
+g_key_file_set_string(keyfile, "general", "method", config->method);
+g_key_file_set_string(keyfile, "general", "path", config->channel_path);
+if (config->log_filepath) {
+g_key_file_set_string(keyfile, "general", "logfile",
+  config->log_filepath);
+}
+g_key_file_set_string(keyfile, "general", "pidfile", config->pid_filepath);
+#ifdef CONFIG_FSFREEZE
+if (config->fsfreeze_hook) {
+g_key_file_set_string(keyfile, "general", "fsfreeze-hook",
+  config->fsfreeze_hook);
+}
+#endif
+g_key_file_set_string(keyfile, "general", "statedir", config->state_dir);
+g_key_file_set_boolean(keyfile, "general", "verbose",
+   config->log_level == G_LOG_LEVEL_MASK);
+tmp = list_join(config->blacklist, ',');
+g_key_file_set_string(keyfile, "general", "blacklist", tmp);
+g_free(tmp);
+
+tmp = g_key_file_to_data(keyfile, NULL, &error);
+printf("%s", tmp);
+
+g_free(tmp);
+if (keyfile) {
+g_key_file_free(keyfile);
+}
+}
+
 static void config_parse(GAConfig *config, int argc, char **argv)
 {
 const char *sopt = "hVvdm:p:l:f:F::b:s:t:D";
@@ -1016,6 +1069,7 @@ static void config_parse(GAConfig *config, int argc, char 
**argv)
 const struct option lopt[] = {
 { "help", 0, NULL, 'h' },
 { "version", 0, NULL, 'V' },
+{ "dump-conf", 0, NULL, 'D' },
 { "logfile", 1, NULL, 'l' },
 { "pidfile", 1, NULL, 'f' },
 #ifdef CONFIG_FSFREEZE
@@ -1067,6 +1121,9 @@ static void config_parse(GAConfig *config, int argc, char 
**argv)
 case 'd':
 config->daemonize = 1;
 break;
+case 'D':
+config->dumpconf = 1;
+break;
 case 'b': {
 if (is_help_option(optarg)) {
 qmp_for_each_command(ga_print_cmd, NULL);
@@ -1310,6 +1367,11 @@ int main(int argc, char **argv)
  config->state_dir);
 s->frozen = check_is_frozen(s);
 
+if (config->dumpconf) {
+config_dump(config);
+goto end;
+}
+
 ret = run_agent(s, config);
 
 end:
-- 
2.4.3

[Qemu-devel] [PATCH v12 1/5] hw/intc: Implement GIC-500 base class

[Pavel Fedin]

From: Shlomo Pongratz 

This class is to be used by both software and KVM implementations of GICv3

Currently it is mostly a placeholder, but in future it is supposed to hold
qemu's representation of GICv3 state, which is necessary for migration.

The interface of this class is fully compatible with GICv2 one. This is
done in order to simplify integration with existing code.

Signed-off-by: Shlomo Pongratz 
Signed-off-by: Pavel Fedin 
Reviewed-by: Eric Auger 
Tested-by: Ashok kumar 
---
 hw/intc/Makefile.objs  |   1 +
 hw/intc/arm_gicv3_common.c | 140 +
 include/hw/intc/arm_gicv3_common.h |  68 ++
 3 files changed, 209 insertions(+)
 create mode 100644 hw/intc/arm_gicv3_common.c
 create mode 100644 include/hw/intc/arm_gicv3_common.h

diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs
index 092d8a8..1317e5a 100644
--- a/hw/intc/Makefile.objs
+++ b/hw/intc/Makefile.objs
@@ -12,6 +12,7 @@ common-obj-$(CONFIG_IOAPIC) += ioapic_common.o
 common-obj-$(CONFIG_ARM_GIC) += arm_gic_common.o
 common-obj-$(CONFIG_ARM_GIC) += arm_gic.o
 common-obj-$(CONFIG_ARM_GIC) += arm_gicv2m.o
+common-obj-$(CONFIG_ARM_GIC) += arm_gicv3_common.o
 common-obj-$(CONFIG_OPENPIC) += openpic.o
 
 obj-$(CONFIG_APIC) += apic.o apic_common.o
diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
new file mode 100644
index 000..032ece2
--- /dev/null
+++ b/hw/intc/arm_gicv3_common.c
@@ -0,0 +1,140 @@
+/*
+ * ARM GICv3 support - common bits of emulated and KVM kernel model
+ *
+ * Copyright (c) 2012 Linaro Limited
+ * Copyright (c) 2015 Huawei.
+ * Written by Peter Maydell
+ * Extended to 64 cores by Shlomo Pongratz
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see .
+ */
+
+#include "hw/intc/arm_gicv3_common.h"
+
+static void gicv3_pre_save(void *opaque)
+{
+GICv3State *s = (GICv3State *)opaque;
+ARMGICv3CommonClass *c = ARM_GICV3_COMMON_GET_CLASS(s);
+
+if (c->pre_save) {
+c->pre_save(s);
+}
+}
+
+static int gicv3_post_load(void *opaque, int version_id)
+{
+GICv3State *s = (GICv3State *)opaque;
+ARMGICv3CommonClass *c = ARM_GICV3_COMMON_GET_CLASS(s);
+
+if (c->post_load) {
+c->post_load(s);
+}
+return 0;
+}
+
+static const VMStateDescription vmstate_gicv3 = {
+.name = "arm_gicv3",
+.unmigratable = 1,
+.pre_save = gicv3_pre_save,
+.post_load = gicv3_post_load,
+};
+
+void gicv3_init_irqs_and_mmio(GICv3State *s, qemu_irq_handler handler,
+  const MemoryRegionOps *ops)
+{
+SysBusDevice *sbd = SYS_BUS_DEVICE(s);
+int i;
+
+/* For the GIC, also expose incoming GPIO lines for PPIs for each CPU.
+ * GPIO array layout is thus:
+ *  [0..N-1] spi
+ *  [N..N+31] PPIs for CPU 0
+ *  [N+32..N+63] PPIs for CPU 1
+ *   ...
+ */
+i = s->num_irq - GIC_INTERNAL + GIC_INTERNAL * s->num_cpu;
+qdev_init_gpio_in(DEVICE(s), handler, i);
+
+s->parent_irq = g_malloc(s->num_cpu * sizeof(qemu_irq));
+s->parent_fiq = g_malloc(s->num_cpu * sizeof(qemu_irq));
+
+for (i = 0; i < s->num_cpu; i++) {
+sysbus_init_irq(sbd, &s->parent_irq[i]);
+}
+for (i = 0; i < s->num_cpu; i++) {
+sysbus_init_irq(sbd, &s->parent_fiq[i]);
+}
+
+memory_region_init_io(&s->iomem_dist, OBJECT(s), ops, s,
+  "gicv3_dist", 0x1);
+memory_region_init_io(&s->iomem_redist, OBJECT(s), ops ? &ops[1] : NULL, s,
+  "gicv3_redist", 0x2 * s->num_cpu);
+
+sysbus_init_mmio(sbd, &s->iomem_dist);
+sysbus_init_mmio(sbd, &s->iomem_redist);
+}
+
+static void arm_gicv3_common_realize(DeviceState *dev, Error **errp)
+{
+GICv3State *s = ARM_GICV3_COMMON(dev);
+
+/* revision property is actually reserved and currently used only in order
+ * to keep the interface compatible with GICv2 code, avoiding extra
+ * conditions. However, in future it could be used, for example, if we
+ * implement GICv4.
+ */
+if (s->revision != 3) {
+error_setg(errp, "unsupported GIC revision %d", s->revision);
+return;
+}
+}
+
+static void arm_gicv3_common_reset(DeviceState *dev)
+{
+/* TODO */
+}
+
+static Property arm_gicv3_common_properties[] = {
+DEFINE_PROP_UINT32("num-cpu", GICv3State, num_cpu, 1),
+DEFINE_PROP_UINT32("num-irq", GICv3Stat

[Qemu-devel] [PATCH v12 0/5] vGICv3 support

[Pavel Fedin]

This series introduces support for GICv3 by KVM. Software emulation is
currently not supported.

V11 => V12
- Do not set timer PPI CPU mask in device tree for GICv3
- Fixed small styling issues in hw/arm/virt.c
- Completely untied kvm_gic_access() from GIC and turned into
  kvm_device_access(). Attribute mapping is now handled in
  gicd/gicc wrapper macros.

v11 => v10
- Fixed minor issues with checkpatch and comments, reported by Eric Auger
- Make reusable kvm_gic_supports_attr(), moved to kvm-all.c and renamed
  as kvm_device_check_attr(). Useful for future live migration.

v9 => v10
- Renamed "gicversion" option to "gic-version" (was forgotten in v9)
- Data pointer in kvm_gic_access() is now void * because in case of
  vGICv3 this function is expected to operate on 64-bit registers too
  (GICD_IROUTER for instance)

v8 => v9
- Removed all limitations on CPU and IRQ number from the base class
- Added back missing properties, interface is now the same as in GICv2
- Refactored reusable parts of vGICv2 code, decreased number of changes
- Removed GIC type check from kvm_arch_irqchip_create(), no more need to
  specify GIC type early
- Fixed up all commit messages / logs
- Removed 'nvic' field assignment in virt machine (was forgotten in v8)
- CPU number limitation for 'virt' machine now comes from memory map
  (how many redistributors can be placed). With current layout it appears
  to be 126.

v7 => v8
- Removed all unused SW emulation code
- Removed unnecessary attributes from common class
- Set "unmigratable" flag for GICv3 device
- Removed unnecessary conditions from kvm_arm_gicv3_realize()
- Fixed GIC type setting in vexpress model, was done in wrong place
- Fixed condition style in hw/intc/Makefile.objs
- Cleaned up virt machine memory map

v6 => v7
- Wrap own GIC type definitions on top of KVM ones. Fixed build on
  non-ARM-Linux hosts

v5 => v6
- Fixed various checkpatch.pl style warnings
- Removed TODO in gicv3_init_irqs_and_mmio(), relevant memory API patch
  included
- gicv3_init_irqs_and_mmio() now takes 3 arguments instead of 4. It is more
  convenient to pass MMIO descriptors as array

v4 => v5
- Do not reintroduce several constants shared with GICv2, reuse them instead.
- Added gicv3_init_irqs_and_mmio() in base class, to be used by both software
  emulation and KVM code. Avoids code duplication.
- Do not add NULL msi-parent phandle to PCI device in the FDT
- Removed a couple of stale things from virt.c

v3 => v4
- Fixed stupid build breakage in patch 0002
- Rebased on top of current master, patch 0003 adjusted according to
  kvm_irqchip_create() changes
- Added assertion against uninitialized kernel_irqchip_type
- Removed kernel_irqchip_type initialization from models which do not
  use KVM vGIC

v2 => v3
- Removed some unrelated and unnecessary changes from virt machine,
  occasionally slipped in; some of them caused qemu to crash on ARM32.
- Fixed build for ARM32; vGICv3 code requires definitions which are
  present only in ARM64 kernel

v1 => v2
- Base class included, taken from the series by Shlomo Pongratz:
  http://lists.nongnu.org/archive/html/qemu-devel/2015-06/msg01512.html
  The code is refactored as little as possible in order to simplify
  further addition of software emulation:
  - Minor fixes in code style and comments, according to old reviews
  - Removed REV_V3 definition because it's currently not used, and it does
not add any meaning to number 3.
  - Removed reserved regions for MBI and ITS (except for 'virt' machine
memory map). These should go to separate classes when implemented.
- Improved commit messages
- vGIC patches restructured
- Use 'gicversion' option instead of virt-v3 machine

Pavel Fedin (4):
  intc/gic: Extract some reusable vGIC code
  arm_kvm: Do not assume particular GIC type in
kvm_arch_irqchip_create()
  hw/intc: Initial implementation of vGICv3
  hw/arm/virt: Add gic-version option to virt machine

Shlomo Pongratz (1):
  hw/intc: Implement GIC-500 base class

 hw/arm/virt.c  | 118 +++--
 hw/intc/Makefile.objs  |   2 +
 hw/intc/arm_gic_kvm.c  |  84 -
 hw/intc/arm_gicv3_common.c | 140 ++
 hw/intc/arm_gicv3_kvm.c| 149 +
 hw/intc/vgic_common.h  |  49 
 include/hw/arm/virt.h  |   5 +-
 include/hw/intc/arm_gicv3_common.h |  68 +
 include/sysemu/kvm.h   |  26 +++
 kvm-all.c  |  33 
 target-arm/kvm.c   |  10 +--
 target-arm/kvm_arm.h   |  10 +++
 target-arm/machine.c   |  18 +
 13 files changed, 611 insertions(+), 101 deletions(-)
 create mode 100644 hw/intc/arm_gicv3_common.c
 create mode 100644 hw/intc/arm_gicv3_kvm.c
 create mode 100644 hw/intc/vgic_common.h
 create mode 100644 include/hw/intc/arm_gicv3_common.h

-- 
1.9.5.msysgit.0

[Qemu-devel] [PATCH v12 3/5] arm_kvm: Do not assume particular GIC type in kvm_arch_irqchip_create()

[Pavel Fedin]

This allows to use different GIC types from v2. There are no kernels which
could advertise KVM_CAP_DEVICE_CTRL without the actual ability to create
GIC with it.

Signed-off-by: Pavel Fedin 
Reviewed-by: Eric Auger 
Tested-by: Ashok kumar 
---
 target-arm/kvm.c | 10 +-
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/target-arm/kvm.c b/target-arm/kvm.c
index b278542..22383c5 100644
--- a/target-arm/kvm.c
+++ b/target-arm/kvm.c
@@ -585,18 +585,10 @@ void kvm_arch_init_irq_routing(KVMState *s)
 
 int kvm_arch_irqchip_create(KVMState *s)
 {
-int ret;
-
 /* If we can create the VGIC using the newer device control API, we
  * let the device do this when it initializes itself, otherwise we
  * fall back to the old API */
-
-ret = kvm_create_device(s, KVM_DEV_TYPE_ARM_VGIC_V2, true);
-if (ret == 0) {
-return 1;
-}
-
-return 0;
+return kvm_check_extension(s, KVM_CAP_DEVICE_CTRL);
 }
 
 int kvm_arch_fixup_msi_route(struct kvm_irq_routing_entry *route,
-- 
1.9.5.msysgit.0

[Qemu-devel] [PATCH v12 4/5] hw/intc: Initial implementation of vGICv3

[Pavel Fedin]

This is the initial version of KVM-accelerated GICv3 support.
State load and save are not yet supported, live migration is
not possible.

In order to get correct class name in a simpler way, gicv3_class_name()
function is implemented, similar to gic_class_name().

Signed-off-by: Pavel Fedin 
Reviewed-by: Peter Maydell 
Tested-by: Ashok kumar 
---
 hw/intc/Makefile.objs   |   1 +
 hw/intc/arm_gicv3_kvm.c | 149 
 target-arm/kvm_arm.h|  10 
 target-arm/machine.c|  18 ++
 4 files changed, 178 insertions(+)
 create mode 100644 hw/intc/arm_gicv3_kvm.c

diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs
index 1317e5a..004b0c2 100644
--- a/hw/intc/Makefile.objs
+++ b/hw/intc/Makefile.objs
@@ -17,6 +17,7 @@ common-obj-$(CONFIG_OPENPIC) += openpic.o
 
 obj-$(CONFIG_APIC) += apic.o apic_common.o
 obj-$(CONFIG_ARM_GIC_KVM) += arm_gic_kvm.o
+obj-$(call land,$(CONFIG_ARM_GIC_KVM),$(TARGET_AARCH64)) += arm_gicv3_kvm.o
 obj-$(CONFIG_STELLARIS) += armv7m_nvic.o
 obj-$(CONFIG_EXYNOS4) += exynos4210_gic.o exynos4210_combiner.o
 obj-$(CONFIG_GRLIB) += grlib_irqmp.o
diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
new file mode 100644
index 000..b48f78f
--- /dev/null
+++ b/hw/intc/arm_gicv3_kvm.c
@@ -0,0 +1,149 @@
+/*
+ * ARM Generic Interrupt Controller using KVM in-kernel support
+ *
+ * Copyright (c) 2015 Samsung Electronics Co., Ltd.
+ * Written by Pavel Fedin
+ * Based on vGICv2 code by Peter Maydell
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see .
+ */
+
+#include "hw/intc/arm_gicv3_common.h"
+#include "hw/sysbus.h"
+#include "sysemu/kvm.h"
+#include "kvm_arm.h"
+#include "vgic_common.h"
+
+#ifdef DEBUG_GICV3_KVM
+#define DPRINTF(fmt, ...) \
+do { fprintf(stderr, "kvm_gicv3: " fmt, ## __VA_ARGS__); } while (0)
+#else
+#define DPRINTF(fmt, ...) \
+do { } while (0)
+#endif
+
+#define TYPE_KVM_ARM_GICV3 "kvm-arm-gicv3"
+#define KVM_ARM_GICV3(obj) \
+ OBJECT_CHECK(GICv3State, (obj), TYPE_KVM_ARM_GICV3)
+#define KVM_ARM_GICV3_CLASS(klass) \
+ OBJECT_CLASS_CHECK(KVMARMGICv3Class, (klass), TYPE_KVM_ARM_GICV3)
+#define KVM_ARM_GICV3_GET_CLASS(obj) \
+ OBJECT_GET_CLASS(KVMARMGICv3Class, (obj), TYPE_KVM_ARM_GICV3)
+
+typedef struct KVMARMGICv3Class {
+ARMGICv3CommonClass parent_class;
+DeviceRealize parent_realize;
+void (*parent_reset)(DeviceState *dev);
+} KVMARMGICv3Class;
+
+static void kvm_arm_gicv3_set_irq(void *opaque, int irq, int level)
+{
+GICv3State *s = (GICv3State *)opaque;
+
+kvm_arm_gic_set_irq(s->num_irq, irq, level);
+}
+
+static void kvm_arm_gicv3_put(GICv3State *s)
+{
+/* TODO */
+DPRINTF("Cannot put kernel gic state, no kernel interface\n");
+}
+
+static void kvm_arm_gicv3_get(GICv3State *s)
+{
+/* TODO */
+DPRINTF("Cannot get kernel gic state, no kernel interface\n");
+}
+
+static void kvm_arm_gicv3_reset(DeviceState *dev)
+{
+GICv3State *s = ARM_GICV3_COMMON(dev);
+KVMARMGICv3Class *kgc = KVM_ARM_GICV3_GET_CLASS(s);
+
+DPRINTF("Reset\n");
+
+kgc->parent_reset(dev);
+kvm_arm_gicv3_put(s);
+}
+
+static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp)
+{
+GICv3State *s = KVM_ARM_GICV3(dev);
+KVMARMGICv3Class *kgc = KVM_ARM_GICV3_GET_CLASS(s);
+Error *local_err = NULL;
+
+DPRINTF("kvm_arm_gicv3_realize\n");
+
+kgc->parent_realize(dev, &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
+
+if (s->security_extn) {
+error_setg(errp, "the in-kernel VGICv3 does not implement the "
+   "security extensions");
+return;
+}
+
+gicv3_init_irqs_and_mmio(s, kvm_arm_gicv3_set_irq, NULL);
+
+/* Try to create the device via the device control API */
+s->dev_fd = kvm_create_device(kvm_state, KVM_DEV_TYPE_ARM_VGIC_V3, false);
+if (s->dev_fd < 0) {
+error_setg_errno(errp, -s->dev_fd, "error creating in-kernel VGIC");
+return;
+}
+
+kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_NR_IRQS,
+  0, &s->num_irq, true);
+
+/* Tell the kernel to complete VGIC initialization now */
+kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_CTRL,
+  KVM_DEV_ARM_VGIC_CTRL_INIT, NULL, true);
+
+kvm_arm_register_device(&s->iomem_dist, -1, KVM_DEV_ARM_VGIC_GRP_ADDR,
+

[Qemu-devel] [PATCH v12 5/5] hw/arm/virt: Add gic-version option to virt machine

[Pavel Fedin]

Add gic_version to VirtMachineState, set it to value of the option
and pass it around where necessary. Instantiate devices and fdt
nodes according to the choice.

max_cpus for virt machine increased to 126 (calculated from redistributor
space available in the memory map). GICv2 compatibility check happens
inside arm_gic_common_realize().

ITS regions are added to the memory map too, however currently they
are not used, just reserved.

Signed-off-by: Pavel Fedin 
Tested-by: Ashok kumar 
---
 hw/arm/virt.c | 118 --
 include/hw/arm/virt.h |   5 ++-
 2 files changed, 99 insertions(+), 24 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index d5a8417..7044cb9 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -50,6 +50,7 @@
 #include "hw/arm/fdt.h"
 #include "hw/intc/arm_gic_common.h"
 #include "kvm_arm.h"
+#include "qapi/visitor.h"
 
 /* Number of external interrupt lines to configure the GIC with */
 #define NUM_IRQS 256
@@ -79,6 +80,7 @@ typedef struct {
 typedef struct {
 MachineState parent;
 bool secure;
+int32_t gic_version;
 } VirtMachineState;
 
 #define TYPE_VIRT_MACHINE   "virt"
@@ -109,6 +111,10 @@ static const MemMapEntry a15memmap[] = {
 [VIRT_GIC_DIST] =   { 0x0800, 0x0001 },
 [VIRT_GIC_CPU] ={ 0x0801, 0x0001 },
 [VIRT_GIC_V2M] ={ 0x0802, 0x1000 },
+[VIRT_ITS_CONTROL] ={ 0x0802, 0x0001 },
+[VIRT_ITS_TRANSLATION] ={ 0x0803, 0x0001 },
+/* This redistributor space allows up to 2*64kB*126 CPUs */
+[VIRT_GIC_REDIST] = { 0x0804, 0x00FC },
 [VIRT_UART] =   { 0x0900, 0x1000 },
 [VIRT_RTC] ={ 0x0901, 0x1000 },
 [VIRT_FW_CFG] = { 0x0902, 0x000a },
@@ -251,7 +257,7 @@ static void fdt_add_psci_node(const VirtBoardInfo *vbi)
 qemu_fdt_setprop_cell(fdt, "/psci", "migrate", migrate_fn);
 }
 
-static void fdt_add_timer_nodes(const VirtBoardInfo *vbi)
+static void fdt_add_timer_nodes(const VirtBoardInfo *vbi, int gictype)
 {
 /* Note that on A15 h/w these interrupts are level-triggered,
  * but for the GIC implementation provided by both QEMU and KVM
@@ -260,8 +266,11 @@ static void fdt_add_timer_nodes(const VirtBoardInfo *vbi)
 ARMCPU *armcpu;
 uint32_t irqflags = GIC_FDT_IRQ_FLAGS_EDGE_LO_HI;
 
-irqflags = deposit32(irqflags, GIC_FDT_IRQ_PPI_CPU_START,
- GIC_FDT_IRQ_PPI_CPU_WIDTH, (1 << vbi->smp_cpus) - 1);
+if (gictype == 2) {
+irqflags = deposit32(irqflags, GIC_FDT_IRQ_PPI_CPU_START,
+ GIC_FDT_IRQ_PPI_CPU_WIDTH,
+ (1 << vbi->smp_cpus) - 1);
+}
 
 qemu_fdt_add_subnode(vbi->fdt, "/timer");
 
@@ -285,6 +294,18 @@ static void fdt_add_cpu_nodes(const VirtBoardInfo *vbi)
 {
 int cpu;
 
+/*
+ * From Documentation/devicetree/bindings/arm/cpus.txt
+ *  On ARM v8 64-bit systems value should be set to 2,
+ *  that corresponds to the MPIDR_EL1 register size.
+ *  If MPIDR_EL1[63:32] value is equal to 0 on all CPUs
+ *  in the system, #address-cells can be set to 1, since
+ *  MPIDR_EL1[63:32] bits are not used for CPUs
+ *  identification.
+ *
+ *  Now GIC500 doesn't support affinities 2 & 3 so currently
+ *  #address-cells can stay 1 until future GIC
+ */
 qemu_fdt_add_subnode(vbi->fdt, "/cpus");
 qemu_fdt_setprop_cell(vbi->fdt, "/cpus", "#address-cells", 0x1);
 qemu_fdt_setprop_cell(vbi->fdt, "/cpus", "#size-cells", 0x0);
@@ -321,25 +342,36 @@ static void fdt_add_v2m_gic_node(VirtBoardInfo *vbi)
 qemu_fdt_setprop_cell(vbi->fdt, "/intc/v2m", "phandle", vbi->v2m_phandle);
 }
 
-static void fdt_add_gic_node(VirtBoardInfo *vbi)
+static void fdt_add_gic_node(VirtBoardInfo *vbi, int type)
 {
 vbi->gic_phandle = qemu_fdt_alloc_phandle(vbi->fdt);
 qemu_fdt_setprop_cell(vbi->fdt, "/", "interrupt-parent", vbi->gic_phandle);
 
 qemu_fdt_add_subnode(vbi->fdt, "/intc");
-/* 'cortex-a15-gic' means 'GIC v2' */
-qemu_fdt_setprop_string(vbi->fdt, "/intc", "compatible",
-"arm,cortex-a15-gic");
 qemu_fdt_setprop_cell(vbi->fdt, "/intc", "#interrupt-cells", 3);
 qemu_fdt_setprop(vbi->fdt, "/intc", "interrupt-controller", NULL, 0);
-qemu_fdt_setprop_sized_cells(vbi->fdt, "/intc", "reg",
- 2, vbi->memmap[VIRT_GIC_DIST].base,
- 2, vbi->memmap[VIRT_GIC_DIST].size,
- 2, vbi->memmap[VIRT_GIC_CPU].base,
- 2, vbi->memmap[VIRT_GIC_CPU].size);
 qemu_fdt_setprop_cell(vbi->fdt, "/intc", "#address-cells", 0x2);
 qemu_fdt_setprop_cell(vbi->fdt, "/intc", "#size-cells", 0x2);
 qemu_fdt_setprop(vbi->fdt, "/intc", "ranges", NULL, 0);
+if (type == 3) {
+qemu_fdt_setprop_st

[Qemu-devel] [PATCH v12 2/5] intc/gic: Extract some reusable vGIC code

[Pavel Fedin]

Some functions previously used only by vGICv2 are useful also for vGICv3
implementation. Untie them from GICState and make accessible from within
other modules:
- kvm_arm_gic_set_irq()
- kvm_gic_supports_attr() - moved to common code and renamed to
  kvm_device_check_attr()
- kvm_gic_access() - turned into GIC-independent kvm_device_access().
  Data pointer changed to void * because some GICv3 registers are
  64-bit wide
- kvm_gicd_access()
- kvm_gicc_access() - actually GICv2-specific, but changed to keep the
  code style unified with kvm_gicd_access()

Some of these changes are not used right now, but they will be helpful for
implementing live migration.

Actually kvm_dist_get() and kvm_dist_put() could also be made reusable, but
they would require two extra parameters (s->dev_fd and s->num_cpu) as well as
lots of typecasts of 's' to DeviceState * and back to GICState *. This makes
the code very ugly so i decided to stop at this point. I tried also an
approach with making a base class for all possible GICs, but it would contain
only three variables (dev_fd, cpu_num and irq_num), and accessing them through
the rest of the code would be again tedious (either ugly casts or qemu-style
separate object pointer). So i disliked it too.

Signed-off-by: Pavel Fedin 
Tested-by: Ashok kumar 
---
 hw/intc/arm_gic_kvm.c | 84 ++-
 hw/intc/vgic_common.h | 49 ++
 include/sysemu/kvm.h  | 26 
 kvm-all.c | 33 
 4 files changed, 124 insertions(+), 68 deletions(-)
 create mode 100644 hw/intc/vgic_common.h

diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c
index e5d0f67..e129c90 100644
--- a/hw/intc/arm_gic_kvm.c
+++ b/hw/intc/arm_gic_kvm.c
@@ -23,6 +23,7 @@
 #include "sysemu/kvm.h"
 #include "kvm_arm.h"
 #include "gic_internal.h"
+#include "vgic_common.h"
 
 //#define DEBUG_GIC_KVM
 
@@ -52,7 +53,7 @@ typedef struct KVMARMGICClass {
 void (*parent_reset)(DeviceState *dev);
 } KVMARMGICClass;
 
-static void kvm_arm_gic_set_irq(void *opaque, int irq, int level)
+void kvm_arm_gic_set_irq(uint32_t num_irq, int irq, int level)
 {
 /* Meaning of the 'irq' parameter:
  *  [0..N-1] : external interrupts
@@ -63,10 +64,9 @@ static void kvm_arm_gic_set_irq(void *opaque, int irq, int 
level)
  * has separate fields in the irq number for type,
  * CPU number and interrupt number.
  */
-GICState *s = (GICState *)opaque;
 int kvm_irq, irqtype, cpu;
 
-if (irq < (s->num_irq - GIC_INTERNAL)) {
+if (irq < (num_irq - GIC_INTERNAL)) {
 /* External interrupt. The kernel numbers these like the GIC
  * hardware, with external interrupt IDs starting after the
  * internal ones.
@@ -77,7 +77,7 @@ static void kvm_arm_gic_set_irq(void *opaque, int irq, int 
level)
 } else {
 /* Internal interrupt: decode into (cpu, interrupt id) */
 irqtype = KVM_ARM_IRQ_TYPE_PPI;
-irq -= (s->num_irq - GIC_INTERNAL);
+irq -= (num_irq - GIC_INTERNAL);
 cpu = irq / GIC_INTERNAL;
 irq %= GIC_INTERNAL;
 }
@@ -87,69 +87,16 @@ static void kvm_arm_gic_set_irq(void *opaque, int irq, int 
level)
 kvm_set_irq(kvm_state, kvm_irq, !!level);
 }
 
-static bool kvm_arm_gic_can_save_restore(GICState *s)
-{
-return s->dev_fd >= 0;
-}
-
-static bool kvm_gic_supports_attr(GICState *s, int group, int attrnum)
-{
-struct kvm_device_attr attr = {
-.group = group,
-.attr = attrnum,
-.flags = 0,
-};
-
-if (s->dev_fd == -1) {
-return false;
-}
-
-return kvm_device_ioctl(s->dev_fd, KVM_HAS_DEVICE_ATTR, &attr) == 0;
-}
-
-static void kvm_gic_access(GICState *s, int group, int offset,
-   int cpu, uint32_t *val, bool write)
+static void kvm_arm_gicv2_set_irq(void *opaque, int irq, int level)
 {
-struct kvm_device_attr attr;
-int type;
-int err;
-
-cpu = cpu & 0xff;
-
-attr.flags = 0;
-attr.group = group;
-attr.attr = (((uint64_t)cpu << KVM_DEV_ARM_VGIC_CPUID_SHIFT) &
- KVM_DEV_ARM_VGIC_CPUID_MASK) |
-(((uint64_t)offset << KVM_DEV_ARM_VGIC_OFFSET_SHIFT) &
- KVM_DEV_ARM_VGIC_OFFSET_MASK);
-attr.addr = (uintptr_t)val;
-
-if (write) {
-type = KVM_SET_DEVICE_ATTR;
-} else {
-type = KVM_GET_DEVICE_ATTR;
-}
-
-err = kvm_device_ioctl(s->dev_fd, type, &attr);
-if (err < 0) {
-fprintf(stderr, "KVM_{SET/GET}_DEVICE_ATTR failed: %s\n",
-strerror(-err));
-abort();
-}
-}
+GICState *s = (GICState *)opaque;
 
-static void kvm_gicd_access(GICState *s, int offset, int cpu,
-uint32_t *val, bool write)
-{
-kvm_gic_access(s, KVM_DEV_ARM_VGIC_GRP_DIST_REGS,
-   offset, cpu, val, write);
+kvm_arm_gic_set_irq(s->num_irq, irq, level);
 }
 
-static void kvm_gicc_access(GICState *

Re: [Qemu-devel] [PATCH v2 08/18] nvdimm: init backend memory mapping and config data area

[Xiao Guangrong]

On 08/26/2015 12:03 AM, Stefan Hajnoczi wrote:

On Fri, Aug 14, 2015 at 10:52:01PM +0800, Xiao Guangrong wrote:

The parameter @file is used as backed memory for NVDIMM which is
divided into two parts if @dataconfig is true:


s/dataconfig/configdata/


Stupid typo, sorry.




@@ -76,13 +109,87 @@ static void pc_nvdimm_init(Object *obj)
   set_configdata, NULL);
  }

+static uint64_t get_file_size(int fd)
+{
+struct stat stat_buf;
+uint64_t size;
+
+if (fstat(fd, &stat_buf) < 0) {
+return 0;
+}
+
+if (S_ISREG(stat_buf.st_mode)) {
+return stat_buf.st_size;
+}
+
+if (S_ISBLK(stat_buf.st_mode) && !ioctl(fd, BLKGETSIZE64, &size)) {
+return size;
+}


#ifdef __linux__ for ioctl(fd, BLKGETSIZE64, &size)?

There is nothing Linux-specific about emulating NVDIMMs so this code
should compile on all platforms.


Right. I have no idea for how block devices work on other platforms so
I will only allow linux to directly use bock device file in the next
version.




+
+return 0;
+}
+
  static void pc_nvdimm_realize(DeviceState *dev, Error **errp)
  {
  PCNVDIMMDevice *nvdimm = PC_NVDIMM(dev);
+char name[512];
+void *buf;
+ram_addr_t addr;
+uint64_t size, nvdimm_size, config_size = MIN_CONFIG_DATA_SIZE;
+int fd;

  if (!nvdimm->file) {
  error_setg(errp, "file property is not set");
  }


Missing return here.


Will fix.




+
+fd = open(nvdimm->file, O_RDWR);


Does it make sense to support read-only NVDIMMs?

It could be handy for sharing a read-only file between unprivileged
guests.  The permissions on the file would only allow read, not write.


Make sense. Currently these patchset just implements "shared" mode so that
write permission is required, however, please see below:




+if (fd < 0) {
+error_setg(errp, "can not open %s", nvdimm->file);


s/can not/cannot/


+return;
+}
+
+size = get_file_size(fd);
+buf = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);


I guess the user will want to choose between MAP_SHARED and MAP_PRIVATE.
This can be added in the future.


Good idea, it will allow guest to write data but discards its content after it
exits. Will implement O_RDONLY + MAP_PRIVATE in the near future.




+if (buf == MAP_FAILED) {
+error_setg(errp, "can not do mmap on %s", nvdimm->file);
+goto do_close;
+}
+
+nvdimm->config_data_size = config_size;
+if (nvdimm->configdata) {
+/* reserve MIN_CONFIGDATA_AREA_SIZE for configue data. */
+nvdimm_size = size - config_size;
+nvdimm->config_data_addr = buf + nvdimm_size;
+} else {
+nvdimm_size = size;
+nvdimm->config_data_addr = NULL;
+}
+
+if ((int64_t)nvdimm_size <= 0) {


The error cases can be detected before mmap(2).  That avoids the int64_t
cast and also avoids nvdimm_size underflow and the bogus
nvdimm->config_data_addr calculation above.


Okay.



size = get_file_size(fd);
if (size == 0) {
 error_setg(errp, "empty file or unable to get file size");
 goto do_close;
} else if (nvdimm->configdata && size < config_size) {{
 error_setg(errp, "file size is too small to store NVDIMM"
  " configure data");
 goto do_close;
}


+error_setg(errp, "file size is too small to store NVDIMM"
+ " configure data");
+goto do_unmap;
+}
+
+addr = reserved_range_push(nvdimm_size);
+if (!addr) {
+error_setg(errp, "do not have enough space for size %#lx.\n", size);


error_setg() messages must not have a newline at the end.

Please use "%#" PRIx64 instead of "%#lx" so compilation works on 32-bit
hosts where sizeof(long) == 4.


Good catch.




+goto do_unmap;
+}
+
+nvdimm->device_index = new_device_index();
+sprintf(name, "NVDIMM-%d", nvdimm->device_index);
+memory_region_init_ram_ptr(&nvdimm->mr, OBJECT(dev), name, nvdimm_size,
+   buf);


How is the autogenerated name used?

Why not just use "pc-nvdimm.memory"?


Ah. Just for debug proposal :) and i am not sure if a name used for multiple
MRs (MemoryRegion) is a good idea.




+vmstate_register_ram(&nvdimm->mr, DEVICE(dev));
+memory_region_add_subregion(get_system_memory(), addr, &nvdimm->mr);
+
+return;


fd is leaked.


Will fix.

Re: [Qemu-devel] [PATCH v2 10/18] nvdimm: init the address region used by DSM method

[Xiao Guangrong]

On 08/26/2015 12:11 AM, Stefan Hajnoczi wrote:

On Fri, Aug 14, 2015 at 10:52:03PM +0800, Xiao Guangrong wrote:

@@ -257,14 +258,91 @@ static void build_nfit_table(GSList *device_list, char 
*buf)
  }
  }

+struct dsm_buffer {
+/* RAM page. */
+uint32_t handle;
+uint8_t arg0[16];
+uint32_t arg1;
+uint32_t arg2;
+union {
+char arg3[PAGE_SIZE - 3 * sizeof(uint32_t) - 16 * sizeof(uint8_t)];
+};
+
+/* MMIO page. */
+union {
+uint32_t notify;
+char pedding[PAGE_SIZE];


s/pedding/padding/


Will fix.




+};
+};
+
+static ram_addr_t dsm_addr;
+static size_t dsm_size;
+
+static uint64_t dsm_read(void *opaque, hwaddr addr,
+ unsigned size)
+{
+return 0;
+}
+
+static void dsm_write(void *opaque, hwaddr addr,
+  uint64_t val, unsigned size)
+{
+}
+
+static const MemoryRegionOps dsm_ops = {
+.read = dsm_read,
+.write = dsm_write,
+.endianness = DEVICE_LITTLE_ENDIAN,
+};
+
+static int build_dsm_buffer(void)
+{
+MemoryRegion *dsm_ram_mr, *dsm_mmio_mr;
+ram_addr_t addr;;


s/;;/;/


Will fix.

Re: [Qemu-devel] [PATCH v2 13/18] nvdimm: build namespace config data

[Xiao Guangrong]

On 08/26/2015 12:16 AM, Stefan Hajnoczi wrote:

On Fri, Aug 14, 2015 at 10:52:06PM +0800, Xiao Guangrong wrote:

+#ifdef NVDIMM_DEBUG
+#define nvdebug(fmt, ...) fprintf(stderr, "nvdimm: " fmt, ## __VA_ARGS__)
+#else
+#define nvdebug(...)
+#endif


The following allows the compiler to check format strings and syntax
check the argument expressions:

#define NVDIMM_DEBUG 0  /* set to 1 for debug output */
#define nvdebug(fmt, ...) \
 if (NVDIMM_DEBUG) { \
 fprintf(stderr, "nvdimm: " fmt, ## __VA_ARGS__); \
 }

This approach avoids bitrot (e.g. debug format string arguments have
become outdated).



Really good tips, thanks for your sharing.

Re: [Qemu-devel] [PATCH v2 14/18] nvdimm: support NFIT_CMD_IMPLEMENTED function

[Xiao Guangrong]

On 08/26/2015 12:23 AM, Stefan Hajnoczi wrote:

On Fri, Aug 14, 2015 at 10:52:07PM +0800, Xiao Guangrong wrote:

@@ -306,6 +354,18 @@ struct dsm_buffer {
  static ram_addr_t dsm_addr;
  static size_t dsm_size;

+struct cmd_out_implemented {


QEMU coding style uses typedef struct {} CamelCase.  Please follow this
convention in all user-defined structs (see ./CODING_STYLE).



Okay, will adjust all the defines in the next version.


  static void dsm_write(void *opaque, hwaddr addr,
uint64_t val, unsigned size)
  {
+struct MemoryRegion *dsm_ram_mr = opaque;
+struct dsm_buffer *dsm;
+struct dsm_out *out;
+void *buf;
+
  assert(val == NOTIFY_VALUE);


The guest should not be able to cause an abort(3).  If val !=
NOTIFY_VALUE we can do nvdebug() and then return.


The ACPI code and emulation code both are from qemu, if that happens,
it's really a bug, aborting the VM is better than throwing a debug
message under this case to avoid potential data corruption.




+
+buf = memory_region_get_ram_ptr(dsm_ram_mr);
+dsm = buf;
+out = buf;
+
+le32_to_cpus(&dsm->handle);
+le32_to_cpus(&dsm->arg1);
+le32_to_cpus(&dsm->arg2);


Can SMP guests modify DSM RAM while this thread is running?

We must avoid race conditions.  It's probably better to copy in data
before byte-swapping or checking input values.


Yes, my mistake, will fix.

Re: [Qemu-devel] [PATCH v2 15/18] nvdimm: support NFIT_CMD_GET_CONFIG_SIZE function

[Xiao Guangrong]

On 08/26/2015 12:24 AM, Stefan Hajnoczi wrote:

On Fri, Aug 14, 2015 at 10:52:08PM +0800, Xiao Guangrong wrote:

Function 4 is used to get Namespace lable size


s/lable/label/



Stupid me, will fix the change log.

Re: [Qemu-devel] [PATCH v2 00/18] implement vNVDIMM

[Xiao Guangrong]

On 08/26/2015 12:26 AM, Stefan Hajnoczi wrote:

On Fri, Aug 14, 2015 at 10:51:53PM +0800, Xiao Guangrong wrote:

Changlog:
- Use litten endian for DSM method, thanks for Stefan's suggestion

- introduce a new parameter, @configdata, if it's false, Qemu will
   build a static and readonly namespace in memory and use it serveing
   for DSM GET_CONFIG_SIZE/GET_CONFIG_DATA requests. In this case, no
   reserved region is needed at the end of the @file, it is good for
   the user who want to pass whole nvdimm device and make its data
   completely be visible to guest

- divide the source code into separated files and add maintain info


I have skipped ACPI patches because I'm not very familiar with that
area.


Thank you very much for your review, your comment is great helpful to
me, Stefan!



Have you thought about live migration?

Are the contents of the NVDIMM migrated since they are registered as a
RAM region?


Will fully test live migration and VM save before sending the V3 out. :)

Re: [Qemu-devel] [PATCH] target-i386: enable cflushopt/clwb/pcommit instructions

[Xiao Guangrong]

On 08/22/2015 12:05 AM, Eduardo Habkost wrote:

On Fri, Aug 21, 2015 at 01:05:12PM +0800, Xiao Guangrong wrote:

These instructions are used by NVDIMM drivers and the specification
locates at:
https://software.intel.com/sites/default/files/managed/0d/53/319433-022.pdf

Let them be enabled on Broadwell on default

Signed-off-by: Xiao Guangrong 


This breaks the ABI. If you change a CPU model you need machine compat
code (see the TYPE_X86_CPU entries in PC_COMPAT_2_3 for example).

And you can only change the CPU model in a new machine-type if that
doesn't affect the runnability of a VM. In other words:

If:
   -machine pc-i440fx-2.4 -cpu ,enforcec
is runnable in a given host, then
   -machine pc-i440fx-2.5 -cpu ,enforce
should be runnable too.



Eduardo, thanks for your info. I will dig into the code and fix this
issue in the next version.

[Qemu-devel] [PATCH 3/7] maint: remove unused include for assert.h

[Daniel P. Berrange]

A number of files were including assert.h but not using any
of the functions it provides

Signed-off-by: Daniel P. Berrange 
---
 disas/ia64.c  | 1 -
 hw/i386/xen/xen_platform.c| 2 --
 linux-user/signal.c   | 1 -
 target-microblaze/op_helper.c | 1 -
 target-moxie/helper.c | 1 -
 target-moxie/translate.c  | 1 -
 target-sh4/op_helper.c| 1 -
 tests/test-xbzrle.c   | 1 -
 8 files changed, 9 deletions(-)

diff --git a/disas/ia64.c b/disas/ia64.c
index a8fe26c..d7c7bdf 100644
--- a/disas/ia64.c
+++ b/disas/ia64.c
@@ -18,7 +18,6 @@
along with this file; see the file COPYING.  If not, see
. */
 
-#include 
 #include 
 
 #include "disas/bfd.h"
diff --git a/hw/i386/xen/xen_platform.c b/hw/i386/xen/xen_platform.c
index 28b324a..ee45f03 100644
--- a/hw/i386/xen/xen_platform.c
+++ b/hw/i386/xen/xen_platform.c
@@ -23,8 +23,6 @@
  * THE SOFTWARE.
  */
 
-#include 
-
 #include "hw/hw.h"
 #include "hw/i386/pc.h"
 #include "hw/ide.h"
diff --git a/linux-user/signal.c b/linux-user/signal.c
index 9d4cef4..502efd9 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -22,7 +22,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 
diff --git a/target-microblaze/op_helper.c b/target-microblaze/op_helper.c
index d2b3624..092c4b5 100644
--- a/target-microblaze/op_helper.c
+++ b/target-microblaze/op_helper.c
@@ -18,7 +18,6 @@
  * License along with this library; if not, see .
  */
 
-#include 
 #include "cpu.h"
 #include "exec/helper-proto.h"
 #include "qemu/host-utils.h"
diff --git a/target-moxie/helper.c b/target-moxie/helper.c
index f21e884..f91ac28 100644
--- a/target-moxie/helper.c
+++ b/target-moxie/helper.c
@@ -19,7 +19,6 @@
 
 #include 
 #include 
-#include 
 
 #include "config.h"
 #include "cpu.h"
diff --git a/target-moxie/translate.c b/target-moxie/translate.c
index e3e9139..cc77366 100644
--- a/target-moxie/translate.c
+++ b/target-moxie/translate.c
@@ -26,7 +26,6 @@
 #include 
 #include 
 #include 
-#include 
 
 #include "cpu.h"
 #include "exec/exec-all.h"
diff --git a/target-sh4/op_helper.c b/target-sh4/op_helper.c
index cbc11ae..a312118 100644
--- a/target-sh4/op_helper.c
+++ b/target-sh4/op_helper.c
@@ -16,7 +16,6 @@
  * You should have received a copy of the GNU Lesser General Public
  * License along with this library; if not, see .
  */
-#include 
 #include 
 #include "cpu.h"
 #include "exec/helper-proto.h"
diff --git a/tests/test-xbzrle.c b/tests/test-xbzrle.c
index db93b0a..b5ee8bb 100644
--- a/tests/test-xbzrle.c
+++ b/tests/test-xbzrle.c
@@ -16,7 +16,6 @@
 #include 
 #include 
 #include 
-#include 
 #include "qemu-common.h"
 #include "include/migration/migration.h"
 
-- 
2.4.3

[Qemu-devel] [PATCH 1/7] maint: remove double semicolons in many files

[Daniel P. Berrange]

A number of source files have statements accidentally
terminated by a double semicolon - eg 'foo = bar;;'.
This is harmless but a mistake none the less.

The tcg/ia64/tcg-target.c file is whitelisted because
it has valid use of ';;' in a comment containing assembly
code.

Signed-off-by: Daniel P. Berrange 
---
 block/vhdx.c | 2 +-
 hw/arm/vexpress.c| 4 ++--
 hw/intc/arm_gic.c| 2 +-
 numa.c   | 2 +-
 qga/commands-win32.c | 2 +-
 5 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/block/vhdx.c b/block/vhdx.c
index 0776de7..f05c7a9 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1454,7 +1454,7 @@ static int vhdx_create_new_metadata(BlockDriverState *bs,
 uint32_t offset = 0;
 void *buffer = NULL;
 void *entry_buffer;
-VHDXMetadataTableHeader *md_table;;
+VHDXMetadataTableHeader *md_table;
 VHDXMetadataTableEntry  *md_table_entry;
 
 /* Metadata entries */
diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
index da21788..0f8bf1c 100644
--- a/hw/arm/vexpress.c
+++ b/hw/arm/vexpress.c
@@ -541,7 +541,7 @@ static void vexpress_common_init(MachineState *machine)
 {
 VexpressMachineState *vms = VEXPRESS_MACHINE(machine);
 VexpressMachineClass *vmc = VEXPRESS_MACHINE_GET_CLASS(machine);
-VEDBoardInfo *daughterboard = vmc->daughterboard;;
+VEDBoardInfo *daughterboard = vmc->daughterboard;
 DeviceState *dev, *sysctl, *pl041;
 qemu_irq pic[64];
 uint32_t sys_id;
@@ -762,7 +762,7 @@ static void vexpress_a9_class_init(ObjectClass *oc, void 
*data)
 mc->name = TYPE_VEXPRESS_A9_MACHINE;
 mc->desc = "ARM Versatile Express for Cortex-A9";
 
-vmc->daughterboard = &a9_daughterboard;;
+vmc->daughterboard = &a9_daughterboard;
 }
 
 static void vexpress_a15_class_init(ObjectClass *oc, void *data)
diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
index a8c5d19..46ebc3f 100644
--- a/hw/intc/arm_gic.c
+++ b/hw/intc/arm_gic.c
@@ -239,7 +239,7 @@ uint32_t gic_acknowledge_irq(GICState *s, int cpu, 
MemTxAttrs attrs)
  * for the case where this GIC supports grouping and the pending interrupt
  * is in the wrong group.
  */
-irq = gic_get_current_pending_irq(s, cpu, attrs);;
+irq = gic_get_current_pending_irq(s, cpu, attrs);
 
 if (irq >= GIC_MAXIRQ) {
 DPRINTF("ACK, no pending interrupt or it is hidden: %d\n", irq);
diff --git a/numa.c b/numa.c
index 402804b..eed8f5d 100644
--- a/numa.c
+++ b/numa.c
@@ -280,7 +280,7 @@ static void validate_numa_cpus(void)
 bitmap_and(seen_cpus, seen_cpus,
numa_info[i].node_cpu, MAX_CPUMASK_BITS);
 error_report("CPU(s) present in multiple NUMA nodes: %s",
- enumerate_cpus(seen_cpus, max_cpus));;
+ enumerate_cpus(seen_cpus, max_cpus));
 exit(EXIT_FAILURE);
 }
 bitmap_or(seen_cpus, seen_cpus,
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index a7822d5..203664c 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -657,7 +657,7 @@ static GuestFilesystemInfo *build_guest_fsinfo(char *guid, 
Error **errp)
 fs->mountpoint = g_strndup(mnt_point, len);
 }
 fs->type = g_strdup(fs_name);
-fs->disk = build_guest_disk_info(guid, errp);;
+fs->disk = build_guest_disk_info(guid, errp);
 free:
 g_free(mnt_point);
 return fs;
-- 
2.4.3

[Qemu-devel] [PATCH 4/7] maint: remove unused include for dirent.h

[Daniel P. Berrange]

A number of files were including dirent.h but not using any
of the functions it provides

Signed-off-by: Daniel P. Berrange 
---
 fsdev/virtio-9p-marshal.c | 1 -
 hw/tpm/tpm_passthrough.c  | 2 --
 hw/usb/redirect.c | 1 -
 hw/vfio/pci.c | 1 -
 qemu-char.c   | 1 -
 5 files changed, 6 deletions(-)

diff --git a/fsdev/virtio-9p-marshal.c b/fsdev/virtio-9p-marshal.c
index 20f308b..7748d32 100644
--- a/fsdev/virtio-9p-marshal.c
+++ b/fsdev/virtio-9p-marshal.c
@@ -14,7 +14,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
index 79a8f98..be160c1 100644
--- a/hw/tpm/tpm_passthrough.c
+++ b/hw/tpm/tpm_passthrough.c
@@ -22,8 +22,6 @@
  * License along with this library; if not, see 
  */
 
-#include 
-
 #include "qemu-common.h"
 #include "qapi/error.h"
 #include "qemu/error-report.h"
diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
index 25df25f..34cf60d 100644
--- a/hw/usb/redirect.c
+++ b/hw/usb/redirect.c
@@ -33,7 +33,6 @@
 #include "qemu/iov.h"
 #include "sysemu/char.h"
 
-#include 
 #include 
 #include 
 #include 
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 4023d8e..5d02099 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -18,7 +18,6 @@
  *  Copyright (C) 2008, IBM, Muli Ben-Yehuda (m...@il.ibm.com)
  */
 
-#include 
 #include 
 #include 
 #include 
diff --git a/qemu-char.c b/qemu-char.c
index d956f8d..fa65159 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -51,7 +51,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #ifdef CONFIG_BSD
-- 
2.4.3

[Qemu-devel] [PATCH 2/7] maint: remove / fix many doubled words

[Daniel P. Berrange]

Many source files have doubled words (eg "the the", "to to",
and so on). Most of these can simply be removed, but a couple
were actual mis-spellings (eg "to to" instead of "to do").
There was even one triple word score "to to to" :-)

Signed-off-by: Daniel P. Berrange 
---
 block/qcow2-cluster.c | 4 ++--
 block/qcow2-refcount.c| 2 +-
 docs/libcacard.txt| 4 ++--
 docs/multiseat.txt| 2 +-
 docs/specs/qcow2.txt  | 2 +-
 docs/specs/rocker.txt | 2 +-
 hw/net/rtl8139.c  | 2 +-
 hw/usb/host-libusb.c  | 2 +-
 hw/vfio/common.c  | 2 +-
 include/block/block.h | 2 +-
 include/exec/memory.h | 2 +-
 linux-user/elfload.c  | 2 +-
 migration/rdma.c  | 2 +-
 qemu-doc.texi | 2 +-
 qemu-img.texi | 2 +-
 qemu-options.hx   | 2 +-
 target-arm/cpu.h  | 4 ++--
 target-arm/helper.c   | 2 +-
 target-arm/translate.c| 2 +-
 target-lm32/helper.c  | 2 +-
 target-microblaze/translate.c | 2 +-
 target-moxie/helper.c | 2 +-
 util/bitmap.c | 2 +-
 23 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index b43f186..61309ae 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -498,7 +498,7 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, uint64_t 
offset,
 
 *cluster_offset = 0;
 
-/* seek the the l2 offset in the l1 table */
+/* seek to the l2 offset in the l1 table */
 
 l1_index = offset >> l1_bits;
 if (l1_index >= s->l1_size) {
@@ -612,7 +612,7 @@ static int get_cluster_table(BlockDriverState *bs, uint64_t 
offset,
 uint64_t *l2_table = NULL;
 int ret;
 
-/* seek the the l2 offset in the l1 table */
+/* seek to the l2 offset in the l1 table */
 
 l1_index = offset >> (s->l2_bits + s->cluster_bits);
 if (l1_index >= s->l1_size) {
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index b0ee42d..d8f0645 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -1294,7 +1294,7 @@ static int realloc_refcount_array(BDRVQcowState *s, void 
**array,
 /*
  * Increases the refcount for a range of clusters in a given refcount table.
  * This is used to construct a temporary refcount table out of L1 and L2 tables
- * which can be compared the the refcount table saved in the image.
+ * which can be compared to the refcount table saved in the image.
  *
  * Modifies the number of errors in res.
  */
diff --git a/docs/libcacard.txt b/docs/libcacard.txt
index 8db421d..499cf7d 100644
--- a/docs/libcacard.txt
+++ b/docs/libcacard.txt
@@ -327,7 +327,7 @@ and applet.
 
 int vcard_emul_get_login_count(VCard *card);
 
-This function returns the the number of remaining login attempts for this
+This function returns the number of remaining login attempts for this
 card. If the card emulator does not know, or the card does not have a
 way of giving this information, this function returns -1.
 
@@ -421,7 +421,7 @@ functions:
   The vcard is the value returned from vcard_new. The type is the
   card type emulator that this card should presented to the guest as.
   The flags are card type emulator specific options. The certs,
-  cert_len, and keys are all arrays of length cert_count. These are the
+  cert_len, and keys are all arrays of length cert_count. These are
   the same of the parameters _card_init() accepts.
 
Finally the card is associated with its reader by the call:
diff --git a/docs/multiseat.txt b/docs/multiseat.txt
index ebf2446..807518c 100644
--- a/docs/multiseat.txt
+++ b/docs/multiseat.txt
@@ -135,7 +135,7 @@ configuration:
 TAG+="seat", ENV{ID_AUTOSEAT}="1"
 
 Patch with this rule has been submitted to upstream udev/systemd, was
-accepted and and should be included in the next systemd release (222).
+accepted and should be included in the next systemd release (222).
 So, if your guest has this or a newer version, multiseat will work just
 fine without any manual guest configuration.
 
diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt
index 121dfc8..f236d8c 100644
--- a/docs/specs/qcow2.txt
+++ b/docs/specs/qcow2.txt
@@ -257,7 +257,7 @@ L2 table entry:
 
   63:   0 for a cluster that is unused or requires COW, 1 if its
 refcount is exactly one. This information is only accurate
-in L2 tables that are reachable from the the active L1
+in L2 tables that are reachable from the active L1
 table.
 
 Standard Cluster Descriptor:
diff --git a/docs/specs/rocker.txt b/docs/specs/rocker.txt
index 1c74351..d2a8262 100644
--- a/docs/specs/rocker.txt
+++ b/docs/specs/rocker.txt
@@ -297,7 +297,7 @@ but not fired.  If only partial credits are returned, the 
interrupt remains
 masked but the device generates an interrupt, signaling the 

[Qemu-devel] [PATCH 0/7] Misc trivial code cleanups

[Daniel P. Berrange]

This is a series of misc trivial code cleanups that I
originally used to illustrate the use of GNULIB's
syntax-check facility:

  http://lists.nongnu.org/archive/html/qemu-devel/2015-07/msg06268.html

Since it appears the license of the GNULIB syntax-check
is problematic, this series has separated the fixes out
from the GNULIB additions, to allow for possibility of
merging them as-is.

Daniel P. Berrange (7):
  maint: remove double semicolons in many files
  maint: remove / fix many doubled words
  maint: remove unused include for assert.h
  maint: remove unused include for dirent.h
  maint: remove unused include for signal.h
  maint: remove unused include for strings.h
  maint: avoid useless "if (foo) free(foo)" pattern

 backends/hostmem-file.c   |  4 +---
 block/qcow2-cluster.c |  4 ++--
 block/qcow2-refcount.c|  2 +-
 block/vhdx.c  |  2 +-
 bsd-user/elfload.c|  4 +---
 bsd-user/signal.c |  1 -
 disas/ia64.c  |  1 -
 disas/microblaze.c|  1 -
 disas/sparc.c |  3 +--
 docs/libcacard.txt|  4 ++--
 docs/multiseat.txt|  2 +-
 docs/specs/qcow2.txt  |  2 +-
 docs/specs/rocker.txt |  2 +-
 fsdev/virtio-9p-marshal.c |  1 -
 hw/arm/vexpress.c |  4 ++--
 hw/block/xen_disk.c   |  1 -
 hw/bt/hci.c   |  9 +++--
 hw/core/loader.c  |  3 +--
 hw/core/qdev-properties.c |  4 +---
 hw/display/exynos4210_fimd.c  |  4 +---
 hw/i386/xen/xen_platform.c|  2 --
 hw/intc/arm_gic.c |  2 +-
 hw/mips/mips_r4k.c|  4 +---
 hw/net/fsl_etsec/rings.c  |  4 +---
 hw/net/rocker/rocker.c|  4 +---
 hw/net/rocker/rocker_desc.c   |  8 ++--
 hw/net/rtl8139.c  |  2 +-
 hw/net/xen_nic.c  |  1 -
 hw/nvram/fw_cfg.c |  4 +---
 hw/pci-host/prep.c|  4 +---
 hw/pci/shpc.c |  1 -
 hw/sd/sd.c|  3 +--
 hw/tpm/tpm_passthrough.c  |  2 --
 hw/usb/hcd-xhci.c |  4 +---
 hw/usb/host-libusb.c  |  2 +-
 hw/usb/redirect.c |  2 --
 hw/vfio/common.c  |  2 +-
 hw/vfio/pci.c |  1 -
 hw/xen/xen_pt_config_init.c   |  4 +---
 include/block/block.h |  2 +-
 include/exec/memory.h |  2 +-
 linux-user/elfload.c  |  2 +-
 linux-user/signal.c   |  1 -
 migration/rdma.c  |  2 +-
 migration/savevm.c|  8 ++--
 numa.c|  2 +-
 os-win32.c|  1 -
 page_cache.c  |  1 -
 qemu-char.c   |  5 +
 qemu-doc.texi |  2 +-
 qemu-img.texi |  2 +-
 qemu-options.hx   |  2 +-
 qga/commands-win32.c  |  2 +-
 target-arm/cpu.h  |  4 ++--
 target-arm/helper.c   |  2 +-
 target-arm/translate.c|  2 +-
 target-i386/translate.c   |  1 -
 target-lm32/helper.c  |  2 +-
 target-microblaze/op_helper.c |  1 -
 target-microblaze/translate.c |  2 +-
 target-mips/helper.c  |  1 -
 target-moxie/helper.c |  3 +--
 target-moxie/translate.c  |  1 -
 target-sh4/helper.c   |  1 -
 target-sh4/op_helper.c|  1 -
 target-tricore/helper.c   |  1 -
 tests/bios-tables-test.c  | 36 +---
 tests/tcg/testthread.c|  1 -
 tests/test-xbzrle.c   |  2 --
 ui/spice-display.c| 14 --
 util/bitmap.c |  2 +-
 71 files changed, 71 insertions(+), 154 deletions(-)

-- 
2.4.3

[Qemu-devel] [PATCH 5/7] maint: remove unused include for signal.h

[Daniel P. Berrange]

A number of files were including signal.h but not using any
of the functions it provides

Signed-off-by: Daniel P. Berrange 
---
 bsd-user/signal.c   | 1 -
 hw/block/xen_disk.c | 1 -
 hw/net/xen_nic.c| 1 -
 hw/usb/redirect.c   | 1 -
 os-win32.c  | 1 -
 target-i386/translate.c | 1 -
 target-mips/helper.c| 1 -
 target-sh4/helper.c | 1 -
 target-tricore/helper.c | 1 -
 tests/tcg/testthread.c  | 1 -
 10 files changed, 10 deletions(-)

diff --git a/bsd-user/signal.c b/bsd-user/signal.c
index 445f69e..e4ee2d0 100644
--- a/bsd-user/signal.c
+++ b/bsd-user/signal.c
@@ -21,7 +21,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 
 #include "qemu.h"
diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index 267d8a8..36d7398 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -24,7 +24,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
diff --git a/hw/net/xen_nic.c b/hw/net/xen_nic.c
index d7cbfc1..0da16b4 100644
--- a/hw/net/xen_nic.c
+++ b/hw/net/xen_nic.c
@@ -24,7 +24,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
index 34cf60d..38086cd 100644
--- a/hw/usb/redirect.c
+++ b/hw/usb/redirect.c
@@ -34,7 +34,6 @@
 #include "sysemu/char.h"
 
 #include 
-#include 
 #include 
 #include 
 
diff --git a/os-win32.c b/os-win32.c
index c0daf8e..cc09196 100644
--- a/os-win32.c
+++ b/os-win32.c
@@ -26,7 +26,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
diff --git a/target-i386/translate.c b/target-i386/translate.c
index 82e2245..b1a5ad9 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -21,7 +21,6 @@
 #include 
 #include 
 #include 
-#include 
 
 #include "qemu/host-utils.h"
 #include "cpu.h"
diff --git a/target-mips/helper.c b/target-mips/helper.c
index f44edbb..37bba67 100644
--- a/target-mips/helper.c
+++ b/target-mips/helper.c
@@ -21,7 +21,6 @@
 #include 
 #include 
 #include 
-#include 
 
 #include "cpu.h"
 #include "sysemu/kvm.h"
diff --git a/target-sh4/helper.c b/target-sh4/helper.c
index a533f08..dc101cb 100644
--- a/target-sh4/helper.c
+++ b/target-sh4/helper.c
@@ -21,7 +21,6 @@
 #include 
 #include 
 #include 
-#include 
 
 #include "cpu.h"
 
diff --git a/target-tricore/helper.c b/target-tricore/helper.c
index f52504c..1808b28 100644
--- a/target-tricore/helper.c
+++ b/target-tricore/helper.c
@@ -20,7 +20,6 @@
 #include 
 #include 
 #include 
-#include 
 
 #include "cpu.h"
 
diff --git a/tests/tcg/testthread.c b/tests/tcg/testthread.c
index 2679af1..810ba5d 100644
--- a/tests/tcg/testthread.c
+++ b/tests/tcg/testthread.c
@@ -2,7 +2,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
-- 
2.4.3

[Qemu-devel] [PATCH 6/7] maint: remove unused include for strings.h

[Daniel P. Berrange]

A number of files were including strings.h but not using any
of the functions it provides

Signed-off-by: Daniel P. Berrange 
---
 disas/microblaze.c  | 1 -
 hw/pci/shpc.c   | 1 -
 page_cache.c| 1 -
 tests/test-xbzrle.c | 1 -
 4 files changed, 4 deletions(-)

diff --git a/disas/microblaze.c b/disas/microblaze.c
index c14ab89..92cd4b1 100644
--- a/disas/microblaze.c
+++ b/disas/microblaze.c
@@ -582,7 +582,6 @@ static const char pvr_register_prefix[] = "rpvr";
 #endif /* MICROBLAZE_OPC */
 
 #include "disas/bfd.h"
-#include 
 
 #define get_field_rd(instr) get_field(instr, RD_MASK, RD_LOW)
 #define get_field_r1(instr) get_field(instr, RA_MASK, RA_LOW)
diff --git a/hw/pci/shpc.c b/hw/pci/shpc.c
index bfb4d31..d34fdf3 100644
--- a/hw/pci/shpc.c
+++ b/hw/pci/shpc.c
@@ -1,5 +1,4 @@
 #include "qemu-common.h"
-#include 
 #include 
 #include "qemu/range.h"
 #include "qemu/error-report.h"
diff --git a/page_cache.c b/page_cache.c
index cf8878d..a9eb076 100644
--- a/page_cache.c
+++ b/page_cache.c
@@ -15,7 +15,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
diff --git a/tests/test-xbzrle.c b/tests/test-xbzrle.c
index b5ee8bb..1cd8cb7 100644
--- a/tests/test-xbzrle.c
+++ b/tests/test-xbzrle.c
@@ -13,7 +13,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include "qemu-common.h"
-- 
2.4.3

[Qemu-devel] [PATCH 7/7] maint: avoid useless "if (foo) free(foo)" pattern

[Daniel P. Berrange]

The free() and g_free() functions both happily accept
NULL on any platform QEMU builds on. As such putting a
conditional 'if (foo)' check before calls to 'free(foo)'
merely serves to bloat the lines of code.

Signed-off-by: Daniel P. Berrange 
---
 backends/hostmem-file.c  |  4 +---
 bsd-user/elfload.c   |  4 +---
 disas/sparc.c|  3 +--
 hw/bt/hci.c  |  9 +++--
 hw/core/loader.c |  3 +--
 hw/core/qdev-properties.c|  4 +---
 hw/display/exynos4210_fimd.c |  4 +---
 hw/mips/mips_r4k.c   |  4 +---
 hw/net/fsl_etsec/rings.c |  4 +---
 hw/net/rocker/rocker.c   |  4 +---
 hw/net/rocker/rocker_desc.c  |  8 ++--
 hw/nvram/fw_cfg.c|  4 +---
 hw/pci-host/prep.c   |  4 +---
 hw/sd/sd.c   |  3 +--
 hw/usb/hcd-xhci.c|  4 +---
 hw/xen/xen_pt_config_init.c  |  4 +---
 migration/savevm.c   |  8 ++--
 qemu-char.c  |  4 +---
 tests/bios-tables-test.c | 36 +---
 ui/spice-display.c   | 14 --
 20 files changed, 39 insertions(+), 93 deletions(-)

diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
index 4b55361..e9b6d21 100644
--- a/backends/hostmem-file.c
+++ b/backends/hostmem-file.c
@@ -83,9 +83,7 @@ static void set_mem_path(Object *o, const char *str, Error 
**errp)
 error_setg(errp, "cannot change property value");
 return;
 }
-if (fb->mem_path) {
-g_free(fb->mem_path);
-}
+g_free(fb->mem_path);
 fb->mem_path = g_strdup(str);
 }
 
diff --git a/bsd-user/elfload.c b/bsd-user/elfload.c
index 2bf57eb..a8ce43d 100644
--- a/bsd-user/elfload.c
+++ b/bsd-user/elfload.c
@@ -1355,9 +1355,7 @@ int load_elf_binary(struct linux_binprm * bprm, struct 
target_pt_regs * regs,
 }
 }
 if (!bprm->p) {
-if (elf_interpreter) {
-free(elf_interpreter);
-}
+free(elf_interpreter);
 free (elf_phdata);
 close(bprm->fd);
 return -E2BIG;
diff --git a/disas/sparc.c b/disas/sparc.c
index f4e3565..59a1e36 100644
--- a/disas/sparc.c
+++ b/disas/sparc.c
@@ -2622,8 +2622,7 @@ build_hash_table (const sparc_opcode **opcode_table,
 
   memset (hash_table, 0, HASH_SIZE * sizeof (hash_table[0]));
   memset (hash_count, 0, HASH_SIZE * sizeof (hash_count[0]));
-  if (hash_buf != NULL)
-free (hash_buf);
+  free(hash_buf);
   hash_buf = malloc (sizeof (* hash_buf) * num_opcodes);
   for (i = num_opcodes - 1; i >= 0; --i)
 {
diff --git a/hw/bt/hci.c b/hw/bt/hci.c
index 7ea3dc6..3fec435 100644
--- a/hw/bt/hci.c
+++ b/hw/bt/hci.c
@@ -1151,8 +1151,7 @@ static void bt_hci_reset(struct bt_hci_s *hci)
 hci->event_mask[7] = 0x00;
 hci->device.inquiry_scan = 0;
 hci->device.page_scan = 0;
-if (hci->device.lmp_name)
-g_free((void *) hci->device.lmp_name);
+g_free((void *) hci->device.lmp_name);
 hci->device.lmp_name = NULL;
 hci->device.class[0] = 0x00;
 hci->device.class[1] = 0x00;
@@ -1829,8 +1828,7 @@ static void bt_submit_hci(struct HCIInfo *info,
 case cmd_opcode_pack(OGF_HOST_CTL, OCF_CHANGE_LOCAL_NAME):
 LENGTH_CHECK(change_local_name);
 
-if (hci->device.lmp_name)
-g_free((void *) hci->device.lmp_name);
+g_free((void *) hci->device.lmp_name);
 hci->device.lmp_name = g_strndup(PARAM(change_local_name, name),
 sizeof(PARAM(change_local_name, name)));
 bt_hci_event_complete_status(hci, HCI_SUCCESS);
@@ -2231,8 +2229,7 @@ static void bt_hci_done(struct HCIInfo *info)
 
 bt_device_done(&hci->device);
 
-if (hci->device.lmp_name)
-g_free((void *) hci->device.lmp_name);
+g_free((void *) hci->device.lmp_name);
 
 /* Be gentle and send DISCONNECT to all connected peers and those
  * currently waiting for us to accept or reject a connection request.
diff --git a/hw/core/loader.c b/hw/core/loader.c
index 216eeeb..a96a74e 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -594,8 +594,7 @@ static int load_uboot_image(const char *filename, hwaddr 
*ep, hwaddr *loadaddr,
 ret = hdr->ih_size;
 
 out:
-if (data)
-g_free(data);
+g_free(data);
 close(fd);
 return ret;
 }
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 04fd80a..33e245e 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -422,9 +422,7 @@ static void set_string(Object *obj, Visitor *v, void 
*opaque,
 error_propagate(errp, local_err);
 return;
 }
-if (*ptr) {
-g_free(*ptr);
-}
+g_free(*ptr);
 *ptr = str;
 }
 
diff --git a/hw/display/exynos4210_fimd.c b/hw/display/exynos4210_fimd.c
index 603ef50..45239e8 100644
--- a/hw/display/exynos4210_fimd.c
+++ b/hw/display/exynos4210_fimd.c
@@ -1354,9 +1354,7 @@ static void exynos4210_fimd_reset(DeviceState *d)
 fimd_update

Re: [Qemu-devel] [PATCH v2 00/45] ivshmem improvements (for 2.5)

[Marc-André Lureau]

Hi

On Tue, Jul 28, 2015 at 2:32 AM, Marc-André Lureau
 wrote:
> Hi,
>
> This series is mostly about adding the client/server code from David
> Marchand, code cleanups, and little improvements for ivshmem. Finally
> there is some ivshmem tests (they work fine without kvm).
>
> The first series didn't get much feedback. As suggested by Andrew
> Jones, this second version adds support for hugepage shm, in qemu with
> a hostmem backend and a fix in the server side (instead of relying on
> bad/old glibc hack). It also adds irqfd usage for msix notification.
>
> (the branch is on github: https://github.com/elmarco/qemu.git ivshmem)


Anybody up to review or test this series?

thanks

-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH v2 22/45] ivshmem: migrate with VMStateDescription

[Marc-André Lureau]

Hi

On Tue, Jul 28, 2015 at 2:32 AM, Marc-André Lureau
 wrote:
> If necessary, load_state_old() could be used to keep compatibility with
> verison 0.


fwiw, I updated this patch with load_state_old()

-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH 1/7] maint: remove double semicolons in many files

[Marc-André Lureau]

Reviewed-by: Marc-André Lureau 




-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH 2/7] maint: remove / fix many doubled words

[Marc-André Lureau]

nice :)

Reviewed-by: Marc-André Lureau 

Re: [Qemu-devel] [PATCH 7/7] maint: avoid useless "if (foo) free(foo)" pattern

[Marc-André Lureau]

Hi

On Wed, Aug 26, 2015 at 1:17 PM, Daniel P. Berrange  wrote:
> The free() and g_free() functions both happily accept
> NULL on any platform QEMU builds on. As such putting a
> conditional 'if (foo)' check before calls to 'free(foo)'
> merely serves to bloat the lines of code.
>
> Signed-off-by: Daniel P. Berrange 
> ---
>  backends/hostmem-file.c  |  4 +---
>  bsd-user/elfload.c   |  4 +---
>  disas/sparc.c|  3 +--
>  hw/bt/hci.c  |  9 +++--
>  hw/core/loader.c |  3 +--
>  hw/core/qdev-properties.c|  4 +---
>  hw/display/exynos4210_fimd.c |  4 +---
>  hw/mips/mips_r4k.c   |  4 +---
>  hw/net/fsl_etsec/rings.c |  4 +---
>  hw/net/rocker/rocker.c   |  4 +---
>  hw/net/rocker/rocker_desc.c  |  8 ++--
>  hw/nvram/fw_cfg.c|  4 +---
>  hw/pci-host/prep.c   |  4 +---
>  hw/sd/sd.c   |  3 +--
>  hw/usb/hcd-xhci.c|  4 +---
>  hw/xen/xen_pt_config_init.c  |  4 +---
>  migration/savevm.c   |  8 ++--
>  qemu-char.c  |  4 +---
>  tests/bios-tables-test.c | 36 +---
>  ui/spice-display.c   | 14 --
>  20 files changed, 39 insertions(+), 93 deletions(-)
>
> diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
> index 4b55361..e9b6d21 100644
> --- a/backends/hostmem-file.c
> +++ b/backends/hostmem-file.c
> @@ -83,9 +83,7 @@ static void set_mem_path(Object *o, const char *str, Error 
> **errp)
>  error_setg(errp, "cannot change property value");
>  return;
>  }
> -if (fb->mem_path) {
> -g_free(fb->mem_path);
> -}
> +g_free(fb->mem_path);
>  fb->mem_path = g_strdup(str);
>  }
>
> diff --git a/bsd-user/elfload.c b/bsd-user/elfload.c
> index 2bf57eb..a8ce43d 100644
> --- a/bsd-user/elfload.c
> +++ b/bsd-user/elfload.c
> @@ -1355,9 +1355,7 @@ int load_elf_binary(struct linux_binprm * bprm, struct 
> target_pt_regs * regs,
>  }
>  }
>  if (!bprm->p) {
> -if (elf_interpreter) {
> -free(elf_interpreter);
> -}
> +free(elf_interpreter);
>  free (elf_phdata);
>  close(bprm->fd);
>  return -E2BIG;
> diff --git a/disas/sparc.c b/disas/sparc.c
> index f4e3565..59a1e36 100644
> --- a/disas/sparc.c
> +++ b/disas/sparc.c
> @@ -2622,8 +2622,7 @@ build_hash_table (const sparc_opcode **opcode_table,
>
>memset (hash_table, 0, HASH_SIZE * sizeof (hash_table[0]));
>memset (hash_count, 0, HASH_SIZE * sizeof (hash_count[0]));
> -  if (hash_buf != NULL)
> -free (hash_buf);
> +  free(hash_buf);
>hash_buf = malloc (sizeof (* hash_buf) * num_opcodes);
>for (i = num_opcodes - 1; i >= 0; --i)
>  {
> diff --git a/hw/bt/hci.c b/hw/bt/hci.c
> index 7ea3dc6..3fec435 100644
> --- a/hw/bt/hci.c
> +++ b/hw/bt/hci.c
> @@ -1151,8 +1151,7 @@ static void bt_hci_reset(struct bt_hci_s *hci)
>  hci->event_mask[7] = 0x00;
>  hci->device.inquiry_scan = 0;
>  hci->device.page_scan = 0;
> -if (hci->device.lmp_name)
> -g_free((void *) hci->device.lmp_name);
> +g_free((void *) hci->device.lmp_name);
>  hci->device.lmp_name = NULL;
>  hci->device.class[0] = 0x00;
>  hci->device.class[1] = 0x00;
> @@ -1829,8 +1828,7 @@ static void bt_submit_hci(struct HCIInfo *info,
>  case cmd_opcode_pack(OGF_HOST_CTL, OCF_CHANGE_LOCAL_NAME):
>  LENGTH_CHECK(change_local_name);
>
> -if (hci->device.lmp_name)
> -g_free((void *) hci->device.lmp_name);
> +g_free((void *) hci->device.lmp_name);
>  hci->device.lmp_name = g_strndup(PARAM(change_local_name, name),
>  sizeof(PARAM(change_local_name, name)));
>  bt_hci_event_complete_status(hci, HCI_SUCCESS);
> @@ -2231,8 +2229,7 @@ static void bt_hci_done(struct HCIInfo *info)
>
>  bt_device_done(&hci->device);
>
> -if (hci->device.lmp_name)
> -g_free((void *) hci->device.lmp_name);
> +g_free((void *) hci->device.lmp_name);
>
>  /* Be gentle and send DISCONNECT to all connected peers and those
>   * currently waiting for us to accept or reject a connection request.
> diff --git a/hw/core/loader.c b/hw/core/loader.c
> index 216eeeb..a96a74e 100644
> --- a/hw/core/loader.c
> +++ b/hw/core/loader.c
> @@ -594,8 +594,7 @@ static int load_uboot_image(const char *filename, hwaddr 
> *ep, hwaddr *loadaddr,
>  ret = hdr->ih_size;
>
>  out:
> -if (data)
> -g_free(data);
> +g_free(data);
>  close(fd);
>  return ret;
>  }
> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> index 04fd80a..33e245e 100644
> --- a/hw/core/qdev-properties.c
> +++ b/hw/core/qdev-properties.c
> @@ -422,9 +422,7 @@ static void set_string(Object *obj, Visitor *v, void 
> *opaque,
>  error_propagate(errp, local_err);
>  return;
>  }
> -if (*ptr) {
> -g_free(*p

Re: [Qemu-devel] [PATCH] virtio dataplane: adapt dataplane for virtio Version 1

[Greg Kurz]

On Tue, 25 Aug 2015 12:33:30 +0200
Pierre Morel  wrote:

> Let dataplane allocate different region for the desc/avail/used
> ring regions.
> 
> Signed-off-by: Pierre Morel 
> ---

Great ! It works !

Since we end up with 3 blocks of code that are identical save the ring
region name, we can simplify further with a macro. I have pasted a
patch below. You may fold it directly into your patch if people agree,

In any case, you have:

Acked-by: Greg Kurz 
Tested-by: Greg Kurz 

Merci Pierre !

--
Greg

>  hw/virtio/dataplane/vring.c |   54 --
>  include/hw/virtio/dataplane/vring.h |4 ++-
>  2 files changed, 47 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/virtio/dataplane/vring.c b/hw/virtio/dataplane/vring.c
> index 07fd69c..49d783b 100644
> --- a/hw/virtio/dataplane/vring.c
> +++ b/hw/virtio/dataplane/vring.c
> @@ -67,22 +67,46 @@ static void vring_unmap(void *buffer, bool is_write)
>  /* Map the guest's vring to host memory */
>  bool vring_setup(Vring *vring, VirtIODevice *vdev, int n)
>  {
> -hwaddr vring_addr = virtio_queue_get_ring_addr(vdev, n);
> -hwaddr vring_size = virtio_queue_get_ring_size(vdev, n);
> +hwaddr vring_addr;
> +hwaddr vring_size;
>  void *vring_ptr;
> +struct vring *vr = &vring->vr;
> 
>  vring->broken = false;
> +vr->num = virtio_queue_get_num(vdev, n);
> 
> -vring_ptr = vring_map(&vring->mr, vring_addr, vring_size, true);
> +vring_addr = virtio_queue_get_desc_addr(vdev, n);
> +vring_size = virtio_queue_get_desc_size(vdev, n);
> +vring_ptr = vring_map(&vring->mr_desc, vring_addr, vring_size, true);
>  if (!vring_ptr) {
> -error_report("Failed to map vring "
> - "addr %#" HWADDR_PRIx " size %" HWADDR_PRIu,
> - vring_addr, vring_size);
> -vring->broken = true;
> -return false;
> +error_report("Failed to map vring desc "
> + "addr %#" HWADDR_PRIx " size %" HWADDR_PRIu,
> + vring_addr, vring_size);
> +goto out_err_desc;
>  }
> +vr->desc = vring_ptr;
> 
> -vring_init(&vring->vr, virtio_queue_get_num(vdev, n), vring_ptr, 4096);
> +vring_addr = virtio_queue_get_avail_addr(vdev, n);
> +vring_size = virtio_queue_get_avail_size(vdev, n);
> +vring_ptr = vring_map(&vring->mr_avail, vring_addr, vring_size, true);
> +if (!vring_ptr) {
> +error_report("Failed to map vring avail "
> + "addr %#" HWADDR_PRIx " size %" HWADDR_PRIu,
> + vring_addr, vring_size);
> +goto out_err_avail;
> +}
> +vr->avail = vring_ptr;
> +
> +vring_addr = virtio_queue_get_used_addr(vdev, n);
> +vring_size = virtio_queue_get_used_size(vdev, n);
> +vring_ptr = vring_map(&vring->mr_used, vring_addr, vring_size, true);
> +if (!vring_ptr) {
> +error_report("Failed to map vring used "
> + "addr %#" HWADDR_PRIx " size %" HWADDR_PRIu,
> + vring_addr, vring_size);
> +goto out_err_used;
> +}
> +vr->used = vring_ptr;
> 
>  vring->last_avail_idx = virtio_queue_get_last_avail_idx(vdev, n);
>  vring->last_used_idx = vring_get_used_idx(vdev, vring);
> @@ -92,6 +116,14 @@ bool vring_setup(Vring *vring, VirtIODevice *vdev, int n)
>  trace_vring_setup(virtio_queue_get_ring_addr(vdev, n),
>vring->vr.desc, vring->vr.avail, vring->vr.used);
>  return true;
> +
> +out_err_used:
> +memory_region_unref(vring->mr_avail);
> +out_err_avail:
> +memory_region_unref(vring->mr_desc);
> +out_err_desc:
> +vring->broken = true;
> +return false;
>  }
> 
>  void vring_teardown(Vring *vring, VirtIODevice *vdev, int n)
> @@ -99,7 +131,9 @@ void vring_teardown(Vring *vring, VirtIODevice *vdev, int 
> n)
>  virtio_queue_set_last_avail_idx(vdev, n, vring->last_avail_idx);
>  virtio_queue_invalidate_signalled_used(vdev, n);
> 
> -memory_region_unref(vring->mr);
> +memory_region_unref(vring->mr_desc);
> +memory_region_unref(vring->mr_avail);
> +memory_region_unref(vring->mr_used);
>  }
> 
>  /* Disable guest->host notifies */
> diff --git a/include/hw/virtio/dataplane/vring.h 
> b/include/hw/virtio/dataplane/vring.h
> index 8d97db9..a596e4c 100644
> --- a/include/hw/virtio/dataplane/vring.h
> +++ b/include/hw/virtio/dataplane/vring.h
> @@ -22,7 +22,9 @@
>  #include "hw/virtio/virtio.h"
> 
>  typedef struct {
> -MemoryRegion *mr;   /* memory region containing the vring */
> +MemoryRegion *mr_desc;  /* memory region for the vring desc */
> +MemoryRegion *mr_avail; /* memory region for the vring avail */
> +MemoryRegion *mr_used;  /* memory region for the vring used */
>  struct vring vr;/* virtqueue vring mapped to host memory 
> */
>  uint16_t last_avail_idx;/* last processed avail ring index */
>  uint16_t last_used_idx; /* last pro

Re: [Qemu-devel] [PATCH] Add another sanity check to smp_parse() function

[Cornelia Huck]

On Tue, 25 Aug 2015 15:25:00 +0200
Thomas Huth  wrote:

> On 19/08/15 17:58, Eduardo Habkost wrote:
> > On Wed, Jul 22, 2015 at 03:59:50PM +0200, Thomas Huth wrote:
> >> The code in smp_parse already checks the topology information for
> >> sockets * cores * threads < cpus and bails out with an error in
> >> that case. However, it is still possible to supply a bad configuration
> >> the other way round, e.g. with:
> >>
> >>  qemu-system-xxx -smp 4,sockets=1,cores=4,threads=2
> >>
> >> QEMU then still starts the guest, with topology configuration that
> >> is rather incomprehensible and likely not what the user wanted.
> >> So let's add another check to refuse such wrong configurations.
> >>
> >> Signed-off-by: Thomas Huth 
> >> ---
> >>  vl.c | 8 +++-
> >>  1 file changed, 7 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/vl.c b/vl.c
> >> index 5856396..c8d24b1 100644
> >> --- a/vl.c
> >> +++ b/vl.c
> >> @@ -1224,7 +1224,13 @@ static void smp_parse(QemuOpts *opts)
> >>  exit(1);
> >>  }
> >>  
> >> -max_cpus = qemu_opt_get_number(opts, "maxcpus", 0);
> >> +max_cpus = qemu_opt_get_number(opts, "maxcpus", cpus);
> >> +if (sockets * cores * threads > max_cpus) {
> >> +fprintf(stderr, "cpu topology: error: "
> >> +"sockets (%u) * cores (%u) * threads (%u) > maxcpus 
> >> (%u)\n",
> >> +sockets, cores, threads, max_cpus);
> >> +exit(1);
> >> +}
> > 
> > I am always afraid of breaking existing setups when we do that, because
> > there may be existing VMs running with these weird configurations, and
> > people won't be able to live-migrate them to a newer QEMU.
> > 
> > But I think we really have to start refusing to run obviously broken
> > configurations one day, or we will never fix this mess, so:
> > 
> > Reviewed-by: Eduardo Habkost 
> > 
> > I want to apply this through the x86 tree, but I would like to get some
> > Acked-by from other maintainers first.
> 
> Ok, thanks!
> 
> So *ping* to the other CPU core maintainers here ... could I get some
> more ACKs, please?

Looks reasonable.

Acked-by: Cornelia Huck 

Re: [Qemu-devel] [PATCH] virtio dataplane: adapt dataplane for virtio Version 1

[Pierre Morel]

On 08/26/2015 01:29 PM, Greg Kurz wrote:

On Tue, 25 Aug 2015 12:33:30 +0200
Pierre Morel  wrote:


Let dataplane allocate different region for the desc/avail/used
ring regions.

Signed-off-by: Pierre Morel 
---

Great ! It works !

Since we end up with 3 blocks of code that are identical save the ring
region name, we can simplify further with a macro. I have pasted a
patch below. You may fold it directly into your patch if people agree,

In any case, you have:

Acked-by: Greg Kurz 
Tested-by: Greg Kurz 

Merci Pierre !

--
Greg

It saves a lot of LOCs, thanks, (and also for review and test)
I apply the patch and send a v2

Pierre


  hw/virtio/dataplane/vring.c |   54 --
  include/hw/virtio/dataplane/vring.h |4 ++-
  2 files changed, 47 insertions(+), 11 deletions(-)

diff --git a/hw/virtio/dataplane/vring.c b/hw/virtio/dataplane/vring.c
index 07fd69c..49d783b 100644
--- a/hw/virtio/dataplane/vring.c
+++ b/hw/virtio/dataplane/vring.c
@@ -67,22 +67,46 @@ static void vring_unmap(void *buffer, bool is_write)
  /* Map the guest's vring to host memory */
  bool vring_setup(Vring *vring, VirtIODevice *vdev, int n)
  {
-hwaddr vring_addr = virtio_queue_get_ring_addr(vdev, n);
-hwaddr vring_size = virtio_queue_get_ring_size(vdev, n);
+hwaddr vring_addr;
+hwaddr vring_size;
  void *vring_ptr;
+struct vring *vr = &vring->vr;

  vring->broken = false;
+vr->num = virtio_queue_get_num(vdev, n);

-vring_ptr = vring_map(&vring->mr, vring_addr, vring_size, true);
+vring_addr = virtio_queue_get_desc_addr(vdev, n);
+vring_size = virtio_queue_get_desc_size(vdev, n);
+vring_ptr = vring_map(&vring->mr_desc, vring_addr, vring_size, true);
  if (!vring_ptr) {
-error_report("Failed to map vring "
- "addr %#" HWADDR_PRIx " size %" HWADDR_PRIu,
- vring_addr, vring_size);
-vring->broken = true;
-return false;
+error_report("Failed to map vring desc "
+ "addr %#" HWADDR_PRIx " size %" HWADDR_PRIu,
+ vring_addr, vring_size);
+goto out_err_desc;
  }
+vr->desc = vring_ptr;

-vring_init(&vring->vr, virtio_queue_get_num(vdev, n), vring_ptr, 4096);
+vring_addr = virtio_queue_get_avail_addr(vdev, n);
+vring_size = virtio_queue_get_avail_size(vdev, n);
+vring_ptr = vring_map(&vring->mr_avail, vring_addr, vring_size, true);
+if (!vring_ptr) {
+error_report("Failed to map vring avail "
+ "addr %#" HWADDR_PRIx " size %" HWADDR_PRIu,
+ vring_addr, vring_size);
+goto out_err_avail;
+}
+vr->avail = vring_ptr;
+
+vring_addr = virtio_queue_get_used_addr(vdev, n);
+vring_size = virtio_queue_get_used_size(vdev, n);
+vring_ptr = vring_map(&vring->mr_used, vring_addr, vring_size, true);
+if (!vring_ptr) {
+error_report("Failed to map vring used "
+ "addr %#" HWADDR_PRIx " size %" HWADDR_PRIu,
+ vring_addr, vring_size);
+goto out_err_used;
+}
+vr->used = vring_ptr;

  vring->last_avail_idx = virtio_queue_get_last_avail_idx(vdev, n);
  vring->last_used_idx = vring_get_used_idx(vdev, vring);
@@ -92,6 +116,14 @@ bool vring_setup(Vring *vring, VirtIODevice *vdev, int n)
  trace_vring_setup(virtio_queue_get_ring_addr(vdev, n),
vring->vr.desc, vring->vr.avail, vring->vr.used);
  return true;
+
+out_err_used:
+memory_region_unref(vring->mr_avail);
+out_err_avail:
+memory_region_unref(vring->mr_desc);
+out_err_desc:
+vring->broken = true;
+return false;
  }

  void vring_teardown(Vring *vring, VirtIODevice *vdev, int n)
@@ -99,7 +131,9 @@ void vring_teardown(Vring *vring, VirtIODevice *vdev, int n)
  virtio_queue_set_last_avail_idx(vdev, n, vring->last_avail_idx);
  virtio_queue_invalidate_signalled_used(vdev, n);

-memory_region_unref(vring->mr);
+memory_region_unref(vring->mr_desc);
+memory_region_unref(vring->mr_avail);
+memory_region_unref(vring->mr_used);
  }

  /* Disable guest->host notifies */
diff --git a/include/hw/virtio/dataplane/vring.h 
b/include/hw/virtio/dataplane/vring.h
index 8d97db9..a596e4c 100644
--- a/include/hw/virtio/dataplane/vring.h
+++ b/include/hw/virtio/dataplane/vring.h
@@ -22,7 +22,9 @@
  #include "hw/virtio/virtio.h"

  typedef struct {
-MemoryRegion *mr;   /* memory region containing the vring */
+MemoryRegion *mr_desc;  /* memory region for the vring desc */
+MemoryRegion *mr_avail; /* memory region for the vring avail */
+MemoryRegion *mr_used;  /* memory region for the vring used */
  struct vring vr;/* virtqueue vring mapped to host memory 
*/
  uint16_t last_avail_idx;/* last processed avail ring index */
  uint16_t last_used_idx; /* last processed used ring index */

--- a/hw/virtio/datapla

Re: [Qemu-devel] [PATCH] target-mips: remove wrong checks for recip.fmt and rsqrt.fmt

[Leon Alrae]

On 25/08/2015 23:40, Petar Jovanovic wrote:
>> @@ -9839,7 +9837,6 @@ static void gen_farith (DisasContext *ctx, enum
> fopcode op1,
>>  opn = "movn.d";
>>  break;
>>  case OPC_RECIP_D:
>> -check_cp1_64bitmode(ctx);
> 
>> I think this needs check_cp1_registers() now, i.e. check for odd fpu
> register access when Status.FR = 0.
> 
> This would raise a "reserved instruction" exception. I am not aware that any
> MIPS CPU implementation would throw an exception for e.g. "recip.d
> $f21,$f11" (let me know if that is not the case), and I do not think MIPS
> documentation obliges us to throw an exception either.

MIPS documentation says that this operation is "UNPREDICTABLE" -- software can
never depend on a result and in QEMU we usually raise RI in such cases in
other *.D instructions which is quite handy (it usually indicates the "forgot
to set Status.FR bit" bug in the guest).

Leon

Re: [Qemu-devel] [PATCH 7/7] maint: avoid useless "if (foo) free(foo)" pattern

[Markus Armbruster]

"Daniel P. Berrange"  writes:

> The free() and g_free() functions both happily accept
> NULL on any platform QEMU builds on.

Do systems where free(NULL) doesn't work even exist?  Even C89
guarantees it does nothing.

>  As such putting a
> conditional 'if (foo)' check before calls to 'free(foo)'
> merely serves to bloat the lines of code.
>
> Signed-off-by: Daniel P. Berrange 

My Coccinelle semantic patch finds a few more, because it also fixes up
the equally pointless conditional

if (foo) {
free(foo);
foo = NULL;
}

@@
expression E;
@@
-   if (E != NULL) {
-   g_free(E);
-   }
+   g_free(E);
@@
expression E;
@@
-   if (E != NULL) {
-   g_free(E);
-   E = NULL;
-   }
+   g_free(E);
+   E = NULL;
@@
expression E;
type T;
@@
-   if (E != NULL) {
-   g_free((T)E);
-   }
+   g_free((T)E);
@@
expression E;
type T;
@@
-   if (E != NULL) {
-   g_free((T)E);
-   E = NULL;
-   }
+   g_free((T)E);
+   E = NULL;

Result (feel free to squash it into your patch):

diff --git a/hw/char/exynos4210_uart.c b/hw/char/exynos4210_uart.c
index 7614e58..215f962 100644
--- a/hw/char/exynos4210_uart.c
+++ b/hw/char/exynos4210_uart.c
@@ -234,10 +234,8 @@ static int fifo_empty_elements_number(Exynos4210UartFIFO 
*q)
 
 static void fifo_reset(Exynos4210UartFIFO *q)
 {
-if (q->data != NULL) {
-g_free(q->data);
-q->data = NULL;
-}
+g_free(q->data);
+q->data = NULL;
 
 q->data = (uint8_t *)g_malloc0(q->size);
 
diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
index 78ae4cd..5742a24 100644
--- a/hw/net/rtl8139.c
+++ b/hw/net/rtl8139.c
@@ -3438,10 +3438,8 @@ static void pci_rtl8139_uninit(PCIDevice *dev)
 {
 RTL8139State *s = RTL8139(dev);
 
-if (s->cplus_txbuffer) {
-g_free(s->cplus_txbuffer);
-s->cplus_txbuffer = NULL;
-}
+g_free(s->cplus_txbuffer);
+s->cplus_txbuffer = NULL;
 timer_del(s->timer);
 timer_free(s->timer);
 qemu_del_nic(s->nic);
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index a8f79d8..a2feb4c 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1460,10 +1460,8 @@ static void spapr_pci_pre_save(void *opaque)
 gpointer key, value;
 int i;
 
-if (sphb->msi_devs) {
-g_free(sphb->msi_devs);
-sphb->msi_devs = NULL;
-}
+g_free(sphb->msi_devs);
+sphb->msi_devs = NULL;
 sphb->msi_devs_num = g_hash_table_size(sphb->msi);
 if (!sphb->msi_devs_num) {
 return;
@@ -1490,10 +1488,8 @@ static int spapr_pci_post_load(void *opaque, int 
version_id)
  sizeof(sphb->msi_devs[i].value));
 g_hash_table_insert(sphb->msi, key, value);
 }
-if (sphb->msi_devs) {
-g_free(sphb->msi_devs);
-sphb->msi_devs = NULL;
-}
+g_free(sphb->msi_devs);
+sphb->msi_devs = NULL;
 sphb->msi_devs_num = 0;
 
 return 0;
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index e63367b..65304cf 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1169,10 +1169,8 @@ static void sdhci_uninitfn(SDHCIState *s)
 qemu_free_irq(s->eject_cb);
 qemu_free_irq(s->ro_cb);
 
-if (s->fifo_buffer) {
-g_free(s->fifo_buffer);
-s->fifo_buffer = NULL;
-}
+g_free(s->fifo_buffer);
+s->fifo_buffer = NULL;
 }
 
 const VMStateDescription sdhci_vmstate = {
diff --git a/hw/usb/hcd-ehci-pci.c b/hw/usb/hcd-ehci-pci.c
index 7afa5f9..16fb845 100644
--- a/hw/usb/hcd-ehci-pci.c
+++ b/hw/usb/hcd-ehci-pci.c
@@ -95,10 +95,8 @@ static void usb_ehci_pci_exit(PCIDevice *dev)
 
 usb_ehci_unrealize(s, DEVICE(dev), NULL);
 
-if (s->irq) {
-g_free(s->irq);
-s->irq = NULL;
-}
+g_free(s->irq);
+s->irq = NULL;
 }
 
 static void usb_ehci_pci_reset(DeviceState *dev)
diff --git a/tests/test-hbitmap.c b/tests/test-hbitmap.c
index 161eeb4..abcea0c 100644
--- a/tests/test-hbitmap.c
+++ b/tests/test-hbitmap.c
@@ -139,10 +139,8 @@ static void hbitmap_test_teardown(TestHBitmapData *data,
 hbitmap_free(data->hb);
 data->hb = NULL;
 }
-if (data->bits) {
-g_free(data->bits);
-data->bits = NULL;
-}
+g_free(data->bits);
+data->bits = NULL;
 }
 
 /* Set a range in the HBitmap and in the shadow "simple" bitmap.
diff --git a/translate-all.c b/translate-all.c
index 9c46ffa..ffe75ed 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -731,10 +731,8 @@ void tb_free(TranslationBlock *tb)
 
 static inline void invalidate_page_bitmap(PageDesc *p)
 {
-if (p->code_bitmap) {
-g_free(p->code_bitmap);
-p->code_bitmap = NULL;
-}
+g_free(p->code_bitmap);
+p->code_bitmap = NULL;
 p->code_write_count = 0;
 }
 
diff --git a/vl.c b/vl.c
index 584ca88..72a3f81 100644
--- a/vl.c
+++ b/vl.c
@@ -534,10 +534,8 @@ const char *qemu_get_vm_name(void)
 
 static void res_free(void)
 {
-if (boot_splash_filed

Re: [Qemu-devel] [PATCH 0/7] Misc trivial code cleanups

[Markus Armbruster]

"Daniel P. Berrange"  writes:

> This is a series of misc trivial code cleanups that I
> originally used to illustrate the use of GNULIB's
> syntax-check facility:
>
>   http://lists.nongnu.org/archive/html/qemu-devel/2015-07/msg06268.html
>
> Since it appears the license of the GNULIB syntax-check
> is problematic, this series has separated the fixes out
> from the GNULIB additions, to allow for possibility of
> merging them as-is.

Series
Reviewed-by: Markus Armbruster 

[Qemu-devel] [PATCH RFC 2/7] hostmem: register properties against the class instead of object

[Daniel P. Berrange]

This converts the hostmem and hostmem-file objects to register
their properties against the class rather than object.

Signed-off-by: Daniel P. Berrange 
---
 backends/hostmem-file.c | 26 +++---
 backends/hostmem.c  | 41 +
 2 files changed, 32 insertions(+), 35 deletions(-)

diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
index 4b55361..4e528d9 100644
--- a/backends/hostmem-file.c
+++ b/backends/hostmem-file.c
@@ -59,14 +59,6 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error 
**errp)
 #endif
 }
 
-static void
-file_backend_class_init(ObjectClass *oc, void *data)
-{
-HostMemoryBackendClass *bc = MEMORY_BACKEND_CLASS(oc);
-
-bc->alloc = file_backend_memory_alloc;
-}
-
 static char *get_mem_path(Object *o, Error **errp)
 {
 HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(o);
@@ -108,21 +100,25 @@ static void file_memory_backend_set_share(Object *o, bool 
value, Error **errp)
 fb->share = value;
 }
 
+
 static void
-file_backend_instance_init(Object *o)
+file_backend_class_init(ObjectClass *oc, void *data)
 {
-object_property_add_bool(o, "share",
-file_memory_backend_get_share,
-file_memory_backend_set_share, NULL);
-object_property_add_str(o, "mem-path", get_mem_path,
-set_mem_path, NULL);
+HostMemoryBackendClass *bc = MEMORY_BACKEND_CLASS(oc);
+
+bc->alloc = file_backend_memory_alloc;
+
+object_class_property_add_bool(oc, "share",
+   file_memory_backend_get_share,
+   file_memory_backend_set_share, NULL);
+object_class_property_add_str(oc, "mem-path", get_mem_path,
+  set_mem_path, NULL);
 }
 
 static const TypeInfo file_backend_info = {
 .name = TYPE_MEMORY_BACKEND_FILE,
 .parent = TYPE_MEMORY_BACKEND,
 .class_init = file_backend_class_init,
-.instance_init = file_backend_instance_init,
 .instance_size = sizeof(HostMemoryBackendFile),
 };
 
diff --git a/backends/hostmem.c b/backends/hostmem.c
index 41ba2af..ca5448b 100644
--- a/backends/hostmem.c
+++ b/backends/hostmem.c
@@ -228,26 +228,6 @@ static void host_memory_backend_init(Object *obj)
 backend->merge = machine_mem_merge(machine);
 backend->dump = machine_dump_guest_core(machine);
 backend->prealloc = mem_prealloc;
-
-object_property_add_bool(obj, "merge",
-host_memory_backend_get_merge,
-host_memory_backend_set_merge, NULL);
-object_property_add_bool(obj, "dump",
-host_memory_backend_get_dump,
-host_memory_backend_set_dump, NULL);
-object_property_add_bool(obj, "prealloc",
-host_memory_backend_get_prealloc,
-host_memory_backend_set_prealloc, NULL);
-object_property_add(obj, "size", "int",
-host_memory_backend_get_size,
-host_memory_backend_set_size, NULL, NULL, NULL);
-object_property_add(obj, "host-nodes", "int",
-host_memory_backend_get_host_nodes,
-host_memory_backend_set_host_nodes, NULL, NULL, NULL);
-object_property_add_enum(obj, "policy", "HostMemPolicy",
- HostMemPolicy_lookup,
- host_memory_backend_get_policy,
- host_memory_backend_set_policy, NULL);
 }
 
 MemoryRegion *
@@ -348,6 +328,27 @@ host_memory_backend_class_init(ObjectClass *oc, void *data)
 
 ucc->complete = host_memory_backend_memory_complete;
 ucc->can_be_deleted = host_memory_backend_can_be_deleted;
+
+object_class_property_add_bool(oc, "merge",
+   host_memory_backend_get_merge,
+   host_memory_backend_set_merge, NULL);
+object_class_property_add_bool(oc, "dump",
+   host_memory_backend_get_dump,
+   host_memory_backend_set_dump, NULL);
+object_class_property_add_bool(oc, "prealloc",
+   host_memory_backend_get_prealloc,
+   host_memory_backend_set_prealloc, NULL);
+object_class_property_add(oc, "size", "int",
+  host_memory_backend_get_size,
+  host_memory_backend_set_size, NULL, NULL, NULL);
+object_class_property_add(oc, "host-nodes", "int",
+  host_memory_backend_get_host_nodes,
+  host_memory_backend_set_host_nodes,
+  NULL, NULL, NULL);
+object_class_property_add_enum(oc, "policy", "HostMemPolicy",
+   HostMemPolicy_lookup,
+   host_memory_backend_get_policy,
+

[Qemu-devel] [PATCH RFC 0/7] Making QOM introspectable

[Daniel P. Berrange]

There are many problems in QEMU related to introspection
of features, which Markus has been attacking/slaying for
a while. One of the remaining elephants in the corner of
the room which I've not seen work on is QOM.

QOM has a nice class/object framework, kind of like GLib's
GObject, but with the difference that properties are
registered against objects rather than classes. IIRC the
rationale for this is that in some cases the properties
registered are going to be dynamically decided at runtime,
so it isn't possible to statically define them against
classes. Now this may well be true of some properties,
but there are equally plenty of cases where the properties
/are/ invariant and could be registered against classes.

There are two core problems with registering properties
against object instances:

 - It is wasteful of memory to duplicate the allocation
   of ObjectProperty structs against each object
   instance. When you have N object instances, you
   have O(N) memory usage, instead of O(1). This is not
   a problem for objects which are mostly singletons,
   but there are cases in QEMU where you instantiate
   many instances of the same class and/or have many
   properties.

 - It prevents static introspection. Since the property
   is only registered in the object initializer, there
   is no way to programmatically query what properties
   an object supports without first instantiating it.
   Taking machine types as an example, if you want to
   introspect every machine type supported by a QEMU
   binary you thus have to launch QEMU many times,
   passing a different -M argument each time. As the
   number of different machine types & objects
   increases this quickly becomes impractical.

This series thus extends QOM to make it possible to register
properties against the classes, in addition to against object
instances. When looking up a property, a search will be done
starting at the base class, then each subclass in turn, and
finally against the object. Names are enforced to be unique
across the parent classes for sanity.

This only currently supports simple scalar properties where
the actual property value storage is managed by the object
instance. The object child and object link properties use
implicit storage in the ObjectProperty struct's 'opaque'
field, so we can't allow those against the class. Solving
this is doable, but more work, so is left as an exercise
for the future.

The first patch adds the neccessary QOM functionality. The
following 6 patches then illustrate the fairly trivial
conversions of a bunch of objects.

The eventual goal ought to be that everything is registered
against the class, leaving only the (hopefully few) cases
where per-instance properties are truely needed unconverted.

This series doesn't attempt to implement introspection
either - this would require a new QMP command such as
'qom-list-type-props' to query the properties against
classes.

I'm not really planning to spend much more time on this
myself. I'm just using this series to illustrate how I
believe the introspection problem in QOM can be fairly
easily addressed. Hopefully this will stimulate some
discussion & interest in doing the full job

Daniel P. Berrange (7):
  qom: allow properties to be registered against classes
  hostmem: register properties against the class instead of object
  rng: register properties against the class instead of object
  tpm: register properties against the class instead of object
  cpu: avoid using object instance state in property getter
  x86-cpu: register properties against the class instead of object
  machine: register properties against the class instead of object

 backends/hostmem-file.c |  26 +++---
 backends/hostmem.c  |  41 -
 backends/rng-egd.c  |  12 +--
 backends/rng-random.c   |  10 +--
 backends/rng.c  |  14 ++-
 backends/tpm.c  |  12 +--
 hw/core/machine.c   | 193 +++
 include/qom/object.h|  44 +
 qom/object.c| 233 ++--
 target-i386/cpu.c   |  88 +++---
 10 files changed, 476 insertions(+), 197 deletions(-)

-- 
2.4.3

[Qemu-devel] [PATCH RFC 1/7] qom: allow properties to be registered against classes

[Daniel P. Berrange]

When there are many instances of a given class, registering
properties against the instance is wasteful of resources. The
majority of objects have a statically defined list of possible
properties, so most of the properties are easily registerable
against the class. Only those properties which are conditionally
registered at runtime need be recorded against the klass.

Registering properties against classes also makes it possible
to provide static introspection of QOM - currently introspection
is only possible after creating an instance of a class, which
severely limits its usefulness.

This impl only supports simple scalar properties. It does not
attempt to allow child object / link object properties against
the class. There are ways to support those too, but it would
make this patch more complicated, so it is left as an exercise
for the future.

Signed-off-by: Daniel P. Berrange 
---
 include/qom/object.h |  44 ++
 qom/object.c | 233 +--
 2 files changed, 270 insertions(+), 7 deletions(-)

diff --git a/include/qom/object.h b/include/qom/object.h
index 807978e..068162e 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -383,6 +383,8 @@ struct ObjectClass
 const char *class_cast_cache[OBJECT_CLASS_CAST_CACHE];
 
 ObjectUnparent *unparent;
+
+QTAILQ_HEAD(, ObjectProperty) properties;
 };
 
 /**
@@ -949,6 +951,13 @@ ObjectProperty *object_property_add(Object *obj, const 
char *name,
 
 void object_property_del(Object *obj, const char *name, Error **errp);
 
+ObjectProperty *object_class_property_add(ObjectClass *klass, const char *name,
+  const char *type,
+  ObjectPropertyAccessor *get,
+  ObjectPropertyAccessor *set,
+  ObjectPropertyRelease *release,
+  void *opaque, Error **errp);
+
 /**
  * object_property_find:
  * @obj: the object
@@ -959,6 +968,8 @@ void object_property_del(Object *obj, const char *name, 
Error **errp);
  */
 ObjectProperty *object_property_find(Object *obj, const char *name,
  Error **errp);
+ObjectProperty *object_class_property_find(ObjectClass *klass, const char 
*name,
+   Error **errp);
 
 void object_unparent(Object *obj);
 
@@ -1327,6 +1338,12 @@ void object_property_add_str(Object *obj, const char 
*name,
  void (*set)(Object *, const char *, Error **),
  Error **errp);
 
+void object_class_property_add_str(ObjectClass *klass, const char *name,
+   char *(*get)(Object *, Error **),
+   void (*set)(Object *, const char *,
+   Error **),
+   Error **errp);
+
 /**
  * object_property_add_bool:
  * @obj: the object to add a property to
@@ -1343,6 +1360,11 @@ void object_property_add_bool(Object *obj, const char 
*name,
   void (*set)(Object *, bool, Error **),
   Error **errp);
 
+void object_class_property_add_bool(ObjectClass *klass, const char *name,
+bool (*get)(Object *, Error **),
+void (*set)(Object *, bool, Error **),
+Error **errp);
+
 /**
  * object_property_add_enum:
  * @obj: the object to add a property to
@@ -1362,6 +1384,13 @@ void object_property_add_enum(Object *obj, const char 
*name,
   void (*set)(Object *, int, Error **),
   Error **errp);
 
+void object_class_property_add_enum(ObjectClass *klass, const char *name,
+const char *typename,
+const char * const *strings,
+int (*get)(Object *, Error **),
+void (*set)(Object *, int, Error **),
+Error **errp);
+
 /**
  * object_property_add_tm:
  * @obj: the object to add a property to
@@ -1376,6 +1405,10 @@ void object_property_add_tm(Object *obj, const char 
*name,
 void (*get)(Object *, struct tm *, Error **),
 Error **errp);
 
+void object_class_property_add_tm(ObjectClass *klass, const char *name,
+  void (*get)(Object *, struct tm *, Error **),
+  Error **errp);
+
 /**
  * object_property_add_uint8_ptr:
  * @obj: the object to add a property to
@@ -1388,6 +1421,8 @@ void object_property_add_tm(Object *obj, const char *name,
  */
 void object_property_add_uint8_ptr(Object *obj, const char *name,
const uint8_t *v, E

[Qemu-devel] [PATCH RFC 4/7] tpm: register properties against the class instead of object

[Daniel P. Berrange]

This converts the tpm object to register its properties against
the class rather than object.

Signed-off-by: Daniel P. Berrange 
---
 backends/tpm.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/backends/tpm.c b/backends/tpm.c
index a512693..cb540b9 100644
--- a/backends/tpm.c
+++ b/backends/tpm.c
@@ -148,12 +148,12 @@ static void tpm_backend_prop_set_opened(Object *obj, bool 
value, Error **errp)
 s->opened = true;
 }
 
-static void tpm_backend_instance_init(Object *obj)
+static void tpm_backend_class_init(ObjectClass *klass, void *data)
 {
-object_property_add_bool(obj, "opened",
- tpm_backend_prop_get_opened,
- tpm_backend_prop_set_opened,
- NULL);
+object_class_property_add_bool(klass, "opened",
+   tpm_backend_prop_get_opened,
+   tpm_backend_prop_set_opened,
+   NULL);
 }
 
 void tpm_backend_thread_deliver_request(TPMBackendThread *tbt)
@@ -183,7 +183,7 @@ static const TypeInfo tpm_backend_info = {
 .name = TYPE_TPM_BACKEND,
 .parent = TYPE_OBJECT,
 .instance_size = sizeof(TPMBackend),
-.instance_init = tpm_backend_instance_init,
+.class_init = tpm_backend_class_init,
 .class_size = sizeof(TPMBackendClass),
 .abstract = true,
 };
-- 
2.4.3

[Qemu-devel] [PATCH RFC 7/7] machine: register properties against the class instead of object

[Daniel P. Berrange]

This converts the machine base object to register its properties
against the class rather than object.

Signed-off-by: Daniel P. Berrange 
---
 hw/core/machine.c | 193 +++---
 1 file changed, 97 insertions(+), 96 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index ac4654e..0fd80d3 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -300,6 +300,103 @@ static void machine_class_init(ObjectClass *oc, void 
*data)
 
 /* Default 128 MB as guest ram size */
 mc->default_ram_size = 128 * M_BYTE;
+
+object_class_property_add_str(oc, "accel",
+  machine_get_accel, machine_set_accel, NULL);
+object_class_property_set_description(oc, "accel",
+  "Accelerator list",
+  NULL);
+object_class_property_add_bool(oc, "kernel-irqchip",
+   NULL,
+   machine_set_kernel_irqchip,
+   NULL);
+object_class_property_set_description(oc, "kernel-irqchip",
+  "Use KVM in-kernel irqchip",
+  NULL);
+object_class_property_add(oc, "kvm-shadow-mem", "int",
+  machine_get_kvm_shadow_mem,
+  machine_set_kvm_shadow_mem,
+  NULL, NULL, NULL);
+object_class_property_set_description(oc, "kvm-shadow-mem",
+  "KVM shadow MMU size",
+  NULL);
+object_class_property_add_str(oc, "kernel",
+  machine_get_kernel, machine_set_kernel, 
NULL);
+object_class_property_set_description(oc, "kernel",
+  "Linux kernel image file",
+  NULL);
+object_class_property_add_str(oc, "initrd",
+  machine_get_initrd, machine_set_initrd, 
NULL);
+object_class_property_set_description(oc, "initrd",
+  "Linux initial ramdisk file",
+  NULL);
+object_class_property_add_str(oc, "append",
+  machine_get_append, machine_set_append, 
NULL);
+object_class_property_set_description(oc, "append",
+  "Linux kernel command line",
+  NULL);
+object_class_property_add_str(oc, "dtb",
+  machine_get_dtb, machine_set_dtb, NULL);
+object_class_property_set_description(oc, "dtb",
+  "Linux kernel device tree file",
+  NULL);
+object_class_property_add_str(oc, "dumpdtb",
+  machine_get_dumpdtb,
+  machine_set_dumpdtb, NULL);
+object_class_property_set_description(oc, "dumpdtb",
+  "Dump current dtb to a file and 
quit",
+  NULL);
+object_class_property_add(oc, "phandle-start", "int",
+  machine_get_phandle_start,
+  machine_set_phandle_start,
+  NULL, NULL, NULL);
+object_class_property_set_description(oc, "phandle-start",
+  "The first phandle ID we may 
generate dynamically",
+  NULL);
+object_class_property_add_str(oc, "dt-compatible",
+  machine_get_dt_compatible,
+  machine_set_dt_compatible,
+  NULL);
+object_class_property_set_description(oc, "dt-compatible",
+  "Overrides the \"compatible\" 
property of the dt root node",
+  NULL);
+object_class_property_add_bool(oc, "dump-guest-core",
+   machine_get_dump_guest_core,
+   machine_set_dump_guest_core,
+   NULL);
+object_class_property_set_description(oc, "dump-guest-core",
+  "Include guest memory in  a core 
dump",
+  NULL);
+object_class_property_add_bool(oc, "mem-merge",
+   machine_get_mem_merge,
+   machine_set_mem_merge, NULL);
+object_class_property_set_description(oc, "mem-merge",
+  "Enable/disable memory merge 
support",
+  NULL);
+object_class_property_add_bool(oc, "usb",
+ 

[Qemu-devel] [PATCH RFC 3/7] rng: register properties against the class instead of object

[Daniel P. Berrange]

This converts the rng, rng-egd & rng-random objects to register
their properties against the class rather than object.

Signed-off-by: Daniel P. Berrange 
---
 backends/rng-egd.c| 12 
 backends/rng-random.c | 10 +-
 backends/rng.c| 14 +-
 3 files changed, 14 insertions(+), 22 deletions(-)

diff --git a/backends/rng-egd.c b/backends/rng-egd.c
index 6c13409..8b2a9dc 100644
--- a/backends/rng-egd.c
+++ b/backends/rng-egd.c
@@ -186,13 +186,6 @@ static char *rng_egd_get_chardev(Object *obj, Error **errp)
 return NULL;
 }
 
-static void rng_egd_init(Object *obj)
-{
-object_property_add_str(obj, "chardev",
-rng_egd_get_chardev, rng_egd_set_chardev,
-NULL);
-}
-
 static void rng_egd_finalize(Object *obj)
 {
 RngEgd *s = RNG_EGD(obj);
@@ -214,6 +207,10 @@ static void rng_egd_class_init(ObjectClass *klass, void 
*data)
 rbc->request_entropy = rng_egd_request_entropy;
 rbc->cancel_requests = rng_egd_cancel_requests;
 rbc->opened = rng_egd_opened;
+
+object_class_property_add_str(klass, "chardev",
+  rng_egd_get_chardev, rng_egd_set_chardev,
+  NULL);
 }
 
 static const TypeInfo rng_egd_info = {
@@ -221,7 +218,6 @@ static const TypeInfo rng_egd_info = {
 .parent = TYPE_RNG_BACKEND,
 .instance_size = sizeof(RngEgd),
 .class_init = rng_egd_class_init,
-.instance_init = rng_egd_init,
 .instance_finalize = rng_egd_finalize,
 };
 
diff --git a/backends/rng-random.c b/backends/rng-random.c
index 4e51f46..175c61b 100644
--- a/backends/rng-random.c
+++ b/backends/rng-random.c
@@ -110,11 +110,6 @@ static void rng_random_init(Object *obj)
 {
 RndRandom *s = RNG_RANDOM(obj);
 
-object_property_add_str(obj, "filename",
-rng_random_get_filename,
-rng_random_set_filename,
-NULL);
-
 s->filename = g_strdup("/dev/random");
 s->fd = -1;
 }
@@ -137,6 +132,11 @@ static void rng_random_class_init(ObjectClass *klass, void 
*data)
 
 rbc->request_entropy = rng_random_request_entropy;
 rbc->opened = rng_random_opened;
+
+object_class_property_add_str(klass, "filename",
+  rng_random_get_filename,
+  rng_random_set_filename,
+  NULL);
 }
 
 static const TypeInfo rng_random_info = {
diff --git a/backends/rng.c b/backends/rng.c
index 5065fdc..c9c0632 100644
--- a/backends/rng.c
+++ b/backends/rng.c
@@ -72,26 +72,22 @@ static void rng_backend_prop_set_opened(Object *obj, bool 
value, Error **errp)
 s->opened = true;
 }
 
-static void rng_backend_init(Object *obj)
-{
-object_property_add_bool(obj, "opened",
- rng_backend_prop_get_opened,
- rng_backend_prop_set_opened,
- NULL);
-}
-
 static void rng_backend_class_init(ObjectClass *oc, void *data)
 {
 UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
 
 ucc->complete = rng_backend_complete;
+
+object_class_property_add_bool(oc, "opened",
+   rng_backend_prop_get_opened,
+   rng_backend_prop_set_opened,
+   NULL);
 }
 
 static const TypeInfo rng_backend_info = {
 .name = TYPE_RNG_BACKEND,
 .parent = TYPE_OBJECT,
 .instance_size = sizeof(RngBackend),
-.instance_init = rng_backend_init,
 .class_size = sizeof(RngBackendClass),
 .class_init = rng_backend_class_init,
 .abstract = true,
-- 
2.4.3

[Qemu-devel] [PATCH RFC 6/7] x86-cpu: register properties against the class instead of object

[Daniel P. Berrange]

This converts the x86 CPU object to register its properties against
the class rather than object.

Signed-off-by: Daniel P. Berrange 
---
 target-i386/cpu.c | 55 ---
 1 file changed, 28 insertions(+), 27 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 780a5bc..e183d0b 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -3038,33 +3038,6 @@ static void x86_cpu_initfn(Object *obj)
 cs->env_ptr = env;
 cpu_exec_init(cs, &error_abort);
 
-object_property_add(obj, "family", "int",
-x86_cpuid_version_get_family,
-x86_cpuid_version_set_family, NULL, NULL, NULL);
-object_property_add(obj, "model", "int",
-x86_cpuid_version_get_model,
-x86_cpuid_version_set_model, NULL, NULL, NULL);
-object_property_add(obj, "stepping", "int",
-x86_cpuid_version_get_stepping,
-x86_cpuid_version_set_stepping, NULL, NULL, NULL);
-object_property_add_str(obj, "vendor",
-x86_cpuid_get_vendor,
-x86_cpuid_set_vendor, NULL);
-object_property_add_str(obj, "model-id",
-x86_cpuid_get_model_id,
-x86_cpuid_set_model_id, NULL);
-object_property_add(obj, "tsc-frequency", "int",
-x86_cpuid_get_tsc_freq,
-x86_cpuid_set_tsc_freq, NULL, NULL, NULL);
-object_property_add(obj, "apic-id", "int",
-x86_cpuid_get_apic_id,
-x86_cpuid_set_apic_id, NULL, NULL, NULL);
-object_property_add(obj, "feature-words", "X86CPUFeatureWordInfo",
-x86_cpu_get_feature_words,
-NULL, NULL, NULL, NULL);
-object_property_add(obj, "filtered-features", "X86CPUFeatureWordInfo",
-x86_cpu_get_filtered_features,
-NULL, NULL, NULL, NULL);
 
 cpu->hyperv_spinlock_attempts = HYPERV_SPINLOCK_NEVER_RETRY;
 
@@ -3199,6 +3172,34 @@ static void x86_cpu_common_class_init(ObjectClass *oc, 
void *data)
 #endif
 cc->cpu_exec_enter = x86_cpu_exec_enter;
 cc->cpu_exec_exit = x86_cpu_exec_exit;
+
+object_class_property_add(oc, "family", "int",
+  x86_cpuid_version_get_family,
+  x86_cpuid_version_set_family, NULL, NULL, NULL);
+object_class_property_add(oc, "model", "int",
+  x86_cpuid_version_get_model,
+  x86_cpuid_version_set_model, NULL, NULL, NULL);
+object_class_property_add(oc, "stepping", "int",
+  x86_cpuid_version_get_stepping,
+  x86_cpuid_version_set_stepping, NULL, NULL, 
NULL);
+object_class_property_add_str(oc, "vendor",
+  x86_cpuid_get_vendor,
+  x86_cpuid_set_vendor, NULL);
+object_class_property_add_str(oc, "model-id",
+  x86_cpuid_get_model_id,
+  x86_cpuid_set_model_id, NULL);
+object_class_property_add(oc, "tsc-frequency", "int",
+  x86_cpuid_get_tsc_freq,
+  x86_cpuid_set_tsc_freq, NULL, NULL, NULL);
+object_class_property_add(oc, "apic-id", "int",
+  x86_cpuid_get_apic_id,
+  x86_cpuid_set_apic_id, NULL, NULL, NULL);
+object_class_property_add(oc, "feature-words", "X86CPUFeatureWordInfo",
+  x86_cpu_get_feature_words,
+  NULL, NULL, NULL, NULL);
+object_class_property_add(oc, "filtered-features", "X86CPUFeatureWordInfo",
+  x86_cpu_get_filtered_features,
+  NULL, NULL, NULL, NULL);
 }
 
 static const TypeInfo x86_cpu_type_info = {
-- 
2.4.3

[Qemu-devel] [PATCH RFC 5/7] cpu: avoid using object instance state in property getter

[Daniel P. Berrange]

When registering the properties 'feature-words' and 'filtered-features'
object instance data is being passed in. This can easily be accessed
directly via the 'Object *obj' parameter passed to the getter, so
the object instance data does not need to be supplied at property
registration time.

Signed-off-by: Daniel P. Berrange 
---
 target-i386/cpu.c | 39 ---
 1 file changed, 32 insertions(+), 7 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index cfb8aa7..780a5bc 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1766,12 +1766,10 @@ static void x86_cpuid_set_apic_id(Object *obj, Visitor 
*v, void *opaque,
 }
 
 /* Generic getter for "feature-words" and "filtered-features" properties */
-static void x86_cpu_get_feature_words(Object *obj, Visitor *v, void *opaque,
-  const char *name, Error **errp)
+static X86CPUFeatureWordInfoList *
+x86_cpu_get_feature_words_helper(Object *obj, uint32_t *array)
 {
-uint32_t *array = (uint32_t *)opaque;
 FeatureWord w;
-Error *err = NULL;
 X86CPUFeatureWordInfo word_infos[FEATURE_WORDS] = { };
 X86CPUFeatureWordInfoList list_entries[FEATURE_WORDS] = { };
 X86CPUFeatureWordInfoList *list = NULL;
@@ -1791,10 +1789,37 @@ static void x86_cpu_get_feature_words(Object *obj, 
Visitor *v, void *opaque,
 list = &list_entries[w];
 }
 
+return list;
+}
+
+
+static void x86_cpu_get_feature_words(Object *obj, Visitor *v, void *opaque,
+  const char *name, Error **errp)
+{
+X86CPU *cpu = X86_CPU(obj);
+uint32_t *array = cpu->env.features;
+X86CPUFeatureWordInfoList *list =
+x86_cpu_get_feature_words_helper(obj, array);
+Error *err = NULL;
+
 visit_type_X86CPUFeatureWordInfoList(v, &list, "feature-words", &err);
 error_propagate(errp, err);
 }
 
+
+static void x86_cpu_get_filtered_features(Object *obj, Visitor *v, void 
*opaque,
+  const char *name, Error **errp)
+{
+X86CPU *cpu = X86_CPU(obj);
+uint32_t *array = cpu->filtered_features;
+X86CPUFeatureWordInfoList *list =
+x86_cpu_get_feature_words_helper(obj, array);
+Error *err = NULL;
+
+visit_type_X86CPUFeatureWordInfoList(v, &list, "filtered-features", &err);
+error_propagate(errp, err);
+}
+
 static void x86_get_hv_spinlocks(Object *obj, Visitor *v, void *opaque,
  const char *name, Error **errp)
 {
@@ -3036,10 +3061,10 @@ static void x86_cpu_initfn(Object *obj)
 x86_cpuid_set_apic_id, NULL, NULL, NULL);
 object_property_add(obj, "feature-words", "X86CPUFeatureWordInfo",
 x86_cpu_get_feature_words,
-NULL, NULL, (void *)env->features, NULL);
+NULL, NULL, NULL, NULL);
 object_property_add(obj, "filtered-features", "X86CPUFeatureWordInfo",
-x86_cpu_get_feature_words,
-NULL, NULL, (void *)cpu->filtered_features, NULL);
+x86_cpu_get_filtered_features,
+NULL, NULL, NULL, NULL);
 
 cpu->hyperv_spinlock_attempts = HYPERV_SPINLOCK_NEVER_RETRY;
 
-- 
2.4.3

Re: [Qemu-devel] QEMU produces invalid JSON due to locale-dependent code

[Markus Armbruster]

"Daniel P. Berrange"  writes:

> On Wed, Aug 26, 2015 at 08:46:35AM +0200, Gerd Hoffmann wrote:
>>   Hi,
>> 
>> > > It seems the only thing that we really care about being localized is
>> > > the messages catalogue, so the GTK UI gets internationalization in
>> > > its menus / dialogs / etc. As such I think that we should do the
>> > > opposite of (C). ie run every LC_* in the C locale, except for
>> > > LC_MESSAGES which we allow to be localized.
>> > > 
>> > > This avoids any unpredictable functional consequences (like number
>> > > formatting) while still giving user decent localization in the UI
>> > 
>> > Except that the LC_MESSAGES catalog may include messages such as "blah
>> > %d blah" that get translated for use as a printf argument, and the lack
>> > of matching LC_NUMERIC will make the translation look wrong.
>> > Translators often assume that their translation is being used with
>> > everything else about the locale matching the locale of the translation.
>> 
>> I don't think this is a big issue in practice.  The menu item names are
>> pretty much the only thing translated in the qemu gtk ui ...
>
> Also we're talking about a tradeoff between functionally incorrect
> formatting of JSON, vs "wrong looking" translations with no functional
> impact, which probably won't even affect QEMU in reality.
>
> In terms of minimizing hacks to QEMU, it seems like only localizing
> LC_MESSAGES is a simple enough tradeoff to avoid functionally
> incorrect behaviour with minimal downside and no code complexity
> messing around with thread-locales, etc

It's a hack, and as such it needs a fat comment explaining it.

[Qemu-devel] [PATCH] qemu-ga: Fixed paths issue with MSI build

[Leonid Bloch]

Previously, if building out-of-tree, the MSI build would fail since it wasn't 
able to find the needed files.

Signed-off-by: Leonid Bloch 
---
 Makefile  | 4 ++--
 qga/installer/qemu-ga.wxs | 6 +++---
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/Makefile b/Makefile
index 340d9c8..2d35331 100644
--- a/Makefile
+++ b/Makefile
@@ -305,8 +305,8 @@ endif
 
 $(QEMU_GA_MSI): config-host.mak
 
-$(QEMU_GA_MSI):  qga/installer/qemu-ga.wxs
-   $(call quiet-command,QEMU_GA_VERSION="$(QEMU_GA_VERSION)" 
QEMU_GA_MANUFACTURER="$(QEMU_GA_MANUFACTURER)" 
QEMU_GA_DISTRO="$(QEMU_GA_DISTRO)" \
+$(QEMU_GA_MSI):  $(SRC_PATH)/qga/installer/qemu-ga.wxs
+   $(call quiet-command,QEMU_GA_VERSION="$(QEMU_GA_VERSION)" 
QEMU_GA_MANUFACTURER="$(QEMU_GA_MANUFACTURER)" 
QEMU_GA_DISTRO="$(QEMU_GA_DISTRO)" BUILD_DIR="$(BUILD_DIR)" \
wixl -o $@ $(QEMU_GA_MSI_ARCH) $(QEMU_GA_MSI_WITH_VSS) 
$(QEMU_GA_MSI_MINGW_DLL_PATH) $<, "  WIXL  $@")
 else
 msi:
diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs
index 41b11cf..7885973 100644
--- a/qga/installer/qemu-ga.wxs
+++ b/qga/installer/qemu-ga.wxs
@@ -66,7 +66,7 @@
   
 
   
-
+
 
   
   
-
+
   
   
-
+
   
   
   
-- 
2.4.3

Re: [Qemu-devel] [PATCH] Add another sanity check to smp_parse() function

[Bastian Koppelmann]

Am 25.08.2015 um 15:25 schrieb Thomas Huth:

On 19/08/15 17:58, Eduardo Habkost wrote:

On Wed, Jul 22, 2015 at 03:59:50PM +0200, Thomas Huth wrote:

The code in smp_parse already checks the topology information for
sockets * cores * threads < cpus and bails out with an error in
that case. However, it is still possible to supply a bad configuration
the other way round, e.g. with:

  qemu-system-xxx -smp 4,sockets=1,cores=4,threads=2

QEMU then still starts the guest, with topology configuration that
is rather incomprehensible and likely not what the user wanted.
So let's add another check to refuse such wrong configurations.

Signed-off-by: Thomas Huth 
---
  vl.c | 8 +++-
  1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/vl.c b/vl.c
index 5856396..c8d24b1 100644
--- a/vl.c
+++ b/vl.c
@@ -1224,7 +1224,13 @@ static void smp_parse(QemuOpts *opts)
  exit(1);
  }
  
-max_cpus = qemu_opt_get_number(opts, "maxcpus", 0);

+max_cpus = qemu_opt_get_number(opts, "maxcpus", cpus);
+if (sockets * cores * threads > max_cpus) {
+fprintf(stderr, "cpu topology: error: "
+"sockets (%u) * cores (%u) * threads (%u) > maxcpus 
(%u)\n",
+sockets, cores, threads, max_cpus);
+exit(1);
+}

I am always afraid of breaking existing setups when we do that, because
there may be existing VMs running with these weird configurations, and
people won't be able to live-migrate them to a newer QEMU.

But I think we really have to start refusing to run obviously broken
configurations one day, or we will never fix this mess, so:

Reviewed-by: Eduardo Habkost 

I want to apply this through the x86 tree, but I would like to get some
Acked-by from other maintainers first.

Ok, thanks!

So *ping* to the other CPU core maintainers here ... could I get some
more ACKs, please?

  Thomas



Acked-by: Bastian Koppelmann 

[Qemu-devel] [PATCH v2] target-mips: remove wrong checks for recip.fmt and rsqrt.fmt

[Petar Jovanovic]

From: Petar Jovanovic 

Instructions recip.{s|d} and rsqrt.{s|d} do not require 64-bit FPU neither
they require any particular mode for its FPU. This patch removes the checks
that may break a program that uses these instructions.

Signed-off-by: Petar Jovanovic 
---
v2:
- add check_cp1_registers() for recip.d and rsqrt.d, as suggested by Leon A.

 target-mips/translate.c |6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/target-mips/translate.c b/target-mips/translate.c
index 93cb4f2..15285bc 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -9294,7 +9294,6 @@ static void gen_farith (DisasContext *ctx, enum fopcode 
op1,
 opn = "movn.s";
 break;
 case OPC_RECIP_S:
-check_cop1x(ctx);
 {
 TCGv_i32 fp0 = tcg_temp_new_i32();
 
@@ -9306,7 +9305,6 @@ static void gen_farith (DisasContext *ctx, enum fopcode 
op1,
 opn = "recip.s";
 break;
 case OPC_RSQRT_S:
-check_cop1x(ctx);
 {
 TCGv_i32 fp0 = tcg_temp_new_i32();
 
@@ -9839,7 +9837,7 @@ static void gen_farith (DisasContext *ctx, enum fopcode 
op1,
 opn = "movn.d";
 break;
 case OPC_RECIP_D:
-check_cp1_64bitmode(ctx);
+check_cp1_registers(ctx, fs | fd);
 {
 TCGv_i64 fp0 = tcg_temp_new_i64();
 
@@ -9851,7 +9849,7 @@ static void gen_farith (DisasContext *ctx, enum fopcode 
op1,
 opn = "recip.d";
 break;
 case OPC_RSQRT_D:
-check_cp1_64bitmode(ctx);
+check_cp1_registers(ctx, fs | fd);
 {
 TCGv_i64 fp0 = tcg_temp_new_i64();
 
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH 0/3] rtl8139: cleanups

[Stefan Hajnoczi]

On Mon, Aug 03, 2015 at 01:15:54PM +0100, Stefan Hajnoczi wrote:
> After addressing CVE-2015-5165 there were several additional rtl8139 cleanups
> that I'm sending separately since they are not security fixes.
> 
> These patches eliminate duplicate eth.h structs/macros and fix unaligned 
> memory
> accesses in tx offload.
> 
> Stefan Hajnoczi (3):
>   rtl8139: remove duplicate net/eth.h definitions
>   rtl8139: use net/eth.h macros instead of custom macros
>   rtl8139: use ldl/stl wrapper for unaligned 32-bit access
> 
>  hw/net/rtl8139.c | 103 
> +++
>  1 file changed, 27 insertions(+), 76 deletions(-)
> 
> -- 
> 2.4.3
> 
> 

Thanks, applied to my net tree:
https://github.com/stefanha/qemu/commits/net

Stefan

[Qemu-devel] MTTCG next version?

[Frederic Konrad]

Hi everybody,

I'm trying to do the next version of the MTTCG work:

I would like to rebase on Alvise atomic instruction branch:
  - Alvise can you rebase it on the 2.4.0 version without MTTCG support 
and then
point me to the MTTCG specific changes so I can include them in my 
tree?

I will add Paolo's linux-user and signal free qemu_cpu_kick series as well.

About tb_flush we think to do that without exiting:
  - Use two buffers for tbs.
  - Use a per tb invalidated flag.
  - when tb_flush just invalidate all tb from the buffer and swap to 
the second buffer:
VCPU which are executing code will discard their tb_jmp_cache when 
they exit

(eg: run_on_cpu).

We need also to fix emulated data barrier so tlb_flush are finished 
before the

instruction is executed. (That might be only data barrier breaks the TB).

Protecting page->code_bitmap and cpu_breakpoint_insert changes will be 
squashed in the tb_lock patch.


More tests must be done especially with gdbstub and icount.

Do that make sense?
Fred

Re: [Qemu-devel] [PATCH 1/2] rtl8139: Do not consume the packet during overflow in standard mode.

[Stefan Hajnoczi]

On Fri, Aug 21, 2015 at 02:59:24PM -0700, Vladislav Yasevich wrote:
> When operation in standard mode, we currently return the size
> of packet during buffer overflow.  This consumes the overflow
> packet.  Return 0 instead so we can re-process the overflow packet
> when we have room.
> 
> This fixes issues with lost/dropped fragments of large messages.
> 
> Signed-off-by: Vladislav Yasevich 
> ---
>  hw/net/rtl8139.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
> index edbb61c..359e001 100644
> --- a/hw/net/rtl8139.c
> +++ b/hw/net/rtl8139.c
> @@ -1157,7 +1157,7 @@ static ssize_t rtl8139_do_receive(NetClientState *nc, 
> const uint8_t *buf, size_t
>  s->IntrStatus |= RxOverflow;
>  ++s->RxMissed;
>  rtl8139_update_irq(s);
> -return size_;
> +return 0;

Every .receive() return 0 must be paired with a
qemu_flush_queued_packets() call.

Is rtl8139_RxBufPtr_write() guaranteed to be called when the guest
refills rx buffers?

Re: [Qemu-devel] MTTCG next version?

[Mark Burton]

Just to remind everybody as well - we’ll have a call next Monday to co-ordinate.
It would be good to make sure everybody knows which bit of this everybody else 
is committing to do, so we avoid replication and treading on each others patch 
sets.

Cheers

Mark.

> On 26 Aug 2015, at 14:18, Frederic Konrad  wrote:
> 
> Hi everybody,
> 
> I'm trying to do the next version of the MTTCG work:
> 
> I would like to rebase on Alvise atomic instruction branch:
>  - Alvise can you rebase it on the 2.4.0 version without MTTCG support and 
> then
>point me to the MTTCG specific changes so I can include them in my tree?
> I will add Paolo's linux-user and signal free qemu_cpu_kick series as well.
> 
> About tb_flush we think to do that without exiting:
>  - Use two buffers for tbs.
>  - Use a per tb invalidated flag.
>  - when tb_flush just invalidate all tb from the buffer and swap to the 
> second buffer:
>VCPU which are executing code will discard their tb_jmp_cache when they 
> exit
>(eg: run_on_cpu).
> 
> We need also to fix emulated data barrier so tlb_flush are finished before the
> instruction is executed. (That might be only data barrier breaks the TB).
> 
> Protecting page->code_bitmap and cpu_breakpoint_insert changes will be 
> squashed in the tb_lock patch.
> 
> More tests must be done especially with gdbstub and icount.
> 
> Do that make sense?
> Fred


 +44 (0)20 7100 3485 x 210
 +33 (0)5 33 52 01 77x 210

+33 (0)603762104
mark.burton

Re: [Qemu-devel] [PATCH] qemu-ga: Fixed paths issue with MSI build

[Marc-André Lureau]

looks good

Reviewed-by: Marc-André Lureau 


On Wed, Aug 26, 2015 at 2:07 PM, Leonid Bloch  wrote:
> Previously, if building out-of-tree, the MSI build would fail since it wasn't 
> able to find the needed files.
>
> Signed-off-by: Leonid Bloch 
> ---
>  Makefile  | 4 ++--
>  qga/installer/qemu-ga.wxs | 6 +++---
>  2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 340d9c8..2d35331 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -305,8 +305,8 @@ endif
>
>  $(QEMU_GA_MSI): config-host.mak
>
> -$(QEMU_GA_MSI):  qga/installer/qemu-ga.wxs
> -   $(call quiet-command,QEMU_GA_VERSION="$(QEMU_GA_VERSION)" 
> QEMU_GA_MANUFACTURER="$(QEMU_GA_MANUFACTURER)" 
> QEMU_GA_DISTRO="$(QEMU_GA_DISTRO)" \
> +$(QEMU_GA_MSI):  $(SRC_PATH)/qga/installer/qemu-ga.wxs
> +   $(call quiet-command,QEMU_GA_VERSION="$(QEMU_GA_VERSION)" 
> QEMU_GA_MANUFACTURER="$(QEMU_GA_MANUFACTURER)" 
> QEMU_GA_DISTRO="$(QEMU_GA_DISTRO)" BUILD_DIR="$(BUILD_DIR)" \
> wixl -o $@ $(QEMU_GA_MSI_ARCH) $(QEMU_GA_MSI_WITH_VSS) 
> $(QEMU_GA_MSI_MINGW_DLL_PATH) $<, "  WIXL  $@")
>  else
>  msi:
> diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs
> index 41b11cf..7885973 100644
> --- a/qga/installer/qemu-ga.wxs
> +++ b/qga/installer/qemu-ga.wxs
> @@ -66,7 +66,7 @@
>
>  
> Guid="{908B7199-DE2A-4DC6-A8D0-27A5AE444FEA}">
> - Source="../../qemu-ga.exe" KeyPath="yes" DiskId="1"/>
> + Source="$(env.BUILD_DIR)/qemu-ga.exe" KeyPath="yes" DiskId="1"/>
>  Id="ServiceInstaller"
>Type="ownProcess"
> @@ -85,10 +85,10 @@
>
>
> Guid="{CB19C453-FABB-4BB1-ABAB-6B74F687BFBB}">
> - Source="../vss-win32/qga-vss.dll" KeyPath="yes" DiskId="1"/>
> + Source="$(env.BUILD_DIR)/qga/vss-win32/qga-vss.dll" KeyPath="yes" DiskId="1"/>
>
> Guid="{D8D584B1-59C2-4FB7-A91F-636FF7BFA66E}">
> - Source="../vss-win32/qga-vss.tlb" KeyPath="yes" DiskId="1"/>
> + Source="$(env.BUILD_DIR)/qga/vss-win32/qga-vss.tlb" KeyPath="yes" DiskId="1"/>
>
>
> Guid="{35EE3558-D34B-4F0A-B8BD-430FF0775246}">
> --
> 2.4.3
>



-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH 2/2] rtl8139: correctly track full receive buffer in standard mode

[Stefan Hajnoczi]

On Fri, Aug 21, 2015 at 02:59:25PM -0700, Vladislav Yasevich wrote:
> In standard operation mode, when the receive ring buffer
> is full, the buffer actually appears empty to the driver since
> the RxBufAddr (the location we wirte new data to) and RxBufPtr
> (the location guest would stat reading from) are the same.
> As a result, the call to rtl8139_RxBufferEmpty ends up
> returning true indicating that the receive buffer is empty.
> This would result in the next packet overwriting the recevie buffer
> again and stalling receive operations.
> 
> This patch catches the "receive buffer full" condition
> using an unused C+ register.  This is done to simplify
> migration and not require a new machine type.
> 
> Signed-off-by: Vladislav Yasevich 
> ---
>  hw/net/rtl8139.c | 36 ++--
>  1 file changed, 34 insertions(+), 2 deletions(-)

The rtl8139 code duplicates the following expression in several places:

  MOD2(s->RxBufferSize + s->RxBufAddr - s->RxBufPtr, s->RxBufferSize);

It may be cleaner to keep a rx_unread_bytes counter so that all these
users can simply look at that variable.

That cleanup also eliminates the rx full vs empty problem because then
we'll know whether rx_unread_bytes == 0 or rx_unread_bytes ==
s->RxBufferSize.

The same trick of stashing the value in s->currCPlusRxDesc could be
used.

> diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
> index 359e001..3d572ab 100644
> --- a/hw/net/rtl8139.c
> +++ b/hw/net/rtl8139.c
> @@ -816,6 +816,23 @@ static int rtl8139_can_receive(NetClientState *nc)
>  }
>  }
>  
> +static void rtl8139_set_rxbuf_full(RTL8139State *s, bool full)
> +{
> +/* In standard mode, C+ RxDesc isn't used.  Reuse it
> + * to store the rx_buf_full status.
> + */

assert(!s->cplus_enabled)?

> +s->currCPlusRxDesc = full;
> +DPRINTF("received: rx buffer full\n");
> +}
> +
> +static bool rtl8139_rxbuf_full(RTL8139State *s)
> +{
> +/* In standard mode, C+ RxDesc isn't used.  Reuse it
> + * to store the rx_buf_full status.
> + */

assert(!s->cplus_enabled)?

> @@ -2601,6 +2630,9 @@ static void rtl8139_RxBufPtr_write(RTL8139State *s, 
> uint32_t val)
>  /* this value is off by 16 */
>  s->RxBufPtr = MOD2(val + 0x10, s->RxBufferSize);
>  
> +/* We just read data, clear full buffer state */
> +rtl8139_set_rxbuf_full(s, false);
> +
>  /* more buffer space may be available so try to receive */
>  qemu_flush_queued_packets(qemu_get_queue(s->nic));

What if the guest writes this register while we're in C+ mode?

Re: [Qemu-devel] [PATCH v4 1/7] crypto: introduce new base module for TLS credentials

[Daniel P. Berrange]

On Mon, Aug 24, 2015 at 02:25:24PM -0600, Eric Blake wrote:
> > +/* #define QCRYPTO_DEBUG */
> > +
> > +#ifdef QCRYPTO_DEBUG
> > +#define DPRINTF(fmt, ...) do { fprintf(stderr, fmt, ## __VA_ARGS__); } 
> > while (0)
> > +#else
> > +#define DPRINTF(fmt, ...) do { } while (0)
> > +#endif
> 
> Please rework this to:
> 
> #ifdef QCRYPTO_DEBUG
> # define QCRYPT_DEBUG_PRINT 1
> #else
> # define QCRYPT_DEBUG_PRINT 0
> #endif
> #define DPRINTF(fmt, ...) \
> do { \
> if (QCRYPT_DEBUG_PRINT) { \
> fprintf(stderr, fmt, ## __VA_ARGS__); \
> } \
> } while (0)

Ah that's a good idea.

One day it would nice if QEMU had a standardized debug logging macro
in qemu-common.h, so we could just turn on/off debugging per file
using

   #define ENABLE_DEBUG 1
   #include "qemu-common.h"

> > +#define DH_BITS 2048
> > +
> > +static const char * const endpoint_map[QCRYPTO_TLS_CREDS_ENDPOINT_LAST + 
> > 1] = {
> > +[QCRYPTO_TLS_CREDS_ENDPOINT_SERVER] = "server",
> > +[QCRYPTO_TLS_CREDS_ENDPOINT_CLIENT] = "client",
> > +[QCRYPTO_TLS_CREDS_ENDPOINT_LAST] = NULL,
> > +};
> 
> Is it worth an entry in a .json file to get qapi to generate this
> mapping automatically?

I guess adding the enum definition itself to QAPI would mean we
would get better introspection support when we solve QOM class
introspection of properties.

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

Re: [Qemu-devel] [PATCH] virtio dataplane: adapt dataplane for virtio Version 1

[Stefan Hajnoczi]

On Tue, Aug 25, 2015 at 12:33:30PM +0200, Pierre Morel wrote:
> -vring_init(&vring->vr, virtio_queue_get_num(vdev, n), vring_ptr, 4096);

vring_init() is no longer used.  Please delete it.