[Qemu-devel] Changes since v2 -qga: Add `guest-get-timezone` command

[Vinzenz 'evilissimo' Feenstra]

Updated the documentation according to comments of eblake

[Qemu-devel] [PATCH v2] qga: Add `guest-get-timezone` command

[Vinzenz 'evilissimo' Feenstra]

From: Vinzenz Feenstra 

Adds a new command `guest-get-timezone` reporting the currently
configured timezone on the system. The information on what timezone is
currently is configured is useful in case of Windows VMs where the
offset of the hardware clock is required to have the same offset. This
can be used for management systems like `oVirt` to detect the timezone
difference and warn administrators of the misconfiguration.

Signed-off-by: Vinzenz Feenstra 
---
 qga/commands.c   | 19 +++
 qga/qapi-schema.json | 26 ++
 2 files changed, 45 insertions(+)

diff --git a/qga/commands.c b/qga/commands.c
index 4d92946..83d7f99 100644
--- a/qga/commands.c
+++ b/qga/commands.c
@@ -499,3 +499,22 @@ int ga_parse_whence(GuestFileWhence *whence, Error **errp)
 error_setg(errp, "invalid whence code %"PRId64, whence->u.value);
 return -1;
 }
+
+GuestTimezone *qmp_guest_get_timezone(Error **errp)
+{
+GuestTimezone *info = g_new0(GuestTimezone, 1);
+GTimeZone *tz = g_time_zone_new_local();
+gint32 interval = g_time_zone_find_interval(tz, G_TIME_TYPE_STANDARD, 0);
+gchar const *name = g_time_zone_get_abbreviation(tz, interval);
+if (name != NULL) {
+info->offset = g_time_zone_get_offset(tz, interval) / 60;
+info->zone = g_strdup(name);
+} else {
+error_setg(errp, "Timezone lookup failed");
+g_free(info);
+info = NULL;
+}
+g_time_zone_unref(tz);
+return info;
+}
+
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index a02dbf2..62d6909 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -1042,3 +1042,29 @@
   'data':{ 'path': 'str', '*arg': ['str'], '*env': ['str'],
'*input-data': 'str', '*capture-output': 'bool' },
   'returns': 'GuestExec' }
+
+
+##
+# @GuestTimezone:
+#
+# @zone:Timezone name
+# @offset:  Offset to UTC in minutes. For western timezones the offset has a
+#   negative value and for eastern the offset is positive value
+#
+# Since: 2.10
+##
+{ 'struct': 'GuestTimezone',
+  'data':   { 'zone': 'str', 'offset': 'int' } }
+
+
+##
+# @guest-get-timezone:
+#
+# Retrieves the timezone information from the guest.
+#
+# Returns: A GuestTimezone dictionary.
+#
+# Since: 2.10
+##
+{ 'command': 'guest-get-timezone',
+  'returns': 'GuestTimezone' }
-- 
2.9.3

Re: [Qemu-devel] [PATCH v3] Allow setting NUMA distance for different NUMA nodes

[He Chen]

On Wed, Mar 22, 2017 at 11:34:05AM +0100, Andrew Jones wrote:
> 
> You should have CC'ed me, considering this version is addressing my
> review comments, but whatever, I usually skim the list so I found it...
> 
> On Wed, Mar 22, 2017 at 05:32:46PM +0800, He Chen wrote:
> > Current, QEMU does not provide a clear command to set vNUMA distance for
> > guest although we already have `-numa` command to set vNUMA nodes.
> > 
> > vNUMA distance makes sense in certain scenario.
> > But now, if we create a guest that has 4 vNUMA nodes, when we check NUMA
> > info via `numactl -H`, we will see:
> > 
> > node distance:
> > node0123
> >   0:   10   20   20   20
> >   1:   20   10   20   20
> >   2:   20   20   10   20
> >   3:   20   20   20   10
> > 
> > Guest kernel regards all local node as distance 10, and all remote node
> > as distance 20 when there is no SLIT table since QEMU doesn't build it.
> > It looks like a little strange when you have seen the distance in an
> > actual physical machine that contains 4 NUMA nodes. My machine shows:
> > 
> > node distance:
> > node0123
> >   0:   10   21   31   41
> >   1:   21   10   21   31
> >   2:   31   21   10   21
> >   3:   41   31   21   10
> > 
> > To set vNUMA distance, guest should see a complete SLIT table.
> > I found QEMU has provide `-acpitable` command that allows users to add
> > a ACPI table into guest, but it requires users building ACPI table by
> > themselves first. Using `-acpitable` to add a SLIT table may be not so
> > straightforward or flexible, imagine that when the vNUMA configuration
> > is changes and we need to generate another SLIT table manually. It may
> > not be friendly to users or upper software like libvirt.
> > 
> > This patch is going to add SLIT table support in QEMU, and provides
> > additional option `dist` for command `-numa` to allow user set vNUMA
> > distance by QEMU command.
> > 
> > With this patch, when a user wants to create a guest that contains
> > several vNUMA nodes and also wants to set distance among those nodes,
> > the QEMU command would like:
> > 
> > ```
> > -object 
> > memory-backend-ram,size=1G,prealloc=yes,host-nodes=0,policy=bind,id=node0 \
> > -numa node,nodeid=0,cpus=0,memdev=node0 \
> > -object 
> > memory-backend-ram,size=1G,prealloc=yes,host-nodes=1,policy=bind,id=node1 \
> > -numa node,nodeid=1,cpus=1,memdev=node1 \
> > -object 
> > memory-backend-ram,size=1G,prealloc=yes,host-nodes=2,policy=bind,id=node2 \
> > -numa node,nodeid=2,cpus=2,memdev=node2 \
> > -object 
> > memory-backend-ram,size=1G,prealloc=yes,host-nodes=3,policy=bind,id=node3 \
> > -numa node,nodeid=3,cpus=3,memdev=node3 \
> > -numa dist,src=0,dst=1,val=21 \
> > -numa dist,src=0,dst=2,val=31 \
> > -numa dist,src=0,dst=3,val=41 \
> > -numa dist,src=1,dst=0,val=21 \
> > ...
> > ```
> > 
> > Signed-off-by: He Chen 
> > 
> > fix
> 
> stray 'fix' above the ---
> 
> > ---
> >  hw/acpi/aml-build.c | 26 +
> >  hw/arm/virt-acpi-build.c|  2 ++
> >  hw/i386/acpi-build.c|  2 ++
> >  include/hw/acpi/aml-build.h |  1 +
> >  include/sysemu/numa.h   |  1 +
> >  include/sysemu/sysemu.h |  4 
> >  numa.c  | 47 
> > +
> >  qapi-schema.json| 30 ++---
> >  qemu-options.hx | 12 +++-
> >  9 files changed, 121 insertions(+), 4 deletions(-)
> > 
> > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> > index c6f2032..410b30e 100644
> > --- a/hw/acpi/aml-build.c
> > +++ b/hw/acpi/aml-build.c
> > @@ -24,6 +24,7 @@
> >  #include "hw/acpi/aml-build.h"
> >  #include "qemu/bswap.h"
> >  #include "qemu/bitops.h"
> > +#include "sysemu/numa.h"
> >  
> >  static GArray *build_alloc_array(void)
> >  {
> > @@ -1609,3 +1610,28 @@ void build_srat_memory(AcpiSratMemoryAffinity 
> > *numamem, uint64_t base,
> >  numamem->base_addr = cpu_to_le64(base);
> >  numamem->range_length = cpu_to_le64(len);
> >  }
> > +
> > +/*
> > + * ACPI spec 5.2.17 System Locality Distance Information Table
> > + * (Revision 2.0 or later)
> > + */
> > +void build_slit(GArray *table_data, BIOSLinker *linker)
> > +{
> > +int slit_start, i, j;
> > +slit_start = table_data->len;
> > +
> > +acpi_data_push(table_data, sizeof(AcpiTableHeader));
> > +
> > +build_append_int_noprefix(table_data, nb_numa_nodes, 8);
> > +for (i = 0; i < nb_numa_nodes; i++) {
> > +for (j = 0; j < nb_numa_nodes; j++) {
> > +build_append_int_noprefix(table_data, 
> > numa_info[i].distance[j], 1);
> > +}
> > +}
> > +
> > +build_header(linker, table_data,
> > + (void *)(table_data->data + slit_start),
> > + "SLIT",
> > + table_data->len - slit_start, 1, NULL, NULL);
> > +}
> > +
> > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> > index 0835e59..d9e6828 100644
> > --- a/hw/arm/virt-acpi-build

Re: [Qemu-devel] [PATCH] Revert "apic: save apic_delivered flag"

[Peter Xu]

On Wed, Mar 22, 2017 at 01:13:44PM +0100, Paolo Bonzini wrote:
> This reverts commit 07bfa354772f2de67008dc66c201b627acff0106.
> The global variable is only read as part of a
> 
> apic_reset_irq_delivered();
> qemu_irq_raise(s->irq);
> if (!apic_get_irq_delivered()) {
> 
> sequence, so the value never matters at migration time.
> 
> Reported-by: Dr. David Alan Gilbert 
> Cc: Pavel Dovgalyuk 
> Signed-off-by: Paolo Bonzini 

Reviewed-by: Peter Xu 

-- peterx

Re: [Qemu-devel] [RFC][PATCH 0/6] "bootonceindex" property

[Janne Huttunen]

On Wed, 2017-03-22 at 16:29 +0100, Laszlo Ersek wrote:
> > I'm not. I'm using QMP to change the index dynamically.
> > 
> Wait, if you are already changing the "bootindex" property
> dynamically (do I understand that right?)

No, I'm not changing "bootindex" dynamically. I'm changing
"bootonceindex" dynamically. The point is that whatever
change I'm making is supposed to affect only one boot, the
next one. Since the guest can trigger reboots by itself,
I don't necessarily know when they are going to happen.

Like I said earlier, I can get very close to the semantics
I need if set the "bootindex" and get an event when the
boot happens so that I know when to reset the bootindex
back to the original value. However doing it like that
is (at least in theory) racy if the event isn't synchronous
and it requires some process that actively monitors those
events which I'm trying to avoid.

> ...and it could have a significant maintenance footprint,
> while the feature does look niche (to me anyway).

Whatever I'm currently doing is definitely a niche, but very
similar setting of a one time boot source while the system
is running is e.g. part of the IPMI standard (see "Set
System Boot Options Command" in IPMI Specification), so
the concept itself is not that much of a niche.

Re: [Qemu-devel] [PATCH] Revert "apic: save apic_delivered flag"

[Pavel Dovgalyuk]

This value is used by mc146818rtc.
Therefore it affects the vitrual machine state.
I've encountered the cases when replay was broken without migrating of this 
variable.

⁣Отправлено с помощью BlueMail ​

На 22 Мар 2017 г., 15:13, в 15:13, Paolo Bonzini  
написал:>This reverts commit 07bfa354772f2de67008dc66c201b627acff0106.
>The global variable is only read as part of a
>
>apic_reset_irq_delivered();
>qemu_irq_raise(s->irq);
>if (!apic_get_irq_delivered()) {
>
>sequence, so the value never matters at migration time.
>
>Reported-by: Dr. David Alan Gilbert 
>Cc: Pavel Dovgalyuk 
>Signed-off-by: Paolo Bonzini 
>---
> hw/intc/apic_common.c   | 33 -
> include/hw/i386/apic_internal.h |  2 --
> 2 files changed, 35 deletions(-)
>
>diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
>index 7a6e771..c3829e3 100644
>--- a/hw/intc/apic_common.c
>+++ b/hw/intc/apic_common.c
>@@ -387,25 +387,6 @@ static bool apic_common_sipi_needed(void *opaque)
> return s->wait_for_sipi != 0;
> }
>
>-static bool apic_irq_delivered_needed(void *opaque)
>-{
>-APICCommonState *s = APIC_COMMON(opaque);
>-return s->cpu == X86_CPU(first_cpu) && apic_irq_delivered != 0;
>-}
>-
>-static void apic_irq_delivered_pre_save(void *opaque)
>-{
>-APICCommonState *s = APIC_COMMON(opaque);
>-s->apic_irq_delivered = apic_irq_delivered;
>-}
>-
>-static int apic_irq_delivered_post_load(void *opaque, int version_id)
>-{
>-APICCommonState *s = APIC_COMMON(opaque);
>-apic_irq_delivered = s->apic_irq_delivered;
>-return 0;
>-}
>-
> static const VMStateDescription vmstate_apic_common_sipi = {
> .name = "apic_sipi",
> .version_id = 1,
>@@ -418,19 +399,6 @@ static const VMStateDescription
>vmstate_apic_common_sipi = {
> }
> };
>
>-static const VMStateDescription vmstate_apic_irq_delivered = {
>-.name = "apic_irq_delivered",
>-.version_id = 1,
>-.minimum_version_id = 1,
>-.needed = apic_irq_delivered_needed,
>-.pre_save = apic_irq_delivered_pre_save,
>-.post_load = apic_irq_delivered_post_load,
>-.fields = (VMStateField[]) {
>-VMSTATE_INT32(apic_irq_delivered, APICCommonState),
>-VMSTATE_END_OF_LIST()
>-}
>-};
>-
> static const VMStateDescription vmstate_apic_common = {
> .name = "apic",
> .version_id = 3,
>@@ -465,7 +433,6 @@ static const VMStateDescription vmstate_apic_common
>= {
> },
> .subsections = (const VMStateDescription*[]) {
> &vmstate_apic_common_sipi,
>-&vmstate_apic_irq_delivered,
> NULL
> }
> };
>diff --git a/include/hw/i386/apic_internal.h
>b/include/hw/i386/apic_internal.h
>index 20ad28c..1209eb4 100644
>--- a/include/hw/i386/apic_internal.h
>+++ b/include/hw/i386/apic_internal.h
>@@ -189,8 +189,6 @@ struct APICCommonState {
> DeviceState *vapic;
> hwaddr vapic_paddr; /* note: persistence via kvmvapic */
> bool legacy_instance_id;
>-
>-int apic_irq_delivered; /* for saving static variable */
> };
>
> typedef struct VAPICState {
>--
>2.9.3

Re: [Qemu-devel] [PATCH RESEND 1/3] usb-ccid: make ccid_write_data_block() cope with null buffers

[Gerd Hoffmann]

  Hi,

> > +if (len == 0) {
> > +return;
> 
> Correct only if messages without data always have the same meaning as no
> message.  Gerd?

Not a ccid expert, but looking through the code it seems writing a
(reply) data block with status and without payload (data = NULL and len
= 0) is perfectly fine and can happen in case no (virtual) smartcard is
inserted into the card reader.  Which this patch breaks.  So,

NACK.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v3] Allow setting NUMA distance for different NUMA nodes

[Andrew Jones]

On Thu, Mar 23, 2017 at 03:23:35PM +0800, He Chen wrote:
> > 
> > And maybe should also point out how to set an unreachable node with 255.
> > 
> Thanks for your careful review. Regarding setting unreachable node,
> which way is good? Set exact 255 as distance or distance that is larger
> than 255 is regarded as unreachable?

If a numeric distance is to be used, then I think following the spec with
255 is best. Alternatively, we could have something like
'-numa dist,src=0,dst=1,unreachable', rather than specifying val at all,
but, IMO, it's not worth it.

Thanks,
drew

[Qemu-devel] What is the best commit for record-replay?

[Igor R]

Hi,

I'm trying to use the deterministic record/replay feature, and I would
like to know which commit I should take to get it work.
In RC0 it seems to be broken. I tried pre-MTTCG commit 2421f381dc, as
mentioned here:
http://lists.nongnu.org/archive/html/qemu-devel/2017-03/msg02657.html
with this patch:
http://lists.nongnu.org/archive/html/qemu-devel/2017-02/msg01316.html
My command line is:
./qemu-system-i386 -net none  -icount
shift=7,rr=replay,rrfile=replay.bin -drive
file=MyFedora386.qcow,if=none,id=img-direct -drive
driver=blkreplay,if=none,image=img-direct,id=img-blkreplay -device
ide-hd,drive=img-blkreplay -monitor stdio

The replay advances for a while, but gets stuck in about 10-15 sec,
and it looks like it encounters deadlock trying to acquire rcu lock.

Is there a working commit of RR?


Thanks.

Re: [Qemu-devel] [PATCH RESEND 1/3] usb-ccid: make ccid_write_data_block() cope with null buffers

[Marc-André Lureau]

Hi

On Thu, Mar 23, 2017 at 11:44 AM Gerd Hoffmann  wrote:

>   Hi,
>
> > > +if (len == 0) {
> > > +return;
> >
> > Correct only if messages without data always have the same meaning as no
> > message.  Gerd?
>
> Not a ccid expert, but looking through the code it seems writing a
> (reply) data block with status and without payload (data = NULL and len
> = 0) is perfectly fine and can happen in case no (virtual) smartcard is
> inserted into the card reader.  Which this patch breaks.  So,
>
> NACK.
>

 oops, there are hard-coded calls with NULL/0. I suppose to fix clang
warning, it would need to check if data != null for memcpy.
-- 
Marc-André Lureau

Re: [Qemu-devel] Minimum RAM size for PC machines?

[Gerd Hoffmann]

On Mi, 2017-03-22 at 11:19 +0100, David Hildenbrand wrote:
> On 22.03.2017 11:03, Thomas Huth wrote:
> > On 22.03.2017 10:08, Markus Armbruster wrote:
> > [...]
> >> Are we now ready to accept a simple & stupid patch that actually helps
> >> users, say letting boards that care declare minimum and maximum RAM
> >> size?  And make PC reject RAM size less than 1MiB, even though "someone"
> >> might conceivably have firmware that works with less?
> > 
> > I'd say enforce a minimum RAM size on the normal "pc" and "q35" machine,
> > but still allow smaller sizes on the "isapc" machine. So if "someone"
> > comes around and claims to have a legacy firmware that wants less memory
> > than 1MiB, just point them to the isapc machine.
> > Just my 0.02 €.
> > 
> >  Thomas
> 
> Or maybe simply warn the user that things may go wrong instead of
> enforcing it.

Why bother?  I have my doubts physical i440fx works with less than 1M
either, given that this memory is needed to shadow the roms.  Possibly
you can't even find dimms that are small to plug them into such a system
to try ...

I'd say just add a hard limit and be done with it.

Maybe exclude isapc.  That one hasn't shadow support so things have at
least a chance to work with less than 1M of memory.  But honestly I'd
rather drop isapc, together with ia64 and sparc.  I mean, what is the
use case?  'pc' machine type is compatible enough with vga and ide ports
being on the standard isa locations so even msdos which has no pci
support at all boots happily.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v2 5/8] ppc/pnv: create the ICP and ICS objects under the machine

[Cédric Le Goater]

On 03/23/2017 05:16 AM, David Gibson wrote:
> On Thu, Mar 16, 2017 at 03:35:09PM +0100, Cédric Le Goater wrote:
>> Like this is done for the sPAPR machine, we use a simple array under
>> the PowerNV machine to store the Interrupt Control Presenters (ICP)
>> objects, one for each vCPU. This array is indexed by 'cpu_index' of
>> the CPUState but the users will provide a core PIR number. The mapping
>> is done in the icp_get() handler of the machine and is transparent to
>> XICS.
> 
> I note that you don't actually seem to be setting the cpu's icp backlink.

Indeed. The PowerNV cpu's ICP backlink is assigned in patch : 

[PATCH v2 7/8] ppc/pnv: link the CPUs to the machine XICSFabric

when the thread realize routine is called, like this is done for 
the sPAPR core. I did not use an object property link because it 
was adding complexity but I suppose it could be done that way if 
needed. 

Ideally, we could allocate the ICP object when the thread object 
is created but we would need to know which type to use, maybe 
through a new XICSFabric handler.

> 
>> We use a list to hold the different Interrupt Control Sources (ICS)
>> objects, as PowerNV needs to handle multiple sources: for PCI-E and
>> for the Processor Service Interface (PSI).
> 
> This is reasonable for now, but I wonder if we could use the fact we
> know the platform architecture to avoid having an explicit ics list.
> IIUC there's one ICS per chip (are there some extras as well?), so we
> could just iterate through the existing chip array, and check each
> one's ICS.

Each PHB object will bring two extra ICS objects (LSI and MSI) and 
there can be a few PHB per chip. But, yes, instead of using a list, 
we could also loop on the machine chips, checking PSI and the PHBs :

pnv->chips[i]->psi.ics
pnv->chips[i]->phb[j].lsi_ics
pnv->chips[i]->phb[j].msis

using QLIST_FOREACH() makes the loop real simple though. Another 
solution would be to use :

object_child_foreach_recursive(OBJECT(chip), ...);

but that would be slower.


>> Finally, to interface with the XICS layer which manipulates the ICP
>> and ICS objects, we extend the PowerNV machine with an XICSFabric
>> interface and its associated handlers.
>>
>> Signed-off-by: Cédric Le Goater 
>> ---
>>
>>  Changes since v1:
>>
>>  - handled pir-to-cpu_index mapping under icp_get 
>>  - removed ics_eio handler
>>  - changed ICP name indexing
>>  - removed sysbus parenting of the ICP object
>>
>>  hw/ppc/pnv.c  | 102 
>> ++
>>  include/hw/ppc/pnv.h  |   4 ++
>>  include/hw/ppc/xics.h |   1 +
>>  3 files changed, 107 insertions(+)
>>
>> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
>> index 3fa722af82e6..6ff01c3b84d5 100644
>> --- a/hw/ppc/pnv.c
>> +++ b/hw/ppc/pnv.c
>> @@ -33,7 +33,10 @@
>>  #include "exec/address-spaces.h"
>>  #include "qemu/cutils.h"
>>  #include "qapi/visitor.h"
>> +#include "monitor/monitor.h"
>> +#include "hw/intc/intc.h"
>>  
>> +#include "hw/ppc/xics.h"
>>  #include "hw/ppc/pnv_xscom.h"
>>  
>>  #include "hw/isa/isa.h"
>> @@ -417,6 +420,26 @@ static void ppc_powernv_init(MachineState *machine)
>>  machine->cpu_model = "POWER8";
>>  }
>>  
>> +/* Create the Interrupt Control Presenters before the vCPUs */
>> +pnv->nr_servers = pnv->num_chips * smp_cores * smp_threads;
>> +pnv->icps = g_new0(PnvICPState, pnv->nr_servers);
>> +for (i = 0; i < pnv->nr_servers; i++) {
>> +PnvICPState *icp = &pnv->icps[i];
>> +char name[32];
>> +
>> +/* TODO: fix ICP object name to be in sync with the core name */
>> +snprintf(name, sizeof(name), "icp[%d]", i);
>> +object_initialize(icp, sizeof(*icp), TYPE_PNV_ICP);
>> +object_property_add_child(OBJECT(pnv), name, OBJECT(icp),
>> +  &error_fatal);
>> +object_property_add_const_link(OBJECT(icp), "xics", OBJECT(pnv),
>> +   &error_fatal);
>> +object_property_set_bool(OBJECT(icp), true, "realized", 
>> &error_fatal);
>> +}
>> +
>> +/* and the list of Interrupt Control Sources */
>> +QLIST_INIT(&pnv->ics);
>> +
>>  /* Create the processor chips */
>>  chip_typename = g_strdup_printf(TYPE_PNV_CHIP "-%s", 
>> machine->cpu_model);
>>  if (!object_class_by_name(chip_typename)) {
>> @@ -743,6 +766,58 @@ static void pnv_get_num_chips(Object *obj, Visitor *v, 
>> const char *name,
>>  visit_type_uint32(v, name, &POWERNV_MACHINE(obj)->num_chips, errp);
>>  }
>>  
>> +static ICSState *pnv_ics_get(XICSFabric *xi, int irq)
>> +{
>> +PnvMachineState *pnv = POWERNV_MACHINE(xi);
>> +ICSState *ics;
>> +
>> +QLIST_FOREACH(ics, &pnv->ics, list) {
>> +if (ics_valid_irq(ics, irq)) {
>> +return ics;
>> +}
>> +}
>> +return NULL;
>> +}
>> +
>> +static void pnv_ics_resend(XICSFabric *xi)
>> +{
>> +PnvMachineState *pnv = POWERNV_MACHINE(xi);
>> +ICSSt

Re: [Qemu-devel] [Xen-devel] [PATCH v4 0/8] xen/9pfs: introduce the Xen 9pfs backend

[Greg Kurz]

On Wed, 22 Mar 2017 11:32:22 -0700 (PDT)
Stefano Stabellini  wrote:

> On Wed, 22 Mar 2017, Greg Kurz wrote:
> > On Tue, 21 Mar 2017 13:14:02 -0700 (PDT)
> > Stefano Stabellini  wrote:
> >   
> > > On Tue, 21 Mar 2017, Greg Kurz wrote:  
> > > > On Mon, 20 Mar 2017 11:18:46 -0700 (PDT)
> > > > Stefano Stabellini  wrote:
> > > > 
> > > > > Hi all,
> > > > > 
> > > > > This patch series implements a new transport for 9pfs, aimed at Xen
> > > > > systems.
> > > > > 
> > > > > The transport is based on a traditional Xen frontend and backend 
> > > > > drivers
> > > > > pair. This patch series implements the backend, which typically runs 
> > > > > in
> > > > > Dom0. I sent another series to implement the frontend in Linux
> > > > > (http://marc.info/?l=linux-kernel&m=148883047125960&w=2).
> > > > > 
> > > > > The backend complies to the Xen transport for 9pfs specification
> > > > > version 1, available here:
> > > > > 
> > > > > https://xenbits.xen.org/docs/unstable/misc/9pfs.html
> > > > > 
> > > > > 
> > > > > Changes in v4:
> > > > > - add reviewed-bys
> > > > > - remove useless if(NULL) checks around g_free
> > > > > - g_free g_malloc'ed sgs
> > > > > - remove XEN_9PFS_RING_ORDER, make the ring order dynamic per ring,
> > > > >   reading the ring_order field in xen_9pfs_data_intf
> > > > > - remove patch not to build Xen backends on non-Xen capable targets
> > > > >   because it is already upstream
> > > > > 
> > > > 
> > > > Hi Stefano,
> > > > 
> > > > This looks good to me. Do you want these patches to go through my 9p
> > > > tree or through your xen tree ?
> > > 
> > > Thanks Greg! It can work both ways. If you have any changes in your queue
> > > that could conflict with this, it's best to go via your tree.
> > > 
> > > Otherwise, I'll merge it in mine, so that I can keep an eye on the
> > > correspondent Xen changes to the header files and make sure they are in
> > > sync (specifically http://marc.info/?l=qemu-devel&m=149003412910278).
> > >   
> > 
> > I don't have any conflicting patches on my side. Please merge this in your
> > tree (as well as the MAINTAINERS patch).  
> 
> All right, thanks! I'll add a reviewed-by you on all patches if for you
> is OK.
> 

I'd prefer not as I haven't reviewed the Xen specific bits actually :)

> 
> > >   
> > > >  Also, I guess you may want to add
> > > > F: hw/9pfs/xen-9p-backend.c to the Xen section in MAINTAINERS.
> > > 
> > > I'll send a patch to be applied on top of the series
> > > 
> > >   
> > > > --
> > > > Greg
> > > > 
> > > > > Changes in v3:
> > > > > - do not build backends for targets that do not support xen
> > > > > - remove xen_9pfs.h, merge its content into xen-9p-backend.c
> > > > > - remove xen_9pfs_header, introduce P9MsgHeader
> > > > > - use le32_to_cpu to access P9MsgHeader fields
> > > > > - many coding style fixes
> > > > > - run checkpatch on all patches
> > > > > - add check if num_rings < 1
> > > > > - use g_strdup_printf
> > > > > - free fsdev_id in xen_9pfs_free
> > > > > - add comments
> > > > > 
> > > > > Changes in v2:
> > > > > - fix coding style
> > > > > - compile xen-9p-backend.c if CONFIG_XEN_BACKEND
> > > > > - add patch to set CONFIG_XEN_BACKEND only for the right targets
> > > > > - add review-bys
> > > > > 
> > > > > 
> > > > > Stefano Stabellini (8):
> > > > >   xen: import ring.h from xen
> > > > >   9p: introduce a type for the 9p header
> > > > >   xen/9pfs: introduce Xen 9pfs backend
> > > > >   xen/9pfs: connect to the frontend
> > > > >   xen/9pfs: receive requests from the frontend
> > > > >   xen/9pfs: implement in/out_iov_from_pdu and vmarshal/vunmarshal
> > > > >   xen/9pfs: send responses back to the frontend
> > > > >   xen/9pfs: build and register Xen 9pfs backend
> > > > > 
> > > > >  hw/9pfs/9p.h |   6 +
> > > > >  hw/9pfs/Makefile.objs|   1 +
> > > > >  hw/9pfs/virtio-9p-device.c   |   6 +-
> > > > >  hw/9pfs/xen-9p-backend.c | 444 
> > > > > +
> > > > >  hw/block/xen_blkif.h |   2 +-
> > > > >  hw/usb/xen-usb.c |   2 +-
> > > > >  hw/xen/xen_backend.c |   3 +
> > > > >  include/hw/xen/io/ring.h | 455 
> > > > > +++
> > > > >  include/hw/xen/xen_backend.h |   3 +
> > > > >  9 files changed, 915 insertions(+), 7 deletions(-)
> > > > >  create mode 100644 hw/9pfs/xen-9p-backend.c
> > > > >  create mode 100644 include/hw/xen/io/ring.h
> > > > 
> > > > 
> > 
> >   



pgpTSatU5ln3t.pgp
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] MAINTAINERS: update mail address for NVDIMM

[Stefan Hajnoczi]

On Tue, Mar 21, 2017 at 01:33:57PM +0800, Xiao Guangrong wrote:
> From: Xiao Guangrong 
> 
> My Intel mail account will be disabled soon, update the mail info
> to my private mail
> 
> Signed-off-by: Xiao Guangrong 
> ---
>  MAINTAINERS | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [Qemu-devel] [Qemu-trivial] [PATCH v5] util: Use g_malloc/g_free in envlist.c

[Stefan Hajnoczi]

On Mon, Mar 20, 2017 at 05:38:28PM +, Saurav Sachidanand wrote:
> Change malloc/strdup/free to g_malloc/g_strdup/g_free in
> util/envlist.c.
> 
> Remove NULL checks for pointers returned from g_malloc and g_strdup
> as they exit in case of failure. Also, update calls to envlist_create
> to reflect this.
> 
> Free array and array contents returned by envlist_to_environ using
> g_free in bsd-user/main.c and linux-user/main.c.
> 
> Update comments to reflect change in semantics.
> 
> Signed-off-by: Saurav Sachidanand 
> ---
>  bsd-user/main.c   | 14 --
>  linux-user/main.c |  9 +++--
>  util/envlist.c| 47 +++
>  3 files changed, 26 insertions(+), 44 deletions(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH] MAINTAINERS: update mail address for NVDIMM

[Stefan Hajnoczi]

On Tue, Mar 21, 2017 at 01:33:57PM +0800, Xiao Guangrong wrote:
> My Intel mail account will be disabled soon, update the mail info
> to my private mail

Is someone else taking over QEMU NVDIMM development work at Intel?

Thanks,
Stefan


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH for-2.10 05/23] numa: move source of default CPUs to NUMA node mapping into boards

[Igor Mammedov]

On Thu, 23 Mar 2017 11:40:29 +0530
Bharata B Rao  wrote:

> On Wed, Mar 22, 2017 at 7:02 PM, Igor Mammedov  wrote:
> 
> > diff --git a/numa.c b/numa.c
> > index e01cb54..b6e71bc 100644
> > --- a/numa.c
> > +++ b/numa.c
> > @@ -294,9 +294,10 @@ static void validate_numa_cpus(void)
> >  g_free(seen_cpus);
> >  }
> >
> > -void parse_numa_opts(MachineClass *mc)
> > +void parse_numa_opts(MachineState *ms)
> >  {
> >  int i;
> > +MachineClass *mc = MACHINE_GET_CLASS(ms);
> >
> >  for (i = 0; i < MAX_NODES; i++) {
> >  numa_info[i].node_cpu = bitmap_new(max_cpus);
> > @@ -378,14 +379,16 @@ void parse_numa_opts(MachineClass *mc)
> >   * rule grouping VCPUs by socket so that VCPUs from the same
> > socket
> >   * would be on the same node.
> >   */
> > +if (!mc->cpu_index_to_instance_props) {
> > +error_report("default CPUs to NUMA node mapping isn't
> > supported");
> > +exit(1);
> > +}
> >  
> 
> Just trying to understand the impact of the above enforcement. So targets
> and machine types that don't define ->cpu_index_to_instance_props() are
> expected not to boot ? Shouldn't they have a default to fall back upon ?
Currently there are 3 boards that support numa and with this series
they all implement cpu_index_to_instance_props callback,
so boards that has supported numa shouldn't be affected.

But if someone used '-numa' with a board that doesn't support numa,
it would stop booting with error message instead of silently parsing
not supported option or falling back bogus defaults (which aren't
used anyway).


> Regards,
> Bharata.

[Qemu-devel] [PATCH] 9pfs: fix file descriptor leak

[Li Qiang]

In v9fs_create/lcreate dispatch handler, the fidp's fid_type is not checked
before used. As these function will set the fid_type, if the guest call
more than once them, it will leak the fidp. This can cause some other
issue, such as memory leak. Check the fid_type before using them.

Signed-off-by: Li Qiang 
---
 hw/9pfs/9p.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index b8c0b99..48babce 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -1550,6 +1550,10 @@ static void coroutine_fn v9fs_lcreate(void *opaque)
 err = -ENOENT;
 goto out_nofid;
 }
+if (fidp->fid_type != P9_FID_NONE) {
+err = -EINVAL;
+goto out;
+}
 
 flags = get_dotl_openflags(pdu->s, flags);
 err = v9fs_co_open2(pdu, fidp, &name, gid,
@@ -2153,6 +2157,10 @@ static void coroutine_fn v9fs_create(void *opaque)
 err = -EINVAL;
 goto out_nofid;
 }
+if (fidp->fid_type != P9_FID_NONE) {
+err = -EINVAL;
+goto out;
+}
 if (perm & P9_STAT_MODE_DIR) {
 err = v9fs_co_mkdir(pdu, fidp, &name, perm & 0777,
 fidp->uid, -1, &stbuf);
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 1/3] libqtest: Ignore QMP events when parsing the response for HMP commands

[Stefan Hajnoczi]

On Tue, Mar 21, 2017 at 11:39:50AM +0100, Thomas Huth wrote:
> When running certain HMP commands (like "device_del") via QMP, we
> can sometimes get a QMP event in the response first, so that the
> "g_assert(ret)" statement in qtest_hmp() triggers and the test
> fails. So ignore such QMP events when looking for the real
> return value from QMP.
> 
> Signed-off-by: Thomas Huth 
> ---
>  tests/libqtest.c | 6 ++
>  1 file changed, 6 insertions(+)

qmp.py keeps a queue of events so they can be processed later.  I guess
an event queue will be needed eventually because discarding events makes
it hard to write reliable test cases that check for events.

But as long as current qtests work correctly:

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH 2/3] libqtest: Add a generic function to run a callback function for every machine

[Stefan Hajnoczi]

On Tue, Mar 21, 2017 at 11:39:51AM +0100, Thomas Huth wrote:
> Some tests need to run single tests for every available machine of the
> current QEMU binary. To avoid code duplication, let's extract this
> code that deals with 'query-machines' into a separate function.
> 
> Signed-off-by: Thomas Huth 
> ---
>  tests/libqtest.c| 30 +
>  tests/libqtest.h|  8 +
>  tests/pc-cpu-test.c | 95 
> -
>  tests/qom-test.c| 36 
>  4 files changed, 80 insertions(+), 89 deletions(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH 3/3] tests: Add a tester for HMP commands

[Stefan Hajnoczi]

On Tue, Mar 21, 2017 at 11:39:52AM +0100, Thomas Huth wrote:
> HMP commands do not get any automatic testing yet, so on certain
> QEMU machines, some HMP commands were causing crashes in the past.
> Thus we should test HMP commands in our test suite, too, to avoid
> that such problems creep in again in the future.
> 
> Signed-off-by: Thomas Huth 
> ---
>  tests/Makefile.include |   2 +
>  tests/test-hmp.c   | 160 
> +
>  2 files changed, 162 insertions(+)
>  create mode 100644 tests/test-hmp.c

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH] configure: Fix cut-n-paste errors in OS deprecation warning

[Stefan Hajnoczi]

On Tue, Mar 21, 2017 at 06:08:49PM +, Peter Maydell wrote:
> Fix some cut-and-paste errors in the OS deprecation warning
> pointed out by Thomas Huth.
> 
> Reported-by: Thomas Huth 
> Signed-off-by: Peter Maydell 
> ---
>  configure | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH 1/3] libqtest: Ignore QMP events when parsing the response for HMP commands

[Thomas Huth]

On 23.03.2017 09:52, Stefan Hajnoczi wrote:
> On Tue, Mar 21, 2017 at 11:39:50AM +0100, Thomas Huth wrote:
>> When running certain HMP commands (like "device_del") via QMP, we
>> can sometimes get a QMP event in the response first, so that the
>> "g_assert(ret)" statement in qtest_hmp() triggers and the test
>> fails. So ignore such QMP events when looking for the real
>> return value from QMP.
>>
>> Signed-off-by: Thomas Huth 
>> ---
>>  tests/libqtest.c | 6 ++
>>  1 file changed, 6 insertions(+)
> 
> qmp.py keeps a queue of events so they can be processed later.  I guess
> an event queue will be needed eventually because discarding events makes
> it hard to write reliable test cases that check for events.

Yes, but in that case the test likely should not use the hmp() functions
at all and use qmp directly.

> But as long as current qtests work correctly:
> 
> Reviewed-by: Stefan Hajnoczi 

Thanks!

 Thomas





signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH v6] vfio error recovery: kernel support

[Cao jin]

From: "Michael S. Tsirkin" 

0. What happens now (PCIE AER only)
   Fatal errors cause a link reset. Non fatal errors don't.
   All errors stop the QEMU guest eventually, but not immediately,
   because it's detected and reported asynchronously.
   Interrupts are forwarded as usual.
   Correctable errors are not reported to user at all.

   Note:
   PPC EEH is different, but this approach won't affect EEH. EEH treat
   all errors as fatal ones in AER, so they will still be signalled to user
   via the legacy eventfd.  Besides, all devices/functions in a PE belongs
   to the same IOMMU group, so the slot_reset handler in this approach
   won't affect EEH either.

1. Correctable errors
   Hardware can correct these errors without software intervention,
   clear the error status is enough, this is what already done now.
   No need to recover it, nothing changed, leave it as it is.

2. Fatal errors
   They will induce a link reset. This is troublesome when user is
   a QEMU guest. This approach doesn't touch the existing mechanism.

3. Non-fatal errors
   Before this patch, they are signalled to user the same way as fatal ones.
   With this patch, a new eventfd is introduced only for non-fatal error
   notification. By splitting non-fatal ones out, it will benefit AER
   recovery of a QEMU guest user.

   To maintain backwards compatibility with userspace, non-fatal errors
   will continue to trigger via the existing error interrupt index if a
   non-fatal signaling mechanism has not been registered.

   Note:
   In case of PCI Express errors, kernel might request a slot reset
   affecting our device (from our point of view this is a passive device
   reset as opposed to an active one requested by vfio itself).
   This might currently happen if a slot reset is requested by a driver
   (other than vfio) bound to another device function in the same slot.
   This will cause our device to lose its state so report this event to
   userspace.

Signed-off-by: Michael S. Tsirkin 
Signed-off-by: Cao jin 
---
v6 changelog:
Address all the comments from MST.

 drivers/vfio/pci/vfio_pci.c | 49 +++--
 drivers/vfio/pci/vfio_pci_intrs.c   | 38 
 drivers/vfio/pci/vfio_pci_private.h |  2 ++
 include/uapi/linux/vfio.h   |  2 ++
 4 files changed, 89 insertions(+), 2 deletions(-)

diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index 324c52e..71f9a8a 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -441,7 +441,9 @@ static int vfio_pci_get_irq_count(struct vfio_pci_device 
*vdev, int irq_type)
 
return (flags & PCI_MSIX_FLAGS_QSIZE) + 1;
}
-   } else if (irq_type == VFIO_PCI_ERR_IRQ_INDEX) {
+   } else if (irq_type == VFIO_PCI_ERR_IRQ_INDEX ||
+  irq_type == VFIO_PCI_NON_FATAL_ERR_IRQ_INDEX ||
+  irq_type == VFIO_PCI_PASSIVE_RESET_IRQ_INDEX) {
if (pci_is_pcie(vdev->pdev))
return 1;
} else if (irq_type == VFIO_PCI_REQ_IRQ_INDEX) {
@@ -796,6 +798,8 @@ static long vfio_pci_ioctl(void *device_data,
case VFIO_PCI_REQ_IRQ_INDEX:
break;
case VFIO_PCI_ERR_IRQ_INDEX:
+   case VFIO_PCI_NON_FATAL_ERR_IRQ_INDEX:
+   case VFIO_PCI_PASSIVE_RESET_IRQ_INDEX:
if (pci_is_pcie(vdev->pdev))
break;
/* pass thru to return error */
@@ -1282,7 +1286,9 @@ static pci_ers_result_t vfio_pci_aer_err_detected(struct 
pci_dev *pdev,
 
mutex_lock(&vdev->igate);
 
-   if (vdev->err_trigger)
+   if (state == pci_channel_io_normal && vdev->non_fatal_err_trigger)
+   eventfd_signal(vdev->non_fatal_err_trigger, 1);
+   else if (vdev->err_trigger)
eventfd_signal(vdev->err_trigger, 1);
 
mutex_unlock(&vdev->igate);
@@ -1292,8 +1298,47 @@ static pci_ers_result_t vfio_pci_aer_err_detected(struct 
pci_dev *pdev,
return PCI_ERS_RESULT_CAN_RECOVER;
 }
 
+/*
+ * In case of PCI Express errors, kernel might request a slot reset
+ * affecting our device (from our point of view, this is a passive device
+ * reset as opposed to an active one requested by vfio itself).
+ * This might currently happen if a slot reset is requested by a driver
+ * (other than vfio) bound to another device function in the same slot.
+ * This will cause our device to lose its state, so report this event to
+ * userspace.
+ */
+static pci_ers_result_t vfio_pci_aer_slot_reset(struct pci_dev *pdev)
+{
+   struct vfio_pci_device *vdev;
+   struct vfio_device *device;
+   static pci_ers_result_t err = PCI_ERS_RESULT_NONE;
+
+   device = vfio_device_get_from_dev(&pdev->dev);
+   if (!device)
+   goto err_dev;
+
+   vdev = vfio_device_data(device);
+   if (!vdev)
+   goto err_data;
+
+   mut

[Qemu-devel] [PATCH v3 1/3] pcie aer: verify if AER functionality is available

[Cao jin]

For devices which support AER function, verify it can work or not in the
system:
1. AER capable device is a PCIe device, it can't be plugged into PCI bus
2. If root port doesn't support AER, then there is no need to expose the
   AER capability

Signed-off-by: Dou Liyang 
Signed-off-by: Cao jin 
---
 hw/pci/pcie_aer.c | 28 
 1 file changed, 28 insertions(+)

diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
index daf1f65..a2e9818 100644
--- a/hw/pci/pcie_aer.c
+++ b/hw/pci/pcie_aer.c
@@ -100,6 +100,34 @@ static void aer_log_clear_all_err(PCIEAERLog *aer_log)
 int pcie_aer_init(PCIDevice *dev, uint8_t cap_ver, uint16_t offset,
   uint16_t size, Error **errp)
 {
+PCIDevice *parent_dev;
+uint8_t type;
+uint8_t parent_type;
+
+/* Topology test: see if there is need to expose AER cap */
+type = pcie_cap_get_type(dev);
+parent_dev = pci_bridge_get_device(dev->bus);
+while (parent_dev) {
+parent_type = pcie_cap_get_type(parent_dev);
+
+if (type == PCI_EXP_TYPE_ENDPOINT &&
+(parent_type != PCI_EXP_TYPE_ROOT_PORT &&
+ parent_type != PCI_EXP_TYPE_DOWNSTREAM)) {
+error_setg(errp, "Parent device is not a PCIe component");
+return -ENOTSUP;
+}
+
+if (parent_type == PCI_EXP_TYPE_ROOT_PORT) {
+if (!parent_dev->exp.aer_cap)
+{
+error_setg(errp, "Root port does not support AER");
+return -ENOTSUP;
+}
+}
+
+parent_dev = pci_bridge_get_device(parent_dev->bus);
+}
+
 pcie_add_capability(dev, PCI_EXT_CAP_ID_ERR, cap_ver,
 offset, size);
 dev->exp.aer_cap = offset;
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 3/3] vfio-pci: process non fatal error of AER

[Cao jin]

Make use of the non fatal error eventfd that the kernel module provide
to process the AER non fatal error. Fatal error still goes into the
legacy way which results in VM stop.

Register the handler, wait for notification. Construct aer message and
pass it to root port on notification. Root port will trigger an interrupt
to signal guest, then guest driver will do the recovery.

Signed-off-by: Dou Liyang 
Signed-off-by: Cao jin 
---
 hw/vfio/pci.c  | 202 +
 hw/vfio/pci.h  |   2 +
 linux-headers/linux/vfio.h |   2 +
 3 files changed, 206 insertions(+)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 3d0d005..c6786d5 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2432,6 +2432,200 @@ static void vfio_put_device(VFIOPCIDevice *vdev)
 vfio_put_base_device(&vdev->vbasedev);
 }
 
+static void vfio_non_fatal_err_notifier_handler(void *opaque)
+{
+VFIOPCIDevice *vdev = opaque;
+PCIDevice *dev = &vdev->pdev;
+PCIEAERMsg msg = {
+.severity = PCI_ERR_ROOT_CMD_NONFATAL_EN,
+.source_id = pci_requester_id(dev),
+};
+
+if (!event_notifier_test_and_clear(&vdev->non_fatal_err_notifier)) {
+return;
+}
+
+/* Populate the aer msg and send it to root port */
+if (dev->exp.aer_cap) {
+uint8_t *aer_cap = dev->config + dev->exp.aer_cap;
+uint32_t uncor_status;
+bool isfatal;
+
+uncor_status = vfio_pci_read_config(dev,
+dev->exp.aer_cap + PCI_ERR_UNCOR_STATUS, 4);
+if (!uncor_status) {
+return;
+}
+
+isfatal = uncor_status & pci_get_long(aer_cap + PCI_ERR_UNCOR_SEVER);
+if (isfatal) {
+goto stop;
+}
+
+error_report("%s sending non fatal event to root port. uncor status = "
+ "0x%"PRIx32, vdev->vbasedev.name, uncor_status);
+pcie_aer_msg(dev, &msg);
+return;
+}
+
+stop:
+/* Terminate the guest in case of fatal error */
+error_report("%s: Device detected a fatal error. VM stopped",
+   vdev->vbasedev.name);
+vm_stop(RUN_STATE_INTERNAL_ERROR);
+}
+
+/*
+ * Register non fatal error notifier for devices supporting error recovery.
+ * If we encounter a failure in this function, we report an error
+ * and continue after disabling error recovery support for the device.
+ */
+static void vfio_register_non_fatal_err_notifier(VFIOPCIDevice *vdev)
+{
+int ret;
+int argsz;
+struct vfio_irq_set *irq_set;
+int32_t *pfd;
+
+if (event_notifier_init(&vdev->non_fatal_err_notifier, 0)) {
+error_report("vfio: Unable to init event notifier for non-fatal error 
detection");
+return;
+}
+
+argsz = sizeof(*irq_set) + sizeof(*pfd);
+
+irq_set = g_malloc0(argsz);
+irq_set->argsz = argsz;
+irq_set->flags = VFIO_IRQ_SET_DATA_EVENTFD |
+ VFIO_IRQ_SET_ACTION_TRIGGER;
+irq_set->index = VFIO_PCI_NON_FATAL_ERR_IRQ_INDEX;
+irq_set->start = 0;
+irq_set->count = 1;
+pfd = (int32_t *)&irq_set->data;
+
+*pfd = event_notifier_get_fd(&vdev->non_fatal_err_notifier);
+qemu_set_fd_handler(*pfd, vfio_non_fatal_err_notifier_handler, NULL, vdev);
+
+ret = ioctl(vdev->vbasedev.fd, VFIO_DEVICE_SET_IRQS, irq_set);
+if (ret) {
+error_report("vfio: Failed to set up non-fatal error notification: 
%m");
+qemu_set_fd_handler(*pfd, NULL, NULL, vdev);
+event_notifier_cleanup(&vdev->non_fatal_err_notifier);
+}
+g_free(irq_set);
+}
+
+static void vfio_unregister_non_fatal_err_notifier(VFIOPCIDevice *vdev)
+{
+int argsz;
+struct vfio_irq_set *irq_set;
+int32_t *pfd;
+int ret;
+
+argsz = sizeof(*irq_set) + sizeof(*pfd);
+
+irq_set = g_malloc0(argsz);
+irq_set->argsz = argsz;
+irq_set->flags = VFIO_IRQ_SET_DATA_EVENTFD |
+ VFIO_IRQ_SET_ACTION_TRIGGER;
+irq_set->index = VFIO_PCI_NON_FATAL_ERR_IRQ_INDEX;
+irq_set->start = 0;
+irq_set->count = 1;
+pfd = (int32_t *)&irq_set->data;
+*pfd = -1;
+
+ret = ioctl(vdev->vbasedev.fd, VFIO_DEVICE_SET_IRQS, irq_set);
+if (ret) {
+error_report("vfio: Failed to de-assign error fd: %m");
+}
+g_free(irq_set);
+qemu_set_fd_handler(event_notifier_get_fd(&vdev->non_fatal_err_notifier),
+NULL, NULL, vdev);
+event_notifier_cleanup(&vdev->non_fatal_err_notifier);
+}
+
+static void vfio_passive_reset_notifier_handler(void *opaque)
+{
+VFIOPCIDevice *vdev = opaque;
+
+if (!event_notifier_test_and_clear(&vdev->passive_reset_notifier)) {
+return;
+}
+
+error_report("%s: Device lost state due to host device reset. VM stopped",
+   vdev->vbasedev.name);
+vm_stop(RUN_STATE_INTERNAL_ERROR);
+}
+
+/*
+ * Register passive reset notifier, in case of certain function of a
+ * multifunction device is passthroughed,  while other functions are still
+ * controlled b

[Qemu-devel] [PATCH v3 0/3] vfio-pci: support recovery of AER non fatal error

[Cao jin]

v3 changelog:
1. Address all comments from MST in patch 3, include remove the flag
   pci_aer_non_fatal & passive_reset, also the boilerplate code.
   The corresponding kernel patch is v6.

Test:
Test with func1 passthroughed while func0 doesn't have user.

Cao jin (3):
  pcie aer: verify if AER functionality is available
  vfio pci: new function to init AER capability
  vfio-pci: process non fatal error of AER

 hw/pci/pcie_aer.c  |  28 ++
 hw/vfio/pci.c  | 243 -
 hw/vfio/pci.h  |   3 +
 linux-headers/linux/vfio.h |   2 +
 4 files changed, 271 insertions(+), 5 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH v3 2/3] vfio pci: new function to init AER capability

[Cao jin]

Signed-off-by: Dou Liyang 
Signed-off-by: Cao jin 
---
 hw/vfio/pci.c | 41 -
 hw/vfio/pci.h |  1 +
 2 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 332f41d..3d0d005 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -1855,18 +1855,42 @@ out:
 return 0;
 }
 
-static void vfio_add_ext_cap(VFIOPCIDevice *vdev)
+static int vfio_setup_aer(VFIOPCIDevice *vdev, uint8_t cap_ver,
+  int pos, uint16_t size, Error **errp)
+{
+PCIDevice *pdev = &vdev->pdev;
+uint32_t errcap;
+
+errcap = vfio_pci_read_config(pdev, pos + PCI_ERR_CAP, 4);
+/*
+ * The ability to record multiple headers is depending on
+ * the state of the Multiple Header Recording Capable bit and
+ * enabled by the Multiple Header Recording Enable bit.
+ */
+if ((errcap & PCI_ERR_CAP_MHRC) &&
+(errcap & PCI_ERR_CAP_MHRE)) {
+pdev->exp.aer_log.log_max = PCIE_AER_LOG_MAX_DEFAULT;
+} else {
+pdev->exp.aer_log.log_max = 0;
+}
+
+pcie_cap_deverr_init(pdev);
+return pcie_aer_init(pdev, cap_ver, pos, size, errp);
+}
+
+static int vfio_add_ext_cap(VFIOPCIDevice *vdev, Error **errp)
 {
 PCIDevice *pdev = &vdev->pdev;
 uint32_t header;
 uint16_t cap_id, next, size;
 uint8_t cap_ver;
 uint8_t *config;
+int ret = 0;
 
 /* Only add extended caps if we have them and the guest can see them */
 if (!pci_is_express(pdev) || !pci_bus_is_express(pdev->bus) ||
 !pci_get_long(pdev->config + PCI_CONFIG_SPACE_SIZE)) {
-return;
+return 0;
 }
 
 /*
@@ -1915,6 +1939,9 @@ static void vfio_add_ext_cap(VFIOPCIDevice *vdev)
PCI_EXT_CAP_NEXT_MASK);
 
 switch (cap_id) {
+case PCI_EXT_CAP_ID_ERR:
+ret = vfio_setup_aer(vdev, cap_ver, next, size, errp);
+break;
 case PCI_EXT_CAP_ID_SRIOV: /* Read-only VF BARs confuse OVMF */
 case PCI_EXT_CAP_ID_ARI: /* XXX Needs next function virtualization */
 trace_vfio_add_ext_cap_dropped(vdev->vbasedev.name, cap_id, next);
@@ -1923,6 +1950,9 @@ static void vfio_add_ext_cap(VFIOPCIDevice *vdev)
 pcie_add_capability(pdev, cap_id, cap_ver, next, size);
 }
 
+if (ret) {
+goto out;
+}
 }
 
 /* Cleanup chain head ID if necessary */
@@ -1930,8 +1960,9 @@ static void vfio_add_ext_cap(VFIOPCIDevice *vdev)
 pci_set_word(pdev->config + PCI_CONFIG_SPACE_SIZE, 0);
 }
 
+out:
 g_free(config);
-return;
+return ret;
 }
 
 static int vfio_add_capabilities(VFIOPCIDevice *vdev, Error **errp)
@@ -1949,8 +1980,8 @@ static int vfio_add_capabilities(VFIOPCIDevice *vdev, 
Error **errp)
 return ret;
 }
 
-vfio_add_ext_cap(vdev);
-return 0;
+ret = vfio_add_ext_cap(vdev, errp);
+return ret;
 }
 
 static void vfio_pci_pre_reset(VFIOPCIDevice *vdev)
diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
index a8366bb..34e8b04 100644
--- a/hw/vfio/pci.h
+++ b/hw/vfio/pci.h
@@ -15,6 +15,7 @@
 #include "qemu-common.h"
 #include "exec/memory.h"
 #include "hw/pci/pci.h"
+#include "hw/pci/pci_bridge.h"
 #include "hw/vfio/vfio-common.h"
 #include "qemu/event_notifier.h"
 #include "qemu/queue.h"
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH] only link current target arch traces to qemu-system

[Stefan Hajnoczi]

On Wed, Mar 22, 2017 at 02:37:21PM +0100, Paolo Bonzini wrote:
> 
> 
> On 22/03/2017 03:03, Xu, Anthony wrote:
> > When building target x86_64-softmmu, all other architectures' trace.o are 
> > linked into 
> > x86_64-softmmu/qemu-system-x86_64, like hw/arm/trace.o, hw/mips/trace.o 
> > etc., 
> > that is not necessary.
> >  Same thing happens when building other targets.
> > 
> > Only current target arch traces should be linked into qemu-system.
> > 
> > Signed-off -by: Anthony Xu 
> 
> It's a bit cleaner, but does the benefit outweight the maintenance cost
> of the additional code added to the Makefiles?

Perhaps all trace.o files should be put into their own .a instead of
being added directly to the linker line:

COMMON_LDADDS = $(trace-obj-y) libqemuutil.a libqemustub.a

I think the linker would only pull in .o files containing symbols that are
actually referenced by the program.

It would look like this:

##
# Build libraries

libqemustub.a: $(stub-obj-y)
libqemuutil.a: $(util-obj-y)
libqemutrace.a: $(trace-obj-y)

##

COMMON_LDADDS = libqemuutil.a libqemustub.a libqemutrace.a

This eliminates the maintenance burden with Anthony's patch.

Stefan


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH] MAINTAINERS: update mail address for NVDIMM

[Xiao Guangrong]

Sure, Haozhong will take the work for the new requirements raised from
Intel.

2017-03-23 16:39 GMT+08:00 Stefan Hajnoczi :

> On Tue, Mar 21, 2017 at 01:33:57PM +0800, Xiao Guangrong wrote:
> > My Intel mail account will be disabled soon, update the mail info
> > to my private mail
>
> Is someone else taking over QEMU NVDIMM development work at Intel?
>
> Thanks,
> Stefan
>

Re: [Qemu-devel] [PATCH] e1000: disable debug by default

[Stefan Hajnoczi]

On Wed, Mar 22, 2017 at 11:07:32AM +0800, Jason Wang wrote:
> Disable debug output by default, the information were not needed for
> release.
> 
> Cc: Peter Maydell 
> Cc: Stefan Hajnoczi 
> Cc: Leonid Bloch 
> Cc: Dmitry Fleytman 
> Cc: qemu-sta...@nongnu.org
> Signed-off-by: Jason Wang 
> ---
>  hw/net/e1000.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH] doc: fix function spelling

[Stefan Hajnoczi]

On Wed, Mar 22, 2017 at 03:52:41PM +0400, Marc-André Lureau wrote:
> Signed-off-by: Marc-André Lureau 
> ---
>  include/io/channel.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [Qemu-devel] [RFC][PATCH 0/6] "bootonceindex" property

[Laszlo Ersek]

On 03/23/17 07:53, Janne Huttunen wrote:
> On Wed, 2017-03-22 at 16:29 +0100, Laszlo Ersek wrote:
>>> I'm not. I'm using QMP to change the index dynamically.
>>>  
>> Wait, if you are already changing the "bootindex" property
>> dynamically (do I understand that right?)
> 
> No, I'm not changing "bootindex" dynamically. I'm changing
> "bootonceindex" dynamically. The point is that whatever
> change I'm making is supposed to affect only one boot, the
> next one. Since the guest can trigger reboots by itself,
> I don't necessarily know when they are going to happen.
> 
> Like I said earlier, I can get very close to the semantics
> I need if set the "bootindex" and get an event when the
> boot happens so that I know when to reset the bootindex
> back to the original value. However doing it like that
> is (at least in theory) racy if the event isn't synchronous
> and it requires some process that actively monitors those
> events which I'm trying to avoid.
> 
>> ...and it could have a significant maintenance footprint,
>> while the feature does look niche (to me anyway).
> 
> Whatever I'm currently doing is definitely a niche, but very
> similar setting of a one time boot source while the system
> is running is e.g. part of the IPMI standard (see "Set
> System Boot Options Command" in IPMI Specification), so
> the concept itself is not that much of a niche.
> 

... Okay, I've spoken my mind on this, and have nothing more to add. I'm
still not convinced, but that doesn't mean you can't convince others.
(And you need to convince others more than me, because I'm not a QEMU
maintainer -- I just wanted to voice my opinion, since this was an RFC.)

What really matters to me though is that the "bootorder" fw_cfg file
preserve
- both its current, exclusive role, for communicating the boot order to
the firmware,
- and its syntax & semantics.

Thanks
Laszlo

Re: [Qemu-devel] Minimum RAM size for PC machines?

[David Hildenbrand]

On 23.03.2017 09:19, Gerd Hoffmann wrote:
> On Mi, 2017-03-22 at 11:19 +0100, David Hildenbrand wrote:
>> On 22.03.2017 11:03, Thomas Huth wrote:
>>> On 22.03.2017 10:08, Markus Armbruster wrote:
>>> [...]
 Are we now ready to accept a simple & stupid patch that actually helps
 users, say letting boards that care declare minimum and maximum RAM
 size?  And make PC reject RAM size less than 1MiB, even though "someone"
 might conceivably have firmware that works with less?
>>>
>>> I'd say enforce a minimum RAM size on the normal "pc" and "q35" machine,
>>> but still allow smaller sizes on the "isapc" machine. So if "someone"
>>> comes around and claims to have a legacy firmware that wants less memory
>>> than 1MiB, just point them to the isapc machine.
>>> Just my 0.02 €.
>>>
>>>  Thomas
>>
>> Or maybe simply warn the user that things may go wrong instead of
>> enforcing it.
> 
> Why bother?  I have my doubts physical i440fx works with less than 1M
> either, given that this memory is needed to shadow the roms.  Possibly
> you can't even find dimms that are small to plug them into such a system
> to try ...

Because it seems to work if you supply the correct rom. We are trying to
catch user errors, don't we?

> 
> I'd say just add a hard limit and be done with it.

"640K ought to be enough for anybody". Any limit we set will become out
of date.

> 
> Maybe exclude isapc.  That one hasn't shadow support so things have at
> least a chance to work with less than 1M of memory.  But honestly I'd
> rather drop isapc, together with ia64 and sparc.  I mean, what is the
> use case?  'pc' machine type is compatible enough with vga and ide ports
> being on the standard isa locations so even msdos which has no pci
> support at all boots happily.

I think I like that idea.

> cheers,
>   Gerd
> 


-- 

Thanks,

David

[Qemu-devel] [PATCH qemu-ga v2] qga: Make qemu-ga compile statically for Windows

[Sameeh Jubran]

Attempting to compile qemu-ga statically as follows for Windows causes
the following error:

Compilation:
./configure --disable-docs --target-list=x86_64-softmmu \
--cross-prefix=x86_64-w64-mingw32- --static \
--enable-guest-agent-msi --with-vss-sdk=/path/to/VSSSDK72

make -j8 qemu-ga

Error:
path/to/qemu/stubs/error-printf.c:7: undefined reference to 
`__imp_g_test_config_vars'
collect2: error: ld returned 1 exit status
Makefile:444: recipe for target 'qemu-ga.exe' failed
make: *** [qemu-ga.exe] Error 1

This is caused by a bug in the pkg-config file for glib as it doesn't define
GLIB_STATIC_COMPILATION for pkg-config --static.

Signed-off-by: Stefan Hajnoczi 
Signed-off-by: Sameeh Jubran 
---
 configure | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/configure b/configure
index b9a30cf..8210494 100755
--- a/configure
+++ b/configure
@@ -3041,6 +3041,13 @@ if test "$modules" = yes; then
 glib_modules="$glib_modules gmodule-2.0"
 fi
 
+# This workaround is required due to a bug in pkg-config file for glib as it
+# doesn't define GLIB_STATIC_COMPILATION for pkg-config --static
+
+if test "$static" = yes -a "$mingw32" = yes; then
+QEMU_CFLAGS="-DGLIB_STATIC_COMPILATION $QEMU_CFLAGS"
+fi
+
 for i in $glib_modules; do
 if $pkg_config --atleast-version=$glib_req_ver $i; then
 glib_cflags=$($pkg_config --cflags $i)
-- 
2.9.3

Re: [Qemu-devel] rawhide gcc failures [was: Proposal for deprecating unsupported host OSes & architecutures]

[Alex Bennée]

Peter Maydell  writes:

> On 22 March 2017 at 19:07, Eric Blake  wrote:
>> Tangentially-related: do we officially support bleeding-edge OS builds?
>> For example, current rawhide has a new-enough gcc that gives some
>> (possibly-useful) new warnings (-Werror=format-truncation)
>
> Depends what you mean by "support". Are we testing them
> in our build/CI loop? Apparently not :-)

We test some. The Travis loop currently installs the toolchain PPAs for
two of its runs to use:

- COMPILER_NAME=clang CXX=clang++-3.9 CC=clang-3.9
- COMPILER_NAME=gcc CXX=g++-5 CC=gcc-5

That said it probably makes more sense to add a rawhide docker image for
testing. Whether it is worth running it on every patch series remains to
be seen as while it might give some advanced warning could it also be
because the bleeding edge distro is broken in some other way?

> Do we fix them?
> Yep, certainly, because bleeding-edge libraries and
> compilers will eventually be standard ones. Do we fix
> them during the release process? Yes, but maybe if
> the fix is very invasive somehow we might prefer to
> postpone to the next release. Do we consider them
> a release-critical bug that we'd spin an extra rc
> for? I don't think so, because (a) for released tarballs
> -Werror is not on by default (b) even for building from
> git you can work around this with -disable-werror
> (c) it's just an accident of timing if a new gcc drops
> 3 weeks before release rather than 3 weeks after, so
> it's always possible that releases will end up being
> built on compilers that warn about things in them.
>
> More generally, I'm not going to go so far as to insist
> that we have a build machine for every flavour of every
> distro -- that would not usefully improve coverage I think.
> Also I'm happy to be ad-hoc about exactly what we test
> (my x86 Linux builds are just "the Ubuntu flavour my
> desktop box happens to be running right now") because
> being less ad-hoc feels like it would be work that
> we don't really need to engage in.
>
> thanks
> -- PMM


--
Alex Bennée

Re: [Qemu-devel] [PATCH v3] Allow setting NUMA distance for different NUMA nodes

[Igor Mammedov]

On Wed, 22 Mar 2017 17:32:46 +0800
He Chen  wrote:

> Current, QEMU does not provide a clear command to set vNUMA distance for
> guest although we already have `-numa` command to set vNUMA nodes.
> 
> vNUMA distance makes sense in certain scenario.
> But now, if we create a guest that has 4 vNUMA nodes, when we check NUMA
> info via `numactl -H`, we will see:
> 
> node distance:
> node0123
>   0:   10   20   20   20
>   1:   20   10   20   20
>   2:   20   20   10   20
>   3:   20   20   20   10
> 
> Guest kernel regards all local node as distance 10, and all remote node
> as distance 20 when there is no SLIT table since QEMU doesn't build it.
> It looks like a little strange when you have seen the distance in an
> actual physical machine that contains 4 NUMA nodes. My machine shows:
> 
> node distance:
> node0123
>   0:   10   21   31   41
>   1:   21   10   21   31
>   2:   31   21   10   21
>   3:   41   31   21   10
> 
> To set vNUMA distance, guest should see a complete SLIT table.
> I found QEMU has provide `-acpitable` command that allows users to add
> a ACPI table into guest, but it requires users building ACPI table by
> themselves first. Using `-acpitable` to add a SLIT table may be not so
> straightforward or flexible, imagine that when the vNUMA configuration
> is changes and we need to generate another SLIT table manually. It may
> not be friendly to users or upper software like libvirt.
> 
> This patch is going to add SLIT table support in QEMU, and provides
> additional option `dist` for command `-numa` to allow user set vNUMA
> distance by QEMU command.
> 
> With this patch, when a user wants to create a guest that contains
> several vNUMA nodes and also wants to set distance among those nodes,
> the QEMU command would like:
> 
> ```
> -object 
> memory-backend-ram,size=1G,prealloc=yes,host-nodes=0,policy=bind,id=node0 \
> -numa node,nodeid=0,cpus=0,memdev=node0 \
> -object 
> memory-backend-ram,size=1G,prealloc=yes,host-nodes=1,policy=bind,id=node1 \
> -numa node,nodeid=1,cpus=1,memdev=node1 \
> -object 
> memory-backend-ram,size=1G,prealloc=yes,host-nodes=2,policy=bind,id=node2 \
> -numa node,nodeid=2,cpus=2,memdev=node2 \
> -object 
> memory-backend-ram,size=1G,prealloc=yes,host-nodes=3,policy=bind,id=node3 \
> -numa node,nodeid=3,cpus=3,memdev=node3 \
> -numa dist,src=0,dst=1,val=21 \
> -numa dist,src=0,dst=2,val=31 \
> -numa dist,src=0,dst=3,val=41 \
> -numa dist,src=1,dst=0,val=21 \
> ...
> ```
> 
> Signed-off-by: He Chen 
> 
> fix
> ---
>  hw/acpi/aml-build.c | 26 +
>  hw/arm/virt-acpi-build.c|  2 ++
>  hw/i386/acpi-build.c|  2 ++
>  include/hw/acpi/aml-build.h |  1 +
>  include/sysemu/numa.h   |  1 +
>  include/sysemu/sysemu.h |  4 
>  numa.c  | 47 
> +
>  qapi-schema.json| 30 ++---
>  qemu-options.hx | 12 +++-
>  9 files changed, 121 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index c6f2032..410b30e 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -24,6 +24,7 @@
>  #include "hw/acpi/aml-build.h"
>  #include "qemu/bswap.h"
>  #include "qemu/bitops.h"
> +#include "sysemu/numa.h"
>  
>  static GArray *build_alloc_array(void)
>  {
> @@ -1609,3 +1610,28 @@ void build_srat_memory(AcpiSratMemoryAffinity 
> *numamem, uint64_t base,
>  numamem->base_addr = cpu_to_le64(base);
>  numamem->range_length = cpu_to_le64(len);
>  }
> +
> +/*
> + * ACPI spec 5.2.17 System Locality Distance Information Table
> + * (Revision 2.0 or later)
> + */
> +void build_slit(GArray *table_data, BIOSLinker *linker)
> +{
> +int slit_start, i, j;
> +slit_start = table_data->len;
> +
> +acpi_data_push(table_data, sizeof(AcpiTableHeader));
> +
> +build_append_int_noprefix(table_data, nb_numa_nodes, 8);
> +for (i = 0; i < nb_numa_nodes; i++) {
> +for (j = 0; j < nb_numa_nodes; j++) {
> +build_append_int_noprefix(table_data, numa_info[i].distance[j], 
> 1);
> +}
> +}
> +
> +build_header(linker, table_data,
> + (void *)(table_data->data + slit_start),
> + "SLIT",
> + table_data->len - slit_start, 1, NULL, NULL);
> +}
> +
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 0835e59..d9e6828 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -781,6 +781,8 @@ void virt_acpi_build(VirtMachineState *vms, 
> AcpiBuildTables *tables)
>  if (nb_numa_nodes > 0) {
>  acpi_add_table(table_offsets, tables_blob);
>  build_srat(tables_blob, tables->linker, vms);
> +acpi_add_table(table_offsets, tables_blob);
> +build_slit(tables_blob, tables->linker);
>  }
>  
>  if (its_class_name() && !vmc->no_its) {
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>

[Qemu-devel] [Bug 1675332] [NEW] qemu-system crashes when use sheepdog driver

[baoxue]

Public bug reported:

Already solved.

** Affects: qemu
 Importance: Undecided
 Status: New


** Tags: sheepdog

** Attachment added: "i have fixed."
   https://bugs.launchpad.net/bugs/1675332/+attachment/4843175/+files/sheepdog.c

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1675332

Title:
  qemu-system crashes when use sheepdog driver

Status in QEMU:
  New

Bug description:
  Already solved.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1675332/+subscriptions

[Qemu-devel] [Bug 1675333] [NEW] qemu-system crashes when use sheepdog driver

[baoxue]

Public bug reported:

Already solved.

** Affects: qemu
 Importance: Undecided
 Status: New


** Tags: sheepdog

** Attachment added: "i have fixed."
   https://bugs.launchpad.net/bugs/1675333/+attachment/4843176/+files/sheepdog.c

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1675333

Title:
  qemu-system crashes when use sheepdog driver

Status in QEMU:
  New

Bug description:
  Already solved.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1675333/+subscriptions

[Qemu-devel] [PULL for-2.9 2/2] cryptodev: fix asserting single queue

[Gonglei]

From: Halil Pasic 

We already check for queues == 1 in cryptodev_builtin_init and when that
is not true raise an error. But before that error is reported the
assertion in cryptodev_builtin_cleanup kicks in (because object is being
finalized and freed).

Let's remove assert(queues == 1) form cryptodev_builtin_cleanup as it
does only harm and no good.

Reported-by: Boris Fiuczynski 
Signed-off-by: Halil Pasic 
Reviewed-by: Eric Blake 
Signed-off-by: Gonglei 
---
 backends/cryptodev-builtin.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/backends/cryptodev-builtin.c b/backends/cryptodev-builtin.c
index b24a299..657c0ba 100644
--- a/backends/cryptodev-builtin.c
+++ b/backends/cryptodev-builtin.c
@@ -361,8 +361,6 @@ static void cryptodev_builtin_cleanup(
 }
 }
 
-assert(queues == 1);
-
 for (i = 0; i < queues; i++) {
 cc = backend->conf.peers.ccs[i];
 if (cc) {
-- 
1.7.12.4

[Qemu-devel] [PULL for-2.9 0/2] cryptodev patches 20170323

[Gonglei]

The following changes since commit 55a19ad8b2d0797e3a8fe90ab99a9bb713824059:

  Update version for v2.9.0-rc1 release (2017-03-21 17:13:29 +)

are available in the git repository at:

  https://github.com/gongleiarei/qemu.git tags/cryptodev-next-20170323

for you to fetch changes up to b7bad50ae81efeb180609eeecdb086ebc7536ed7:

  cryptodev: fix asserting single queue (2017-03-23 17:22:01 +0800)


cryptodev fixes



Halil Pasic (1):
  cryptodev: fix asserting single queue

Longpeng(Mike) (1):
  cryptodev: setiv only when really need

 backends/cryptodev-builtin.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

-- 
1.7.12.4

[Qemu-devel] [PULL for-2.9 1/2] cryptodev: setiv only when really need

[Gonglei]

From: "Longpeng(Mike)" 

ECB mode cipher doesn't need IV, if we setiv for it then qemu
crypto API would report "Expected IV size 0 not **", so we should
setiv only when the cipher algos really need.

Signed-off-by: Longpeng(Mike) 
Signed-off-by: Gonglei 
---
 backends/cryptodev-builtin.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/backends/cryptodev-builtin.c b/backends/cryptodev-builtin.c
index 82a068e..b24a299 100644
--- a/backends/cryptodev-builtin.c
+++ b/backends/cryptodev-builtin.c
@@ -320,10 +320,12 @@ static int cryptodev_builtin_sym_operation(
 
 sess = builtin->sessions[op_info->session_id];
 
-ret = qcrypto_cipher_setiv(sess->cipher, op_info->iv,
-   op_info->iv_len, errp);
-if (ret < 0) {
-return -VIRTIO_CRYPTO_ERR;
+if (op_info->iv_len > 0) {
+ret = qcrypto_cipher_setiv(sess->cipher, op_info->iv,
+   op_info->iv_len, errp);
+if (ret < 0) {
+return -VIRTIO_CRYPTO_ERR;
+}
 }
 
 if (sess->direction == VIRTIO_CRYPTO_OP_ENCRYPT) {
-- 
1.7.12.4

Re: [Qemu-devel] help please how to use qemu.

[Alex Bennée]

Richard Odell  writes:

> step by step instructions please

https://qemu.weilnetz.de/doc/qemu-doc.html

--
Alex Bennée

Re: [Qemu-devel] [Bug 1675108] [NEW] Cocoa UI always crashes on startup

[Peter Maydell]

On 22 March 2017 at 17:26, Brendan Shanks  wrote:
> Public bug reported:
>
> Commit 8bb93c6f99a42c2e0943bc904b283cd622d302c5 ("ui/console: ensure
> graphic updates don't race with TCG vCPUs") causes the graphic update to
> run on a non-main thread, which Cocoa is not happy with. It crashes
> immediately after startup.

Oops. Alex, we can't just run UI code on random threads like this.
Any ideas?

thanks
-- PMM

Re: [Qemu-devel] [PATCH] 9pfs: fix file descriptor leak

[Greg Kurz]

On Thu, 23 Mar 2017 01:52:13 -0700
Li Qiang  wrote:

> In v9fs_create/lcreate dispatch handler, the fidp's fid_type is not checked
> before used. As these function will set the fid_type, if the guest call
> more than once them, it will leak the fidp. This can cause some other

Not leak the fidp but rather a file descriptor or directory handle...

> issue, such as memory leak. Check the fid_type before using them.
> 

or memory previously allocated for an extended attribute.

I'll fix the changelog before pushing the fix.

Thanks,

--
Greg

> Signed-off-by: Li Qiang 
> ---
>  hw/9pfs/9p.c | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index b8c0b99..48babce 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -1550,6 +1550,10 @@ static void coroutine_fn v9fs_lcreate(void *opaque)
>  err = -ENOENT;
>  goto out_nofid;
>  }
> +if (fidp->fid_type != P9_FID_NONE) {
> +err = -EINVAL;
> +goto out;
> +}
>  
>  flags = get_dotl_openflags(pdu->s, flags);
>  err = v9fs_co_open2(pdu, fidp, &name, gid,
> @@ -2153,6 +2157,10 @@ static void coroutine_fn v9fs_create(void *opaque)
>  err = -EINVAL;
>  goto out_nofid;
>  }
> +if (fidp->fid_type != P9_FID_NONE) {
> +err = -EINVAL;
> +goto out;
> +}
>  if (perm & P9_STAT_MODE_DIR) {
>  err = v9fs_co_mkdir(pdu, fidp, &name, perm & 0777,
>  fidp->uid, -1, &stbuf);



pgpqgTG1mVx4H.pgp
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH RESEND 1/3] usb-ccid: make ccid_write_data_block() cope with null buffers

[Gerd Hoffmann]

  Hi,

>  oops, there are hard-coded calls with NULL/0. I suppose to fix clang
> warning, it would need to check if data != null for memcpy.

I'd check for len > 0, and in that if branch we can also assert on data
== NULL and thereby check that len and data are consistent.

cheers,
  Gerd

[Qemu-devel] [PULL for-2.9 1/1] target/s390x: Fix broken user mode

[Cornelia Huck]

From: Stefan Weil 

Returning NULL from get_max_cpu_model results in a SIGSEGV runtime error.

Signed-off-by: Stefan Weil 
Reviewed-by: David Hildenbrand 
Message-Id: <20170130131517.8092-1...@weilnetz.de>
Cc: qemu-sta...@nongnu.org
Signed-off-by: Christian Borntraeger 
Signed-off-by: Cornelia Huck 
---
 target/s390x/cpu_models.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index 4ea3a2de80..1434d15315 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -660,7 +660,6 @@ static void check_compatibility(const S390CPUModel 
*max_model,
 
 static S390CPUModel *get_max_cpu_model(Error **errp)
 {
-#ifndef CONFIG_USER_ONLY
 static S390CPUModel max_model;
 static bool cached;
 
@@ -680,7 +679,6 @@ static S390CPUModel *get_max_cpu_model(Error **errp)
 cached = true;
 return &max_model;
 }
-#endif
 return NULL;
 }
 
-- 
2.11.0

[Qemu-devel] [PULL for-2.9 0/1] another s390x bugfix

[Cornelia Huck]

The following changes since commit 55a19ad8b2d0797e3a8fe90ab99a9bb713824059:

  Update version for v2.9.0-rc1 release (2017-03-21 17:13:29 +)

are available in the git repository at:

  git://github.com/cohuck/qemu tags/s390x-20170323

for you to fetch changes up to a352aa62a75fcb1db35a0c71a10af3b2c1f8b89f:

  target/s390x: Fix broken user mode (2017-03-23 10:49:13 +0100)


Fix linux-user vs. cpu models.



Stefan Weil (1):
  target/s390x: Fix broken user mode

 target/s390x/cpu_models.c | 2 --
 1 file changed, 2 deletions(-)

-- 
2.11.0

[Qemu-devel] [PATCH] spapr: fix buffer-overflow

[Marc-André Lureau]

Running postcopy-test with ASAN produces the following error:

QTEST_QEMU_BINARY=ppc64-softmmu/qemu-system-ppc64  tests/postcopy-test
...
=
==23641==ERROR: AddressSanitizer: heap-buffer-overflow on address 
0x7f155660 at pc 0x55b8e9d28208 bp 0x7f1555f4d3c0 sp 0x7f1555f4d3b0
READ of size 8 at 0x7f155660 thread T6
#0 0x55b8e9d28207 in htab_save_first_pass 
/home/elmarco/src/qq/hw/ppc/spapr.c:1528
#1 0x55b8e9d2939c in htab_save_iterate 
/home/elmarco/src/qq/hw/ppc/spapr.c:1665
#2 0x55b8e9beae3a in qemu_savevm_state_iterate 
/home/elmarco/src/qq/migration/savevm.c:1044
#3 0x55b8ea677733 in migration_thread 
/home/elmarco/src/qq/migration/migration.c:1976
#4 0x7f15845f46c9 in start_thread (/lib64/libpthread.so.0+0x76c9)
#5 0x7f157d9d0f7e in clone (/lib64/libc.so.6+0x107f7e)

0x7f155660 is located 0 bytes to the right of 2097152-byte region 
[0x7f155640,0x7f155660)
allocated by thread T0 here:
#0 0x7f159bb76980 in posix_memalign (/lib64/libasan.so.3+0xc7980)
#1 0x55b8eab185b2 in qemu_try_memalign 
/home/elmarco/src/qq/util/oslib-posix.c:106
#2 0x55b8eab186c8 in qemu_memalign 
/home/elmarco/src/qq/util/oslib-posix.c:122
#3 0x55b8e9d268a8 in spapr_reallocate_hpt 
/home/elmarco/src/qq/hw/ppc/spapr.c:1214
#4 0x55b8e9d26e04 in ppc_spapr_reset 
/home/elmarco/src/qq/hw/ppc/spapr.c:1261
#5 0x55b8ea12e913 in qemu_system_reset /home/elmarco/src/qq/vl.c:1697
#6 0x55b8ea13fa40 in main /home/elmarco/src/qq/vl.c:4679
#7 0x7f157d8e9400 in __libc_start_main (/lib64/libc.so.6+0x20400)

Thread T6 created by T0 here:
#0 0x7f159bae0488 in __interceptor_pthread_create 
(/lib64/libasan.so.3+0x31488)
#1 0x55b8eab1d9cb in qemu_thread_create 
/home/elmarco/src/qq/util/qemu-thread-posix.c:465
#2 0x55b8ea67874c in migrate_fd_connect 
/home/elmarco/src/qq/migration/migration.c:2096
#3 0x55b8ea66cbb0 in migration_channel_connect 
/home/elmarco/src/qq/migration/migration.c:500
#4 0x55b8ea678f38 in socket_outgoing_migration 
/home/elmarco/src/qq/migration/socket.c:87
#5 0x55b8eaa5a03a in qio_task_complete /home/elmarco/src/qq/io/task.c:142
#6 0x55b8eaa599cc in gio_task_thread_result 
/home/elmarco/src/qq/io/task.c:88
#7 0x7f15823e38e6  (/lib64/libglib-2.0.so.0+0x468e6)
SUMMARY: AddressSanitizer: heap-buffer-overflow 
/home/elmarco/src/qq/hw/ppc/spapr.c:1528 in htab_save_first_pass

index seems to be wrongly incremented, unless I miss something that
would be worth a comment.

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

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 6ee566d658..63439811d5 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1524,16 +1524,16 @@ static void htab_save_first_pass(QEMUFile *f, 
sPAPRMachineState *spapr,
 /* Consume invalid HPTEs */
 while ((index < htabslots)
&& !HPTE_VALID(HPTE(spapr->htab, index))) {
-index++;
 CLEAN_HPTE(HPTE(spapr->htab, index));
+index++;
 }
 
 /* Consume valid HPTEs */
 chunkstart = index;
 while ((index < htabslots) && (index - chunkstart < USHRT_MAX)
&& HPTE_VALID(HPTE(spapr->htab, index))) {
-index++;
 CLEAN_HPTE(HPTE(spapr->htab, index));
+index++;
 }
 
 if (index > chunkstart) {
-- 
2.12.0.191.gc5d8de91d

[Qemu-devel] [PATCH] xen-platform: separate unplugging of NVMe disks

[Paul Durrant]

Commit 090fa1c8 "add support for unplugging NVMe disks..." extended the
existing disk unplug flag to cover NVMe disks as well as IDE and SCSI.

The cecent thread on the xen-devel mailing list [1] have highlighted that
this is not desirable behaviour: PV frontends should be able to distinguish
NVMe disks from other types of disk and should have separate control over
whether they are unplugged.

This patch defines a new bit in the unplug mask for this purpose and also
tidies up the definitions of, and improves the comments regarding, the
previously exiting bits in the protocol.

[1] https://lists.xen.org/archives/html/xen-devel/2017-03/msg02924.html

Signed-off-by: Paul Durrant 
--
Cc: Stefano Stabellini 
Cc: Anthony Perard 

NOTE: A companion patch will be submitted to xen-devel to align the
  unplug protocol documentation once this patch is acked.
---
 hw/i386/xen/xen_platform.c | 47 ++
 1 file changed, 35 insertions(+), 12 deletions(-)

diff --git a/hw/i386/xen/xen_platform.c b/hw/i386/xen/xen_platform.c
index 6010f35..983d532 100644
--- a/hw/i386/xen/xen_platform.c
+++ b/hw/i386/xen/xen_platform.c
@@ -87,10 +87,30 @@ static void log_writeb(PCIXenPlatformState *s, char val)
 }
 }
 
-/* Xen Platform, Fixed IOPort */
-#define UNPLUG_ALL_DISKS 1
-#define UNPLUG_ALL_NICS 2
-#define UNPLUG_AUX_IDE_DISKS 4
+/*
+ * Unplug device flags.
+ *
+ * The logic got a little confused at some point in the past but this is
+ * what they do now.
+ *
+ * bit 0: Unplug all IDE and SCSI disks.
+ * bit 1: Unplug all NICs.
+ * bit 2: Unplug IDE disks except primary master. This is overridden if
+ *bit 0 is also present in the mask.
+ * bit 3: Unplug all NVMe disks.
+ *
+ */
+#define _UNPLUG_IDE_SCSI_DISKS 0
+#define UNPLUG_IDE_SCSI_DISKS (1u << _UNPLUG_IDE_SCSI_DISKS)
+
+#define _UNPLUG_ALL_NICS 1
+#define UNPLUG_ALL_NICS (1u << _UNPLUG_ALL_NICS)
+
+#define _UNPLUG_AUX_IDE_DISKS 2
+#define UNPLUG_AUX_IDE_DISKS (1u << _UNPLUG_AUX_IDE_DISKS)
+
+#define _UNPLUG_NVME_DISKS 3
+#define UNPLUG_NVME_DISKS (1u << _UNPLUG_NVME_DISKS)
 
 static void unplug_nic(PCIBus *b, PCIDevice *d, void *o)
 {
@@ -111,7 +131,7 @@ static void unplug_disks(PCIBus *b, PCIDevice *d, void 
*opaque)
 {
 uint32_t flags = *(uint32_t *)opaque;
 bool aux = (flags & UNPLUG_AUX_IDE_DISKS) &&
-!(flags & UNPLUG_ALL_DISKS);
+!(flags & UNPLUG_IDE_SCSI_DISKS);
 
 /* We have to ignore passthrough devices */
 if (!strcmp(d->name, "xen-pci-passthrough")) {
@@ -124,12 +144,16 @@ static void unplug_disks(PCIBus *b, PCIDevice *d, void 
*opaque)
 break;
 
 case PCI_CLASS_STORAGE_SCSI:
-case PCI_CLASS_STORAGE_EXPRESS:
 if (!aux) {
 object_unparent(OBJECT(d));
 }
 break;
 
+case PCI_CLASS_STORAGE_EXPRESS:
+if (flags & UNPLUG_NVME_DISKS) {
+object_unparent(OBJECT(d));
+}
+
 default:
 break;
 }
@@ -147,10 +171,9 @@ static void platform_fixed_ioport_writew(void *opaque, 
uint32_t addr, uint32_t v
 switch (addr) {
 case 0: {
 PCIDevice *pci_dev = PCI_DEVICE(s);
-/* Unplug devices.  Value is a bitmask of which devices to
-   unplug, with bit 0 the disk devices, bit 1 the network
-   devices, and bit 2 the non-primary-master IDE devices. */
-if (val & (UNPLUG_ALL_DISKS | UNPLUG_AUX_IDE_DISKS)) {
+/* Unplug devices. See comment above flag definitions */
+if (val & (UNPLUG_IDE_SCSI_DISKS | UNPLUG_AUX_IDE_DISKS |
+   UNPLUG_NVME_DISKS)) {
 DPRINTF("unplug disks\n");
 pci_unplug_disks(pci_dev->bus, val);
 }
@@ -338,14 +361,14 @@ static void xen_platform_ioport_writeb(void *opaque, 
hwaddr addr,
  * If VMDP was to control both disk and LAN it would use 4.
  * If it controlled just disk or just LAN, it would use 8 below.
  */
-pci_unplug_disks(pci_dev->bus, UNPLUG_ALL_DISKS);
+pci_unplug_disks(pci_dev->bus, UNPLUG_IDE_SCSI_DISKS);
 pci_unplug_nics(pci_dev->bus);
 }
 break;
 case 8:
 switch (val) {
 case 1:
-pci_unplug_disks(pci_dev->bus, UNPLUG_ALL_DISKS);
+pci_unplug_disks(pci_dev->bus, UNPLUG_IDE_SCSI_DISKS);
 break;
 case 2:
 pci_unplug_nics(pci_dev->bus);
-- 
2.1.4

[Qemu-devel] [PULL 1/1] numa, spapr: align default numa node memory size to 256MB

[David Gibson]

From: Laurent Vivier 

Since commit 224245b ("spapr: Add LMB DR connectors"), NUMA node
memory size must be aligned to 256MB (SPAPR_MEMORY_BLOCK_SIZE).

But when "-numa" option is provided without "mem" parameter,
the memory is equally divided between nodes, but 8MB aligned.
This can be not valid for pseries.

In that case we can have:
$ ./ppc64-softmmu/qemu-system-ppc64 -m 4G -numa node -numa node -numa node
qemu-system-ppc64: Node 0 memory size 0x5500 is not aligned to 256 MiB

With this patch, we have:
(qemu) info numa
3 nodes
node 0 cpus: 0
node 0 size: 1280 MB
node 1 cpus:
node 1 size: 1280 MB
node 2 cpus:
node 2 size: 1536 MB

Signed-off-by: Laurent Vivier 
Signed-off-by: David Gibson 
---
 hw/core/machine.c   | 5 +
 hw/ppc/spapr.c  | 6 ++
 include/hw/boards.h | 1 +
 numa.c  | 6 +++---
 4 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 0d92672..ada9eea 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -396,6 +396,11 @@ static void machine_class_init(ObjectClass *oc, void *data)
 mc->default_ram_size = 128 * M_BYTE;
 mc->rom_file_has_mr = true;
 
+/* numa node memory size aligned on 8MB by default.
+ * On Linux, each node's border has to be 8MB aligned
+ */
+mc->numa_mem_align_shift = 23;
+
 object_class_property_add_str(oc, "accel",
 machine_get_accel, machine_set_accel, &error_abort);
 object_class_property_set_description(oc, "accel",
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 6ee566d..8aecea3 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -3096,6 +3096,11 @@ static void spapr_machine_class_init(ObjectClass *oc, 
void *data)
 xic->ics_resend = spapr_ics_resend;
 xic->icp_get = spapr_icp_get;
 ispc->print_info = spapr_pic_print_info;
+/* Force NUMA node memory size to be a multiple of
+ * SPAPR_MEMORY_BLOCK_SIZE (256M) since that's the granularity
+ * in which LMBs are represented and hot-added
+ */
+mc->numa_mem_align_shift = 28;
 }
 
 static const TypeInfo spapr_machine_info = {
@@ -3180,6 +3185,7 @@ static void spapr_machine_2_8_class_options(MachineClass 
*mc)
 {
 spapr_machine_2_9_class_options(mc);
 SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_8);
+mc->numa_mem_align_shift = 23;
 }
 
 DEFINE_SPAPR_MACHINE(2_8, "2.8", false);
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 269d0ba..31d9c72 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -135,6 +135,7 @@ struct MachineClass {
 bool rom_file_has_mr;
 int minimum_page_bits;
 bool has_hotpluggable_cpus;
+int numa_mem_align_shift;
 
 HotplugHandler *(*get_hotplug_handler)(MachineState *machine,
DeviceState *dev);
diff --git a/numa.c b/numa.c
index e01cb54..6fc2393 100644
--- a/numa.c
+++ b/numa.c
@@ -338,12 +338,12 @@ void parse_numa_opts(MachineClass *mc)
 if (i == nb_numa_nodes) {
 uint64_t usedmem = 0;
 
-/* On Linux, each node's border has to be 8MB aligned,
- * the final node gets the rest.
+/* Align each node according to the alignment
+ * requirements of the machine class
  */
 for (i = 0; i < nb_numa_nodes - 1; i++) {
 numa_info[i].node_mem = (ram_size / nb_numa_nodes) &
-~((1 << 23UL) - 1);
+~((1 << mc->numa_mem_align_shift) - 1);
 usedmem += numa_info[i].node_mem;
 }
 numa_info[i].node_mem = ram_size - usedmem;
-- 
2.9.3

[Qemu-devel] [PULL 0/1] ppc-for-2.9 queue 20170323

[David Gibson]

The following changes since commit 55a19ad8b2d0797e3a8fe90ab99a9bb713824059:

  Update version for v2.9.0-rc1 release (2017-03-21 17:13:29 +)

are available in the git repository at:

  git://github.com/dgibson/qemu.git tags/ppc-for-2.9-20170323

for you to fetch changes up to 55641213fc81cdfc3bcd56c9e9991faa119201b2:

  numa,spapr: align default numa node memory size to 256MB (2017-03-22 11:32:42 
+1100)


ppc patch queue for 2017-03-23

Just a single bugfix in this batch.  It's not strictly in ppc code,
though it's for the pseries machine's benefit.  Eduardo suggested it
go through my tree however.


Laurent Vivier (1):
  numa,spapr: align default numa node memory size to 256MB

 hw/core/machine.c   | 5 +
 hw/ppc/spapr.c  | 6 ++
 include/hw/boards.h | 1 +
 numa.c  | 6 +++---
 4 files changed, 15 insertions(+), 3 deletions(-)

Re: [Qemu-devel] Guest application reading from pl011 without device driver

[Jiahuan Zhang]

On 22 March 2017 at 12:27, Paolo Bonzini  wrote:

>
>
> On 22/03/2017 11:28, Jiahuan Zhang wrote:
> >
> > A function that lets a process sleep until data is available on the
> > socket.  The solution is to rewrite Windows chardev handling in QEMU
> to
> > use threads or overlapped I/O.
> >
> > Yes, socket is working well. Will you add the "select" to pipe
> > implementation?
>
> It's Windows that doesn't support it (the Windows function name is
> WaitForSingleObject).


Hi, I have checked the Windows chardev implimentation in QEMU.
I learned from char-win-stdio.c to using thread and WaitForSingleObject for
interlocking.
char-win-stdio.c uses qemu_add_wait_object().
Char-pipe.c uses qemu_add_polling_cb().
I found all over qemu, only char-pipe uses qemu_add_polling_cb().
Does this mean that somebody has already looked into this issue,
and failed in using qemu_add_wait_object(),
and then he/she created qemu_add_polling_cb()?

If so, what I am going to do is just something has been proved to be
useless.
Please tell me if my guess is true.

Best,
Huan

>
> Paolo
>
> > Or shall I look into it and fix? Since in my case, I prefer windows pipe
> > to socket.
> > But if I do, definitly, I will spend much more effort than you, the
> experts.
>

Re: [Qemu-devel] [PATCH] xen-platform: separate unplugging of NVMe disks

[Paul Durrant]

> -Original Message-
> From: Paul Durrant [mailto:paul.durr...@citrix.com]
> Sent: 23 March 2017 10:10
> To: qemu-devel@nongnu.org; xen-de...@lists.xenproject.org
> Cc: Paul Durrant ; Stefano Stabellini
> ; Anthony Perard 
> Subject: [PATCH] xen-platform: separate unplugging of NVMe disks
> 
> Commit 090fa1c8 "add support for unplugging NVMe disks..." extended the
> existing disk unplug flag to cover NVMe disks as well as IDE and SCSI.
> 
> The cecent thread on the xen-devel mailing list [1] have highlighted that

That spelling and grammar is terrible. Sorry about that... I'll send a v2 with 
it fixed.

  Paul

> this is not desirable behaviour: PV frontends should be able to distinguish
> NVMe disks from other types of disk and should have separate control over
> whether they are unplugged.
> 
> This patch defines a new bit in the unplug mask for this purpose and also
> tidies up the definitions of, and improves the comments regarding, the
> previously exiting bits in the protocol.
> 
> [1] https://lists.xen.org/archives/html/xen-devel/2017-03/msg02924.html
> 
> Signed-off-by: Paul Durrant 
> --
> Cc: Stefano Stabellini 
> Cc: Anthony Perard 
> 
> NOTE: A companion patch will be submitted to xen-devel to align the
>   unplug protocol documentation once this patch is acked.
> ---
>  hw/i386/xen/xen_platform.c | 47
> ++
>  1 file changed, 35 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/i386/xen/xen_platform.c b/hw/i386/xen/xen_platform.c
> index 6010f35..983d532 100644
> --- a/hw/i386/xen/xen_platform.c
> +++ b/hw/i386/xen/xen_platform.c
> @@ -87,10 +87,30 @@ static void log_writeb(PCIXenPlatformState *s, char
> val)
>  }
>  }
> 
> -/* Xen Platform, Fixed IOPort */
> -#define UNPLUG_ALL_DISKS 1
> -#define UNPLUG_ALL_NICS 2
> -#define UNPLUG_AUX_IDE_DISKS 4
> +/*
> + * Unplug device flags.
> + *
> + * The logic got a little confused at some point in the past but this is
> + * what they do now.
> + *
> + * bit 0: Unplug all IDE and SCSI disks.
> + * bit 1: Unplug all NICs.
> + * bit 2: Unplug IDE disks except primary master. This is overridden if
> + *bit 0 is also present in the mask.
> + * bit 3: Unplug all NVMe disks.
> + *
> + */
> +#define _UNPLUG_IDE_SCSI_DISKS 0
> +#define UNPLUG_IDE_SCSI_DISKS (1u << _UNPLUG_IDE_SCSI_DISKS)
> +
> +#define _UNPLUG_ALL_NICS 1
> +#define UNPLUG_ALL_NICS (1u << _UNPLUG_ALL_NICS)
> +
> +#define _UNPLUG_AUX_IDE_DISKS 2
> +#define UNPLUG_AUX_IDE_DISKS (1u << _UNPLUG_AUX_IDE_DISKS)
> +
> +#define _UNPLUG_NVME_DISKS 3
> +#define UNPLUG_NVME_DISKS (1u << _UNPLUG_NVME_DISKS)
> 
>  static void unplug_nic(PCIBus *b, PCIDevice *d, void *o)
>  {
> @@ -111,7 +131,7 @@ static void unplug_disks(PCIBus *b, PCIDevice *d,
> void *opaque)
>  {
>  uint32_t flags = *(uint32_t *)opaque;
>  bool aux = (flags & UNPLUG_AUX_IDE_DISKS) &&
> -!(flags & UNPLUG_ALL_DISKS);
> +!(flags & UNPLUG_IDE_SCSI_DISKS);
> 
>  /* We have to ignore passthrough devices */
>  if (!strcmp(d->name, "xen-pci-passthrough")) {
> @@ -124,12 +144,16 @@ static void unplug_disks(PCIBus *b, PCIDevice *d,
> void *opaque)
>  break;
> 
>  case PCI_CLASS_STORAGE_SCSI:
> -case PCI_CLASS_STORAGE_EXPRESS:
>  if (!aux) {
>  object_unparent(OBJECT(d));
>  }
>  break;
> 
> +case PCI_CLASS_STORAGE_EXPRESS:
> +if (flags & UNPLUG_NVME_DISKS) {
> +object_unparent(OBJECT(d));
> +}
> +
>  default:
>  break;
>  }
> @@ -147,10 +171,9 @@ static void platform_fixed_ioport_writew(void
> *opaque, uint32_t addr, uint32_t v
>  switch (addr) {
>  case 0: {
>  PCIDevice *pci_dev = PCI_DEVICE(s);
> -/* Unplug devices.  Value is a bitmask of which devices to
> -   unplug, with bit 0 the disk devices, bit 1 the network
> -   devices, and bit 2 the non-primary-master IDE devices. */
> -if (val & (UNPLUG_ALL_DISKS | UNPLUG_AUX_IDE_DISKS)) {
> +/* Unplug devices. See comment above flag definitions */
> +if (val & (UNPLUG_IDE_SCSI_DISKS | UNPLUG_AUX_IDE_DISKS |
> +   UNPLUG_NVME_DISKS)) {
>  DPRINTF("unplug disks\n");
>  pci_unplug_disks(pci_dev->bus, val);
>  }
> @@ -338,14 +361,14 @@ static void xen_platform_ioport_writeb(void
> *opaque, hwaddr addr,
>   * If VMDP was to control both disk and LAN it would use 4.
>   * If it controlled just disk or just LAN, it would use 8 below.
>   */
> -pci_unplug_disks(pci_dev->bus, UNPLUG_ALL_DISKS);
> +pci_unplug_disks(pci_dev->bus, UNPLUG_IDE_SCSI_DISKS);
>  pci_unplug_nics(pci_dev->bus);
>  }
>  break;
>  case 8:
>  switch (val) {
>  case 1:
> -pci_unplug_disks(pci_dev->bus, UNPLUG_ALL_DISKS);
> +pci_unplug_disks(pci_dev->bus, UNPLUG_IDE_SCSI_DISKS);
>  break

[Qemu-devel] [PATCH v2] xen-platform: separate unplugging of NVMe disks

[Paul Durrant]

Commit 090fa1c8 "add support for unplugging NVMe disks..." extended the
existing disk unplug flag to cover NVMe disks as well as IDE and SCSI.

The recent thread on the xen-devel mailing list [1] has highlighted that
this is not desirable behaviour: PV frontends should be able to distinguish
NVMe disks from other types of disk and should have separate control over
whether they are unplugged.

This patch defines a new bit in the unplug mask for this purpose and also
tidies up the definitions of, and improves the comments regarding, the
previously exiting bits in the protocol.

[1] https://lists.xen.org/archives/html/xen-devel/2017-03/msg02924.html

Signed-off-by: Paul Durrant 
--
Cc: Stefano Stabellini 
Cc: Anthony Perard 

NOTE: A companion patch will be submitted to xen-devel to align the
  unplug protocol documentation once this patch is acked.

v2:
- Fix the commit comment
---
 hw/i386/xen/xen_platform.c | 47 ++
 1 file changed, 35 insertions(+), 12 deletions(-)

diff --git a/hw/i386/xen/xen_platform.c b/hw/i386/xen/xen_platform.c
index 6010f35..983d532 100644
--- a/hw/i386/xen/xen_platform.c
+++ b/hw/i386/xen/xen_platform.c
@@ -87,10 +87,30 @@ static void log_writeb(PCIXenPlatformState *s, char val)
 }
 }
 
-/* Xen Platform, Fixed IOPort */
-#define UNPLUG_ALL_DISKS 1
-#define UNPLUG_ALL_NICS 2
-#define UNPLUG_AUX_IDE_DISKS 4
+/*
+ * Unplug device flags.
+ *
+ * The logic got a little confused at some point in the past but this is
+ * what they do now.
+ *
+ * bit 0: Unplug all IDE and SCSI disks.
+ * bit 1: Unplug all NICs.
+ * bit 2: Unplug IDE disks except primary master. This is overridden if
+ *bit 0 is also present in the mask.
+ * bit 3: Unplug all NVMe disks.
+ *
+ */
+#define _UNPLUG_IDE_SCSI_DISKS 0
+#define UNPLUG_IDE_SCSI_DISKS (1u << _UNPLUG_IDE_SCSI_DISKS)
+
+#define _UNPLUG_ALL_NICS 1
+#define UNPLUG_ALL_NICS (1u << _UNPLUG_ALL_NICS)
+
+#define _UNPLUG_AUX_IDE_DISKS 2
+#define UNPLUG_AUX_IDE_DISKS (1u << _UNPLUG_AUX_IDE_DISKS)
+
+#define _UNPLUG_NVME_DISKS 3
+#define UNPLUG_NVME_DISKS (1u << _UNPLUG_NVME_DISKS)
 
 static void unplug_nic(PCIBus *b, PCIDevice *d, void *o)
 {
@@ -111,7 +131,7 @@ static void unplug_disks(PCIBus *b, PCIDevice *d, void 
*opaque)
 {
 uint32_t flags = *(uint32_t *)opaque;
 bool aux = (flags & UNPLUG_AUX_IDE_DISKS) &&
-!(flags & UNPLUG_ALL_DISKS);
+!(flags & UNPLUG_IDE_SCSI_DISKS);
 
 /* We have to ignore passthrough devices */
 if (!strcmp(d->name, "xen-pci-passthrough")) {
@@ -124,12 +144,16 @@ static void unplug_disks(PCIBus *b, PCIDevice *d, void 
*opaque)
 break;
 
 case PCI_CLASS_STORAGE_SCSI:
-case PCI_CLASS_STORAGE_EXPRESS:
 if (!aux) {
 object_unparent(OBJECT(d));
 }
 break;
 
+case PCI_CLASS_STORAGE_EXPRESS:
+if (flags & UNPLUG_NVME_DISKS) {
+object_unparent(OBJECT(d));
+}
+
 default:
 break;
 }
@@ -147,10 +171,9 @@ static void platform_fixed_ioport_writew(void *opaque, 
uint32_t addr, uint32_t v
 switch (addr) {
 case 0: {
 PCIDevice *pci_dev = PCI_DEVICE(s);
-/* Unplug devices.  Value is a bitmask of which devices to
-   unplug, with bit 0 the disk devices, bit 1 the network
-   devices, and bit 2 the non-primary-master IDE devices. */
-if (val & (UNPLUG_ALL_DISKS | UNPLUG_AUX_IDE_DISKS)) {
+/* Unplug devices. See comment above flag definitions */
+if (val & (UNPLUG_IDE_SCSI_DISKS | UNPLUG_AUX_IDE_DISKS |
+   UNPLUG_NVME_DISKS)) {
 DPRINTF("unplug disks\n");
 pci_unplug_disks(pci_dev->bus, val);
 }
@@ -338,14 +361,14 @@ static void xen_platform_ioport_writeb(void *opaque, 
hwaddr addr,
  * If VMDP was to control both disk and LAN it would use 4.
  * If it controlled just disk or just LAN, it would use 8 below.
  */
-pci_unplug_disks(pci_dev->bus, UNPLUG_ALL_DISKS);
+pci_unplug_disks(pci_dev->bus, UNPLUG_IDE_SCSI_DISKS);
 pci_unplug_nics(pci_dev->bus);
 }
 break;
 case 8:
 switch (val) {
 case 1:
-pci_unplug_disks(pci_dev->bus, UNPLUG_ALL_DISKS);
+pci_unplug_disks(pci_dev->bus, UNPLUG_IDE_SCSI_DISKS);
 break;
 case 2:
 pci_unplug_nics(pci_dev->bus);
-- 
2.1.4

Re: [Qemu-devel] [BUG] user-to-root privesc inside VM via bad translation caching

[Paolo Bonzini]

On 22/03/2017 21:01, Richard Henderson wrote:
>>
>> Ah, OK. Thanks for the explanation. May be we should check the size of
>> the instruction while decoding the prefixes and error out once we
>> exceed the limit. We would not generate any IR code.
> 
> Yes.
> 
> It would not enforce a true limit of 15 bytes, since you can't know that
> until you've done the rest of the decode.  But you'd be able to say that
> no more than 14 prefix + 1 opc + 6 modrm+sib+ofs + 4 immediate = 25
> bytes is used.
>
> Which does fix the bug. 

Yeah, that would work for 2.9 if somebody wants to put together a patch.
 Ensuring that all instruction fetching happens before translation side
effects is a little harder, but perhaps it's also the opportunity to get
rid of s->rip_offset which is a little ugly.

Paolo

Re: [Qemu-devel] Proposal for deprecating unsupported host OSes & architecutures

[Paolo Bonzini]

On 22/03/2017 14:09, Peter Maydell wrote:
> On 22 March 2017 at 12:51, Alex Bennée  wrote:
>> Peter Maydell  writes:
>>> ...unfortunately the gcc compile farm mips board (1) is very slow
>>> and (2) has very little disk space free in /tmp, which means that
>>> it can't pass "make check" because for instance tests/test-replication
>>> assumes it can write comparatively large test files to /tmp/...
>>
>> That makes it sound like a mips cross build or mips linux-user powered
>> image would be useful then?
> 
> Cross build can't actually run 'make check' and I wouldn't
> trust linux-user to run our test suite. Also, if there's
> no hardware that we can sensibly do make/make check
> on then how much can people really care about QEMU on MIPS?
> (In fact since MIPS supports KVM these days, there really
> ought to be sufficiently capable hardware to work as
> a build system.)

I own a MIPS Creator ci20 board (donated by Imagination Technologies).
I cannot give it a public IP address, but I can try and use it to do
builds every now and then.

Paolo

Re: [Qemu-devel] Guest application reading from pl011 without device driver

[Paolo Bonzini]

On 23/03/2017 11:12, Jiahuan Zhang wrote:
> 
> It's Windows that doesn't support it (the Windows function name is
> WaitForSingleObject).
> 
> 
> Hi, I have checked the Windows chardev implimentation in QEMU.
> I learned from char-win-stdio.c to using thread and WaitForSingleObject
> for interlocking.
> char-win-stdio.c uses qemu_add_wait_object().
> Char-pipe.c uses qemu_add_polling_cb().
> I found all over qemu, only char-pipe uses qemu_add_polling_cb().
> Does this mean that somebody has already looked into this issue,
> and failed in using qemu_add_wait_object(),
> and then he/she created qemu_add_polling_cb()?

I don't know, but using threads sounds like a way to solve the bug, indeed.

Thanks,

Paolo

[Qemu-devel] [PATCH for-2.9 5/5] rbd: Reject options server.*.{numeric, to, ipv4, ipv6}

[Markus Armbruster]

We use InetSocketAddress in the QAPI schema.  However, the code
doesn't use inet_connect_saddr(), but formats "host" and "port" into a
configuration string for rados_conf_set().  Thus, members "numeric",
"to", "ipv4" and "ipv6" are silently ignored.  Not nice.

Factor a suitable InetSocketAddressBase out of InetSocketAddress, and
use that.  "numeric", "to", "ipv4" and "ipv6" are now rejected.

Signed-off-by: Markus Armbruster 
---
 qapi-schema.json | 21 ++---
 qapi/block-core.json |  2 +-
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 68a4327..b921994 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4051,19 +4051,27 @@
   'data': [ 'all', 'rx', 'tx' ] }
 
 ##
+# @InetSocketAddressBase:
+#
+# @host: host part of the address
+# @port: port part of the address
+##
+{ 'struct': 'InetSocketAddressBase',
+  'data': {
+'host': 'str',
+'port': 'str' } }
+
+##
 # @InetSocketAddress:
 #
 # Captures a socket address or address range in the Internet namespace.
 #
-# @host: host part of the address
-#
-# @port: port part of the address, or lowest port if @to is present
-#
 # @numeric: true if the host/port are guaranteed to be numeric,
 #   false if name resolution should be attempted. Defaults to false.
 #   (Since 2.9)
 #
-# @to: highest port to try
+# @to: If present, this is range of possible addresses, with port
+#  between @port and @to.
 #
 # @ipv4: whether to accept IPv4 addresses, default try both IPv4 and IPv6
 #
@@ -4072,9 +4080,8 @@
 # Since: 1.3
 ##
 { 'struct': 'InetSocketAddress',
+  'base': 'InetSocketAddressBase',
   'data': {
-'host': 'str',
-'port': 'str',
 '*numeric':  'bool',
 '*to': 'uint16',
 '*ipv4': 'bool',
diff --git a/qapi/block-core.json b/qapi/block-core.json
index fe1e40f..a57c9e8 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2641,7 +2641,7 @@
 '*conf': 'str',
 '*snapshot': 'str',
 '*user': 'str',
-'*server': ['InetSocketAddress'],
+'*server': ['InetSocketAddressBase'],
 '*auth-supported': ['RbdAuthMethod'],
 '*password-secret': 'str' } }
 
-- 
2.7.4

[Qemu-devel] [PATCH for-2.9 4/5] rbd: Peel off redundant RbdAuthMethod wrapper struct

[Markus Armbruster]

BlockdevOptionsRbd member auth-supported is a list of struct
RbdAuthMethod, whose only member is enum RbdAuthSupport.  There is no
reason for wrapping the enum in a struct.  Delete the wrapper, and
rename the enum.

Signed-off-by: Markus Armbruster 
---
 block/rbd.c  |  2 +-
 qapi/block-core.json | 15 ++-
 2 files changed, 3 insertions(+), 14 deletions(-)

diff --git a/block/rbd.c b/block/rbd.c
index 8ba0f79..f460d71 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -566,7 +566,7 @@ static char *rbd_auth(QDict *options)
 int i;
 
 for (i = 0;; i++) {
-sprintf(keybuf, "auth-supported.%d.auth", i);
+sprintf(keybuf, "auth-supported.%d", i);
 val = qdict_get(options, keybuf);
 if (!val) {
 break;
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 0f132fc..fe1e40f 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2601,25 +2601,14 @@
 
 
 ##
-# @RbdAuthSupport:
-#
-# An enumeration of RBD auth support
-#
-# Since: 2.9
-##
-{ 'enum': 'RbdAuthSupport',
-  'data': [ 'cephx', 'none' ] }
-
-
-##
 # @RbdAuthMethod:
 #
 # An enumeration of rados auth_supported types
 #
 # Since: 2.9
 ##
-{ 'struct': 'RbdAuthMethod',
-  'data': { 'auth': 'RbdAuthSupport' } }
+{ 'enum': 'RbdAuthMethod',
+  'data': [ 'cephx', 'none' ] }
 
 ##
 # @BlockdevOptionsRbd:
-- 
2.7.4

[Qemu-devel] [PATCH for-2.9 1/5] rbd: Clean up runtime_opts

[Markus Armbruster]

runtime_opts is used for three different purposes:

* qemu_rbd_open() uses it to accept options it recognizes, such as
  "pool" and "image".  Other .bdrv_open() methods do it similarly.

* qemu_rbd_open() accepts additional list-valued options
  auth-supported and server, with the help of qemu_rbd_array_opts().
  The list elements are again dictionaries.  qemu_rbd_array_opts()
  uses runtime_opts to accept their members.  Thus, runtime_opts
  contains recognized sub-sub-options "auth", "host", "port" in
  addition to recognized options.  No other block driver does that.

* qemu_rbd_create() uses it to converts the QDict produced by
  qemu_rbd_parse_filename() to QemuOpts.  No other block driver does
  that.  The keys produced by qemu_rbd_parse_filename() are "pool",
  "image", "snapshot", "conf", "user" and "keyvalue-pairs".
  qemu_rbd_open() accepts these, so no additional ones here.

This is a confusing mess.  Dates back to commit 0f9d252.  First step
to clean it up is documenting runtime_opts.desc[]:

* Reorder entries to match the QAPI schema, like we do in other block
  drivers.

* Document why the schema's "server" and "auth-supported" aren't in
  .desc[].

* Document why "keyvalue-pairs", "host", "port" and "auth" are in
  .desc[], but not the schema.

* Delete "filename", because none of the three users actually uses it.

Signed-off-by: Markus Armbruster 
---
 block/rbd.c | 39 ---
 1 file changed, 24 insertions(+), 15 deletions(-)

diff --git a/block/rbd.c b/block/rbd.c
index ee13f3d..6562fbd 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -365,21 +365,6 @@ static QemuOptsList runtime_opts = {
 .head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head),
 .desc = {
 {
-.name = "filename",
-.type = QEMU_OPT_STRING,
-.help = "Specification of the rbd image",
-},
-{
-.name = "password-secret",
-.type = QEMU_OPT_STRING,
-.help = "ID of secret providing the password",
-},
-{
-.name = "conf",
-.type = QEMU_OPT_STRING,
-.help = "Rados config file location",
-},
-{
 .name = "pool",
 .type = QEMU_OPT_STRING,
 .help = "Rados pool name",
@@ -390,6 +375,11 @@ static QemuOptsList runtime_opts = {
 .help = "Image name in the pool",
 },
 {
+.name = "conf",
+.type = QEMU_OPT_STRING,
+.help = "Rados config file location",
+},
+{
 .name = "snapshot",
 .type = QEMU_OPT_STRING,
 .help = "Ceph snapshot name",
@@ -400,11 +390,30 @@ static QemuOptsList runtime_opts = {
 .type = QEMU_OPT_STRING,
 .help = "Rados id name",
 },
+/*
+ * server.* and auth-supported.* extracted manually, see
+ * qemu_rbd_array_opts()
+ */
+{
+.name = "password-secret",
+.type = QEMU_OPT_STRING,
+.help = "ID of secret providing the password",
+},
+/*
+ * Legacy keys accepted by qemu_rbd_parse_filename(), not in
+ * the QAPI schema
+ */
 {
 .name = "keyvalue-pairs",
 .type = QEMU_OPT_STRING,
 .help = "Legacy rados key/value option parameters",
 },
+/*
+ * The remainder aren't option keys, but option sub-sub-keys,
+ * so that qemu_rbd_array_opts() can abuse runtime_opts for
+ * its own purposes
+ * TODO clean this up
+ */
 {
 .name = "host",
 .type = QEMU_OPT_STRING,
-- 
2.7.4

[Qemu-devel] [PATCH for-2.9 0/5] rbd: Clean up API and code

[Markus Armbruster]

These API cleanups need to go into 2.9.

Markus Armbruster (5):
  rbd: Clean up runtime_opts
  rbd: Clean up qemu_rbd_create()'s detour through QemuOpts
  rbd: Rewrite the code to extract list-valued options
  rbd: Peel off redundant RbdAuthMethod wrapper struct
  rbd: Reject options server.*.{numeric,to,ipv4,ipv6}

 block/rbd.c  | 198 +--
 qapi-schema.json |  21 --
 qapi/block-core.json |  17 +
 3 files changed, 82 insertions(+), 154 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCH for-2.9 3/5] rbd: Rewrite the code to extract list-valued options

[Markus Armbruster]

We have two list-values options:

* "server" is a list of InetSocketAddress.  We use members "host" and
  "port", and silently ignore the rest.

* "auth-supported" is a list of RbdAuthMethod.  We use its only member
  "auth".

Since qemu_rbd_open() takes options as a flattened QDict, options has
keys of the form server.%d.host, server.%d.port and
auth-supported.%d.auth, where %d counts up from zero.

qemu_rbd_array_opts() extracts these values as follows.  First, it
calls qdict_array_entries() to find the list's length.  For each list
element, it first formats the list's key prefix (e.g. "server.0."),
then creates a new QDict holding the options with that key prefix,
then converts that to a QemuOpts, so it can finally get the member
values from there.

If there's one surefire way to make code using QDict more awkward,
it's creating more of them and mixing in QemuOpts for good measure.

The conversion to QemuOpts abuses runtime_opts, as described in the
commit before previous.

Rewrite to simply get the values straight from the options QDict.
This removes the abuse of runtime_opts, so clean it up.

Signed-off-by: Markus Armbruster 
---
 block/rbd.c | 151 +---
 1 file changed, 42 insertions(+), 109 deletions(-)

diff --git a/block/rbd.c b/block/rbd.c
index 59c822a..8ba0f79 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -13,6 +13,7 @@
 
 #include "qemu/osdep.h"
 
+#include 
 #include "qapi/error.h"
 #include "qemu/error-report.h"
 #include "block/block_int.h"
@@ -20,8 +21,6 @@
 #include "qemu/cutils.h"
 #include "qapi/qmp/qstring.h"
 
-#include 
-
 /*
  * When specifying the image filename use:
  *
@@ -408,25 +407,6 @@ static QemuOptsList runtime_opts = {
 .type = QEMU_OPT_STRING,
 .help = "Legacy rados key/value option parameters",
 },
-/*
- * The remainder aren't option keys, but option sub-sub-keys,
- * so that qemu_rbd_array_opts() can abuse runtime_opts for
- * its own purposes
- * TODO clean this up
- */
-{
-.name = "host",
-.type = QEMU_OPT_STRING,
-},
-{
-.name = "port",
-.type = QEMU_OPT_STRING,
-},
-{
-.name = "auth",
-.type = QEMU_OPT_STRING,
-.help = "Supported authentication method, either cephx or none",
-},
 { /* end of list */ }
 },
 };
@@ -577,91 +557,59 @@ static void qemu_rbd_complete_aio(RADOSCB *rcb)
 qemu_aio_unref(acb);
 }
 
-#define RBD_MON_HOST  0
-#define RBD_AUTH_SUPPORTED1
-
-static char *qemu_rbd_array_opts(QDict *options, const char *prefix, int type,
- Error **errp)
+static char *rbd_auth(QDict *options)
 {
-int num_entries;
-QemuOpts *opts = NULL;
-QDict *sub_options;
-const char *host;
-const char *port;
-char *str;
-char *rados_str = NULL;
-Error *local_err = NULL;
+const char **vals = g_new(const char *, qdict_size(options));
+char keybuf[32];
+QObject *val;
+char *rados_str;
 int i;
 
-assert(type == RBD_MON_HOST || type == RBD_AUTH_SUPPORTED);
-
-num_entries = qdict_array_entries(options, prefix);
+for (i = 0;; i++) {
+sprintf(keybuf, "auth-supported.%d.auth", i);
+val = qdict_get(options, keybuf);
+if (!val) {
+break;
+}
 
-if (num_entries < 0) {
-error_setg(errp, "Parse error on RBD QDict array");
-return NULL;
+vals[i] = qstring_get_str(qobject_to_qstring(val));
 }
+vals[i] = NULL;
 
-for (i = 0; i < num_entries; i++) {
-char *strbuf = NULL;
-const char *value;
-char *rados_str_tmp;
-
-str = g_strdup_printf("%s%d.", prefix, i);
-qdict_extract_subqdict(options, &sub_options, str);
-g_free(str);
-
-opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
-qemu_opts_absorb_qdict(opts, sub_options, &local_err);
-QDECREF(sub_options);
-if (local_err) {
-error_propagate(errp, local_err);
-g_free(rados_str);
-rados_str = NULL;
-goto exit;
-}
+rados_str = g_strjoinv(";", (char **)vals);
+g_free(vals);
+return rados_str;
+}
 
-if (type == RBD_MON_HOST) {
-host = qemu_opt_get(opts, "host");
-port = qemu_opt_get(opts, "port");
+static char *rbd_mon_host(QDict *options)
+{
+const char **vals = g_new(const char *, qdict_size(options));
+char keybuf[32];
+QObject *val;
+const char *host, *port;
+char *rados_str;
+int i;
 
-value = host;
-if (port) {
-/* check for ipv6 */
-if (strchr(host, ':')) {
-strbuf = g_strdup_printf("[%s]:%s", host, port);
-} else {
-strbuf = g_strdup_printf("%s:%s", host, port);

[Qemu-devel] [PATCH for-2.9 2/5] rbd: Clean up qemu_rbd_create()'s detour through QemuOpts

[Markus Armbruster]

The conversion from QDict to QemuOpts is pointless.  Simply get the
stuff straight from the QDict.

Signed-off-by: Markus Armbruster 
---
 block/rbd.c | 20 +---
 1 file changed, 5 insertions(+), 15 deletions(-)

diff --git a/block/rbd.c b/block/rbd.c
index 6562fbd..59c822a 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -442,7 +442,6 @@ static int qemu_rbd_create(const char *filename, QemuOpts 
*opts, Error **errp)
 rados_t cluster;
 rados_ioctx_t io_ctx;
 QDict *options = NULL;
-QemuOpts *rbd_opts = NULL;
 int ret = 0;
 
 secretid = qemu_opt_get(opts, "password-secret");
@@ -473,19 +472,11 @@ static int qemu_rbd_create(const char *filename, QemuOpts 
*opts, Error **errp)
 goto exit;
 }
 
-rbd_opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
-qemu_opts_absorb_qdict(rbd_opts, options, &local_err);
-if (local_err) {
-error_propagate(errp, local_err);
-ret = -EINVAL;
-goto exit;
-}
-
-pool   = qemu_opt_get(rbd_opts, "pool");
-conf   = qemu_opt_get(rbd_opts, "conf");
-clientname = qemu_opt_get(rbd_opts, "user");
-name   = qemu_opt_get(rbd_opts, "image");
-keypairs   = qemu_opt_get(rbd_opts, "keyvalue-pairs");
+pool   = qdict_get_str(options, "pool");
+conf   = qdict_get_str(options, "conf");
+clientname = qdict_get_str(options, "user");
+name   = qdict_get_str(options, "image");
+keypairs   = qdict_get_str(options, "keyvalue-pairs");
 
 ret = rados_create(&cluster, clientname);
 if (ret < 0) {
@@ -536,7 +527,6 @@ shutdown:
 
 exit:
 QDECREF(options);
-qemu_opts_del(rbd_opts);
 return ret;
 }
 
-- 
2.7.4

Re: [Qemu-devel] Proposal for deprecating unsupported host OSes & architecutures

[Peter Maydell]

On 23 March 2017 at 10:33, Paolo Bonzini  wrote:
> I own a MIPS Creator ci20 board (donated by Imagination Technologies).
> I cannot give it a public IP address, but I can try and use it to do
> builds every now and then.

I think our best bet is probably to fix our test suite's
overreliance on /tmp/. Then the gcc cfarm box will probably do.

I've also had a kind offer of access to a SPARC box, which
I'm following up on.

thanks
-- PMM

Re: [Qemu-devel] [PULL for-2.9 0/1] Block patches

[Peter Maydell]

On 22 March 2017 at 12:54, Stefan Hajnoczi  wrote:
> The following changes since commit 940a8ce075e3408742a4edcabfd6c2a15e2539eb:
>
>   Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into 
> staging (2017-03-20 16:34:26 +)
>
> are available in the git repository at:
>
>   git://github.com/stefanha/qemu.git tags/block-pull-request
>
> for you to fetch changes up to ff5bbe56c6f9a74c2d77389a21d5d2368458c939:
>
>   parallels: fix default options parsing (2017-03-21 10:02:36 +)
>
> 
>
> 
>
> Edgar Kaziahmedov (1):
>   parallels: fix default options parsing
>
>  block/parallels.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied, thanks.

-- PMM

Re: [Qemu-devel] [Bug 1675108] [NEW] Cocoa UI always crashes on startup

[Alex Bennée]

Peter Maydell  writes:

> On 22 March 2017 at 17:26, Brendan Shanks  wrote:
>> Public bug reported:
>>
>> Commit 8bb93c6f99a42c2e0943bc904b283cd622d302c5 ("ui/console: ensure
>> graphic updates don't race with TCG vCPUs") causes the graphic update to
>> run on a non-main thread, which Cocoa is not happy with. It crashes
>> immediately after startup.
>
> Oops. Alex, we can't just run UI code on random threads like this.

Technically its not a random thread its the vCPU context (which ensures
the vCPU isn't updating while the display is being updated). But I guess
the Cocoa is limited to not being able to update from an arbitrary
thread?

There was a patch posted yesterday to ensure the BQL is held during the
deferred work but this doesn't look like that.

> Any ideas?

Hmm a quick Google seems to imply Cocoa is inflexible in its
requirements. You can try this ugly but untested patch (I don't have any
Macs handy):

modified   ui/console.c
@@ -1598,8 +1598,16 @@ static void dpy_refresh(DisplayState *s)
 QLIST_FOREACH(dcl, &s->listeners, next) {
 if (dcl->ops->dpy_refresh) {
 if (tcg_enabled()) {
+#ifdef CONFIG_COCOA
+qemu_mutex_unlock_iothread();
+start_exclusive();
+do_safe_dpy_refresh(first_cpu, RUN_ON_CPU_HOST_PTR(dcl));
+end_exclusive();
+qemu_mutex_lock_iothread();
+#else
 async_safe_run_on_cpu(first_cpu, do_safe_dpy_refresh,
   RUN_ON_CPU_HOST_PTR(dcl));
+#endif
 } else {
 dcl->ops->dpy_refresh(dcl);
 }


Other than that I guess we need to bring forward the plans to "fixed the dirty 
tracking
races in display adapters"

>
> thanks
> -- PMM


--
Alex Bennée

Re: [Qemu-devel] [Bug 1675108] [NEW] Cocoa UI always crashes on startup

[Peter Maydell]

On 23 March 2017 at 11:13, Alex Bennée  wrote:
> Technically its not a random thread its the vCPU context (which ensures
> the vCPU isn't updating while the display is being updated). But I guess
> the Cocoa is limited to not being able to update from an arbitrary
> thread?

Yes. It's very common for windowing libraries to mandate that you
do all windowing operations from one specific thread.

thanks
-- PMM

[Qemu-devel] [PATCH for-2.9] sockets: Fix socket_address_to_string() hostname truncation

[Markus Armbruster]

We first snprintf() to a fixed buffer, then g_strdup() the result
*boggle*.

Worse, the size of the fixed buffer INET6_ADDRSTRLEN + 5 + 4 is bogus:
the 4 correctly accounts for '[', ']', ':' and '\0', but
INET6_ADDRSTRLEN is not a suitable limit for inet->host, and 5 is not
one for inet->port!  They are for host and port in *numeric* form
(exploiting that INET6_ADDRSTRLEN > INET_ADDRSTRLEN), but inet->host
can also be a hostname, and inet->port can be a service name, to be
resolved with getaddrinfo().

Fortunately, the only user so far is the "socket" network backend's
net_socket_connected(), which uses it to initialize a NetSocketState's
info_str[].  info_str[] has considerable more space: 256 instead of
55.  So the bug's impact appears to be limited to truncated "info
networks" with the "socket" network backend.

The fix is obvious: use g_strdup_printf().

Signed-off-by: Markus Armbruster 
---
 util/qemu-sockets.c | 9 ++---
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 7c120c4..40164bf 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -1307,19 +1307,14 @@ char *socket_address_to_string(struct SocketAddress 
*addr, Error **errp)
 {
 char *buf;
 InetSocketAddress *inet;
-char host_port[INET6_ADDRSTRLEN + 5 + 4];
 
 switch (addr->type) {
 case SOCKET_ADDRESS_KIND_INET:
 inet = addr->u.inet.data;
 if (strchr(inet->host, ':') == NULL) {
-snprintf(host_port, sizeof(host_port), "%s:%s", inet->host,
-inet->port);
-buf = g_strdup(host_port);
+buf = g_strdup_printf("%s:%s", inet->host, inet->port);
 } else {
-snprintf(host_port, sizeof(host_port), "[%s]:%s", inet->host,
-inet->port);
-buf = g_strdup(host_port);
+buf = g_strdup_printf("[%s]:%s", inet->host, inet->port);
 }
 break;
 
-- 
2.7.4

Re: [Qemu-devel] [RFC PATCH v4 02/20] memattrs: add debug attribute

[Stefan Hajnoczi]

On Wed, Mar 08, 2017 at 03:51:28PM -0500, Brijesh Singh wrote:
> Add a new debug attribute, the attribute should be set when guest memory
> accesses are performed for debug purposes.
> The attribute will be used in SEV guest, where we need to distinguish normal
> vs debug access to guest memory. In debug mode, we need to use SEV commands
> to access the guest memory.
> 
> Signed-off-by: Brijesh Singh 
> ---
>  include/exec/memattrs.h |4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/include/exec/memattrs.h b/include/exec/memattrs.h
> index e601061..b802073 100644
> --- a/include/exec/memattrs.h
> +++ b/include/exec/memattrs.h
> @@ -37,6 +37,8 @@ typedef struct MemTxAttrs {
>  unsigned int user:1;
>  /* Requester ID (for MSI for example) */
>  unsigned int requester_id:16;
> +/* Memory access for debug purposes */

What does "debug purposes" mean?  gdbstub?  Can the guest initiate debug
memory accesses or is the purely QEMU-internal?

> +unsigned int debug:1;
>  } MemTxAttrs;
>  
>  /* Bus masters which don't specify any attributes will get this,
> @@ -46,4 +48,6 @@ typedef struct MemTxAttrs {
>   */
>  #define MEMTXATTRS_UNSPECIFIED ((MemTxAttrs) { .unspecified = 1 })
>  
> +/* Access the guest memory for debug purposes */
> +#define MEMTXATTRS_DEBUG ((MemTxAttrs) { .debug = 1 })
>  #endif
> 
> 


signature.asc
Description: PGP signature

Re: [Qemu-devel] [Bug 1675108] [NEW] Cocoa UI always crashes on startup

[Alex Bennée]

Peter Maydell  writes:

> On 23 March 2017 at 11:13, Alex Bennée  wrote:
>> Technically its not a random thread its the vCPU context (which ensures
>> the vCPU isn't updating while the display is being updated). But I guess
>> the Cocoa is limited to not being able to update from an arbitrary
>> thread?
>
> Yes. It's very common for windowing libraries to mandate that you
> do all windowing operations from one specific thread.

Fair enough. Well let me know if that works OK on MacOS and I'll fold it
in to the other console patches. In fact I might as well do the
start/end_exclusive dance for all OSes and it will achieve the same thing.

--
Alex Bennée

[Qemu-devel] [PATCH] slirp: tftp, copy sockaddr_size

[Marc-André Lureau]

ASAN detects an "unknown-crash" when running pxe-test:

/ppc64/pxe/spapr-vlan: 
=
==7143==ERROR: AddressSanitizer: unknown-crash on address 0x7f6dcd298d30 at pc 
0x55e22218830d bp 0x7f6dcd2989e0 sp 0x7f6dcd2989d0
READ of size 128 at 0x7f6dcd298d30 thread T2
#0 0x55e22218830c in tftp_session_allocate 
/home/elmarco/src/qq/slirp/tftp.c:73
#1 0x55e22218a1f8 in tftp_handle_rrq /home/elmarco/src/qq/slirp/tftp.c:289
#2 0x55e22218b54c in tftp_input /home/elmarco/src/qq/slirp/tftp.c:446
#3 0x55e2221833fe in udp6_input /home/elmarco/src/qq/slirp/udp6.c:82
#4 0x55e222137b17 in ip6_input /home/elmarco/src/qq/slirp/ip6_input.c:67

Address 0x7f6dcd298d30 is located in stack of thread T2 at offset 96 in frame
#0 0x55e222182420 in udp6_input /home/elmarco/src/qq/slirp/udp6.c:13

  This frame has 3 object(s):
[32, 48) ''
[96, 124) 'lhost' <== Memory access at offset 96 partially overflows this 
variable
[160, 200) 'save_ip' <== Memory access at offset 96 partially underflows 
this variable

The sockaddr_storage pointer is the sockaddr_in6 lhost on the
stack. Copy only the source addr size.

Signed-off-by: Marc-André Lureau 
---
 slirp/tftp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/slirp/tftp.c b/slirp/tftp.c
index 50e714807d..a9bc4bb1b6 100644
--- a/slirp/tftp.c
+++ b/slirp/tftp.c
@@ -70,7 +70,7 @@ static int tftp_session_allocate(Slirp *slirp, struct 
sockaddr_storage *srcsas,
 
  found:
   memset(spt, 0, sizeof(*spt));
-  spt->client_addr = *srcsas;
+  memcpy(&spt->client_addr, srcsas, sockaddr_size(srcsas));
   spt->fd = -1;
   spt->block_size = 512;
   spt->client_port = tp->udp.uh_sport;
-- 
2.12.0.191.gc5d8de91d

Re: [Qemu-devel] [RFC PATCH v4 06/20] core: add new security-policy object

[Stefan Hajnoczi]

On Wed, Mar 08, 2017 at 03:52:09PM -0500, Brijesh Singh wrote:
> The object can be used to define global security policy for the guest.

"security-policy" is very vague.  Lots of parts of QEMU have security
related options (e.g. VNC display, networking, etc).

I'd prefer a
-machine memory-encryption=on|off,memory-encryption-debug=on|off
or -m encryption=on|off,encryption-debug=on|off switch instead of a new
security policy object with questionable scope.

> object provides two properties:
> 
>  1) debug: can be used to disable guest memory access from hypervisor.
> 
>e.g to disable guest memory debug accesses
> 
> # $QEMU \
>   -object security-policy,debug=false,id=mypolicy \
>   -machine ...,security-policy=mypolicy
> 
>  2) memory-encryption: if hypervisor supports memory encryption then this
> property can be used to define object for encryption.
> 
> # $QEMU \
> -object sev-guest,id=sev0 \
> -object security-policy,id=memory-encryption=sev0,id=mypolicy \

s/id=memory-encryption=/id=mypolicy,memory-encryption=/

> -machine ...,security-policy=mypolicy
> 
> The memory-encryption property will be used for enabling AMD's SEV feature.
> 
> Signed-off-by: Brijesh Singh 
> ---
>  exec.c   |7 ++
>  hw/core/Makefile.objs|1 
>  hw/core/machine.c|   22 +
>  hw/core/security-policy.c|  165 
> ++
>  include/hw/boards.h  |1 
>  include/sysemu/security-policy.h |   75 +
>  qemu-options.hx  |   21 +
>  7 files changed, 292 insertions(+)
>  create mode 100644 hw/core/security-policy.c
>  create mode 100644 include/sysemu/security-policy.h
> 
> diff --git a/exec.c b/exec.c
> index 772a959..2c7c891 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -40,6 +40,7 @@
>  #else /* !CONFIG_USER_ONLY */
>  #include "hw/hw.h"
>  #include "exec/memory.h"
> +#include "sysemu/security-policy.h"
>  #include "exec/ioport.h"
>  #include "sysemu/dma.h"
>  #include "sysemu/numa.h"
> @@ -2926,6 +2927,12 @@ static inline void 
> cpu_physical_memory_rw_debug_internal(AddressSpace *as,
>  hwaddr addr1;
>  MemoryRegion *mr;
>  
> +/* Check if debug accesses is allowed */
> +if (attrs.debug &&
> +!security_policy_debug_allowed(current_machine->security_policy)) {
> +return;
> +}
> +
>  rcu_read_lock();
>  while (len > 0) {
>  l = len;
> diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs
> index 91450b2..3c413b1 100644
> --- a/hw/core/Makefile.objs
> +++ b/hw/core/Makefile.objs
> @@ -18,6 +18,7 @@ common-obj-$(CONFIG_SOFTMMU) += qdev-properties-system.o
>  common-obj-$(CONFIG_SOFTMMU) += register.o
>  common-obj-$(CONFIG_SOFTMMU) += or-irq.o
>  common-obj-$(CONFIG_PLATFORM_BUS) += platform-bus.o
> +common-obj-$(CONFIG_SOFTMMU) += security-policy.o
>  
>  obj-$(CONFIG_SOFTMMU) += generic-loader.o
>  obj-$(CONFIG_SOFTMMU) += null-machine.o
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 0699750..c14f59c 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -332,6 +332,23 @@ static bool machine_get_enforce_config_section(Object 
> *obj, Error **errp)
>  return ms->enforce_config_section;
>  }
>  
> +static char *machine_get_security_policy(Object *obj, Error **errp)
> +{
> +MachineState *ms = MACHINE(obj);
> +
> +return g_strdup(ms->security_policy);
> +}
> +
> +static void machine_set_security_policy(Object *obj,
> +const char *value, Error **errp)
> +{
> +MachineState *ms = MACHINE(obj);
> +
> +g_free(ms->security_policy);
> +ms->security_policy = g_strdup(value);
> +}
> +
> +
>  static void error_on_sysbus_device(SysBusDevice *sbdev, void *opaque)
>  {
>  error_report("Option '-device %s' cannot be handled by this machine",
> @@ -493,6 +510,11 @@ static void machine_class_init(ObjectClass *oc, void 
> *data)
>  &error_abort);
>  object_class_property_set_description(oc, "enforce-config-section",
>  "Set on to enforce configuration section migration", &error_abort);
> +
> +object_class_property_add_str(oc, "security-policy",
> +machine_get_security_policy, machine_set_security_policy, NULL);
> +object_class_property_set_description(oc, "security-policy",
> +"Set the security policy for the machine", NULL);
>  }
>  
>  static void machine_class_base_init(ObjectClass *oc, void *data)
> diff --git a/hw/core/security-policy.c b/hw/core/security-policy.c
> new file mode 100644
> index 000..4d4658e
> --- /dev/null
> +++ b/hw/core/security-policy.c
> @@ -0,0 +1,165 @@
> +/*
> + * QEMU security policy support
> + *
> + * Copyright (c) 2016 Advanced Micro Devices
> + *
> + * Author:
> + *  Brijesh Singh 
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> 

Re: [Qemu-devel] [PULL 0/4] virtio, pc: fixes

[Peter Maydell]

On 22 March 2017 at 16:01, Michael S. Tsirkin  wrote:
> The following changes since commit 55a19ad8b2d0797e3a8fe90ab99a9bb713824059:
>
>   Update version for v2.9.0-rc1 release (2017-03-21 17:13:29 +)
>
> are available in the git repository at:
>
>   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
>
> for you to fetch changes up to 807211b76603b094c46a0cd95a3c785f47f3ab73:
>
>   hw/acpi/vmgenid: prevent more than one vmgenid device (2017-03-22 17:56:00 
> +0200)
>
> 
> virtio, pc: fixes
>
> virtio and misc fixes for 2.9.
>
> Signed-off-by: Michael S. Tsirkin 
>
> 

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PATCH for-2.9] sockets: Fix socket_address_to_string() hostname truncation

[Paolo Bonzini]

On 23/03/2017 12:23, Markus Armbruster wrote:
> We first snprintf() to a fixed buffer, then g_strdup() the result
> *boggle*.
> 
> Worse, the size of the fixed buffer INET6_ADDRSTRLEN + 5 + 4 is bogus:
> the 4 correctly accounts for '[', ']', ':' and '\0', but
> INET6_ADDRSTRLEN is not a suitable limit for inet->host, and 5 is not
> one for inet->port!  They are for host and port in *numeric* form
> (exploiting that INET6_ADDRSTRLEN > INET_ADDRSTRLEN), but inet->host
> can also be a hostname, and inet->port can be a service name, to be
> resolved with getaddrinfo().
> 
> Fortunately, the only user so far is the "socket" network backend's
> net_socket_connected(), which uses it to initialize a NetSocketState's
> info_str[].  info_str[] has considerable more space: 256 instead of
> 55.  So the bug's impact appears to be limited to truncated "info
> networks" with the "socket" network backend.
> 
> The fix is obvious: use g_strdup_printf().
> 
> Signed-off-by: Markus Armbruster 
> ---
>  util/qemu-sockets.c | 9 ++---
>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> index 7c120c4..40164bf 100644
> --- a/util/qemu-sockets.c
> +++ b/util/qemu-sockets.c
> @@ -1307,19 +1307,14 @@ char *socket_address_to_string(struct SocketAddress 
> *addr, Error **errp)
>  {
>  char *buf;
>  InetSocketAddress *inet;
> -char host_port[INET6_ADDRSTRLEN + 5 + 4];

/me learnt about INET6_ADDRSTRLEN today.

>  switch (addr->type) {
>  case SOCKET_ADDRESS_KIND_INET:
>  inet = addr->u.inet.data;
>  if (strchr(inet->host, ':') == NULL) {
> -snprintf(host_port, sizeof(host_port), "%s:%s", inet->host,
> -inet->port);
> -buf = g_strdup(host_port);
> +buf = g_strdup_printf("%s:%s", inet->host, inet->port);
>  } else {
> -snprintf(host_port, sizeof(host_port), "[%s]:%s", inet->host,
> -inet->port);
> -buf = g_strdup(host_port);
> +buf = g_strdup_printf("[%s]:%s", inet->host, inet->port);
>  }
>  break;
>  
> 

Reviewed-by: Paolo Bonzini 

Re: [Qemu-devel] [PATCH] Revert "apic: save apic_delivered flag"

[Paolo Bonzini]

On 23/03/2017 08:34, Pavel Dovgalyuk wrote:
> This value is used by mc146818rtc.
> Therefore it affects the vitrual machine state.

It is used indeed, but it should not be used across virtual machine
migration or savevm.  The value doesn't matter as soon as
apic_get_irq_delivered is called, because there will be an
apic_reset_irq_delivered call before the next access; and when migrating
or saving a virtual machine, you cannot be between the calls to
apic_reset_irq_delivered and apic_get_irq_delivered.

It would be interesting to try something like this:

diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index 7a6e771..10a114d 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -32,6 +32,7 @@
 #include "hw/sysbus.h"

 static int apic_irq_delivered;
+static bool apic_irq_delivered_valid;
 bool apic_report_tpr_access;

 void cpu_set_apic_base(DeviceState *dev, uint64_t val)
@@ -136,13 +137,18 @@ void apic_reset_irq_delivered(void)
 volatile int a_i_d = apic_irq_delivered;
 trace_apic_reset_irq_delivered(a_i_d);

+assert(!apic_irq_delivered_valid);
 apic_irq_delivered = 0;
+apic_irq_delivered_valid = true;
 }

 int apic_get_irq_delivered(void)
 {
 trace_apic_get_irq_delivered(apic_irq_delivered);

+assert(apic_irq_delivered_valid);
+apic_irq_delivered_valid = false;
+
 return apic_irq_delivered;
 }


Paolo

> I've encountered the cases when replay was broken without migrating of
> this variable.
> Отправлено с помощью BlueMail 
> На 22 Мар 2017 г., в 15:13, Paolo Bonzini  > написал:
> 
> This reverts commit 07bfa354772f2de67008dc66c201b627acff0106.
> The global variable is only read as part of a
> 
> apic_reset_irq_delivered();
> qemu_irq_raise(s->irq);
> if (!apic_get_irq_delivered()) {
> 
> sequence, so the value never matters at migration time.
> 
> Reported-by: Dr. David Alan Gilbert 
> Cc: Pavel Dovgalyuk 
> Signed-off-by: Paolo Bonzini 
> ---
>  hw/intc/apic_common.c   | 33
> 
> 
>  include/hw/i386/apic_internal.h |  2 --
>  2 files changed, 35 deletions(-)
> 
> diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
> index 7a6e771..c3829e3 100644
> --- a/hw/intc/apic_common.c
> +++ b/hw/intc/apic_common.c
> @@ -387,25 +387,6 @@ static bool apic_common_sipi_needed(void *opaque)
>  return s->wait_for_sipi != 0;
>  }
>  
> -static bool apic_irq_delivered_needed(void *opaque)
> -{
> -APICCommonState *s = APIC_COMMON(opaque);
> -return s->cpu == X86_CPU(first_cpu) && apic_irq_delivered != 0;
> -}
> -
> -static void apic_irq_delivered_pre_save(void *opaque)
> -{
> -APICCommonState *s = APIC_COMMON(opaque);
> -s->apic_irq_delivered = apic_irq_delivered;
> -}
> -
> -static int apic_irq_delivered_post_load(void *opaque, int version_id)
> -{
> -APICCommonState *s = APIC_COMMON(opaque);
> -apic_irq_delivered = s->apic_irq_delivered;
> -return 0;
> -}
> -
>  static const VMStateDescription vmstate_apic_common_sipi = {
>  .name = "apic_sipi",
>  .version_id = 1,
> @@ -418,19 +399,6 @@ static const VMStateDescription 
> vmstate_apic_common_sipi = {
>  }
>  };
>  
> -static const VMStateDescription vmstate_apic_irq_delivered = {
> -.name = "apic_irq_delivered",
> -.version_id = 1,
> -.minimum_version_id = 1,
> -.needed = apic_irq_delivered_needed,
> -.pre_save = apic_irq_delivered_pre_save,
> -.post_load = apic_irq_delivered_post_load,
> -.fields = (VMStateField[]) {
> -VMSTATE_INT32(apic_irq_delivered, APICCommonState),
> -VMSTATE_END_OF_LIST()
> -}
> -};
> -
>  static const VMStateDescription vmstate_apic_common = {
>  .name = "apic",
>  .version_id = 3,
> @@ -465,7 +433,6 @@ static const VMStateDescription vmstate_apic_common = 
> {
>  },
>  .subsections = (const VMStateDescription*[]) {
>  &vmstate_apic_common_sipi,
> -&vmstate_apic_irq_delivered,
>  NULL
>  }
>  };
> diff --git a/include/hw/i386/apic_internal.h 
> b/include/hw/i386/apic_internal.h
> index 20ad28c..1209eb4 100644
> --- a/include/hw/i386/apic_internal.h
> +++ b/include/hw/i386/apic_internal.h
> @@ -189,8 +189,6 @@ struct APICCommonState {
>  DeviceState *vapic;
>  hwaddr vapic_paddr; /* note: persistence via kvmvapic */
>  bool legacy_instance_id;
> -
> -int apic_irq_delivered; /* for saving static variable */
>  };
>  
>  typedef struct VAPICState {
> 

Re: [Qemu-devel] [PATCH v4 01/16] s390: cio: introduce cio_cancel_halt_clear

[Sebastian Ott]

On Fri, 17 Mar 2017, Dong Jia Shi wrote:
> For future code reuse purpose, this decouples the cio code with
> the ccw device specific parts from ccw_device_cancel_halt_clear,
> and makes a new common I/O interface named cio_cancel_halt_clear.
> 
> Reviewed-by: Pierre Morel 
> Signed-off-by: Dong Jia Shi 
> Cc: Sebastian Ott 
> Cc: Peter Oberparleiter 
[...]
> +/**
> + * cio_cancel_halt_clear - Cancel running I/O by performing cancel, halt
> + * and clear ordinally if subchannel is valid.
> + * @sch: subchannel on which to perform the cancel_halt_clear operation
> + * @iretry: the number of the times remained to retry the next operation
> + *
> + * This should be called repeatedly since halt/clear are asynchronous
> + * operations. We do one try with cio_cancel, two tries with cio_halt,
 ^
 three

Acked-by: Sebastian Ott 

Re: [Qemu-devel] [PATCH v4 02/16] s390: cio: export more interfaces

[Sebastian Ott]

On Fri, 17 Mar 2017, Dong Jia Shi wrote:
> Export the common I/O interfaces those are needed by an I/O
> subchannel driver to actually talk to the subchannel.
> 
> Reviewed-by: Pierre Morel 
> Signed-off-by: Dong Jia Shi 
> Cc: Sebastian Ott 
> Cc: Peter Oberparleiter 
> ---

Acked-by: Sebastian Ott 

Re: [Qemu-devel] [PATCH v0] fsdev: QMP interface for throttling

[Pradeep Jagadeesh]

On 3/21/2017 1:06 PM, Greg Kurz wrote:

On Tue, 21 Mar 2017 10:44:32 +0100
Pradeep Jagadeesh  wrote:


Hi Eric,

Thanks for having a look at the patch. My answers are inline.


On 03/20/2017 08:07 AM, Pradeep Jagadeesh wrote:

This patchset enables qmp interfaces for the 9pfs
devices (fsdev).This provides two interfaces one


Space between English sentences, after '.'

OK



for querying all the 9pfs devices info. The second one
to set the IO limits for the required 9pfs device.

Signed-off-by: Pradeep Jagadeesh 
---



+++ b/qapi-schema.json
@@ -81,6 +81,9 @@
 # QAPI block definitions
 { 'include': 'qapi/block.json' }

+# QAPI 9pfs definitions
+{ 'include': 'qapi/9pfs.json' }
+
 # QAPI event definitions
 { 'include': 'qapi/event.json' }

diff --git a/qapi/9pfs.json b/qapi/9pfs.json
new file mode 100644
index 000..c068474
--- /dev/null
+++ b/qapi/9pfs.json
@@ -0,0 +1,169 @@
+# -*- Mode: Python -*-
+
+##
+# == QAPI 9p definitions
+##
+
+# QAPI common definitions
+{ 'include': 'common.json' }
+
+##
+# @fs9p_set_io_throttle:
+#
+# Change I/O limits for a 9p/fsdev device.
+#
+# Since QEMU 2.9, I/0 limits can be enabled on each  fsdev(9pfs) device


This says 2.9...

I meant that, the qemu cli io throttle facility for 9p/fsdev is enabled
in 2.9.But the qmp interfaces are from 2.10.


QMP users don't care about the cli API. The only important thing is:

Since: 2.10

The curious will find out about the background in git log, no need to
mention this in the *code*.

OK





+#
+# I/O limits can be disabled by setting all of them to 0.
+#
+# Returns: Nothing on success
+#  If @device is not a valid 9p device, DeviceNotFound
+#
+# Since: 2:10


...but this says 2:10 (typo, should be 2.10).  No need to mention the
version twice, especially if one of them is wrong (keep the Since: line).

OK, mentioned above.



+#
+# Example:
+#
+# -> { "execute": "fs9p_set_io_throttle",
+#  "arguments": { "device": "ide0-1-0",
+# "bps": 100,
+# "bps_rd": 0,
+# "bps_wr": 0,
+# "iops": 0,
+# "iops_rd": 0,
+# "iops_wr": 0,
+# "bps_max": 800,
+# "bps_rd_max": 0,
+# "bps_wr_max": 0,
+# "iops_max": 0,
+# "iops_rd_max": 0,
+# "iops_wr_max": 0,
+# "bps_max_length": 60,
+# "iops_size": 0 } }
+# <- { "returns": {} }
+##
+{ 'command': 'fs9p_set_io_throttle', 'boxed': true,
+  'data': 'FS9PIOThrottle' }


New commands and members should be named with '-' rather than '_' as the
word separator, so this should be 'fs9p-set-io-throttle', 'bps-rd', etc.

OK, I will change.


+##
+# @FS9PIOThrottle:
+#
+# A set of parameters describing block
+#
+# @device: Block device name
+#
+# @bps: total throughput limit in bytes per second
+#
+# @bps_rd: read throughput limit in bytes per second
+#
+# @bps_wr: write throughput limit in bytes per second
+#
+# @iops: total I/O operations per second
+#
+# @iops_rd: read I/O operations per second
+#
+# @iops_wr: write I/O operations per second
+#
+# @bps_max: total throughput limit during bursts,
+# in bytes (Since 1.7)


You're introducing this struct in 2.10, so this member is not since 1.7.
 Either that, or you're copying-and-pasting when you should be sharing
code and reusing an existing struct.

Hmm..copied the block devices code, I will correct it.
I thought of reusing the code, but the whole struct from block devices
can not be used, as there is one member called "group" that is not used
in case of 9p. Also this needs lot of changes even in case of block
devices. Because I may need to rename the structure as IOThrottle or
something like that.
Shall I reuse the code and avoid setting the group member in case of 9p?
What do you think?


The code factoring would affect:
- hmp_9pfs_set_io_throttle() which looks identical to the existing
  hmp_block_set_io_throttle() function
- a bunch of lines to handle the throttle arguments in fsdev_set_io_throttle()
  which are the same as in qmp_block_set_io_throttle(). And BTW, a similar
  refactoring seems doable in the cli API between fsdev_throttle_parse_opts()
  and extract_common_blockdev_options().

Yes, I am going to take care of this.

This being said, I don't know what this would mean with json files.

As of now IOThrottle structure I have put into a different file.






+#
+# Since: 2.10
+##
+{ 'struct': 'FS9PIOThrottle',
+  'data': { '*device': 'str', 'bps': 'int', 'bps_rd': 'int',
+'bps_wr': 'int', 'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int',
+'*bps_max': 'int', '*bps_rd_max': 'int',
+'*bps_wr_max': 'int', '*iops_max': 'int',
+'*iops_rd_max': 'int', '*iops_wr_max': 'int',
+'*bps_max_length': 'int', '*bps_rd_max_length': 'int',
+'*bps_wr_max_length': 'int', '*iops_max_

[Qemu-devel] [PATCH 1/2 v1] fsdev: QMP interface for throttling

[Pradeep Jagadeesh]

This patchset enables qmp interfaces for the 9pfs 
devices (fsdev). This provides two interfaces one 
for querying all the 9pfs devices info. The second one
to set the IO limits for the required 9pfs device.

Signed-off-by: Pradeep Jagadeesh 
---
 Makefile|  5 ++-
 fsdev/qemu-fsdev-throttle.c | 96 +
 fsdev/qemu-fsdev-throttle.h | 11 ++
 fsdev/qemu-fsdev.c  | 38 +-
 fsdev/qemu-fsdev.h  |  2 +-
 hmp-commands-info.hx| 18 +
 hmp-commands.hx | 18 +
 hmp.c   | 74 ++
 hmp.h   |  5 +++
 qapi-schema.json|  6 +++
 qapi/fsdev.json | 87 
 qapi/iothrottle.json| 93 +++
 qmp.c   | 14 +++
 13 files changed, 464 insertions(+), 3 deletions(-)
 create mode 100644 qapi/fsdev.json
 create mode 100644 qapi/iothrottle.json

v0 -> v1:

   Addressed the comments by Erik Blake, Greg Kurz and Daniel P.Berrange
   Mainly renaming the functions and removing the redundant code

diff --git a/Makefile b/Makefile
index 73e0c12..c33b46d 100644
--- a/Makefile
+++ b/Makefile
@@ -413,7 +413,10 @@ qapi-modules = $(SRC_PATH)/qapi-schema.json 
$(SRC_PATH)/qapi/common.json \
$(SRC_PATH)/qapi/block.json $(SRC_PATH)/qapi/block-core.json \
$(SRC_PATH)/qapi/event.json $(SRC_PATH)/qapi/introspect.json \
$(SRC_PATH)/qapi/crypto.json $(SRC_PATH)/qapi/rocker.json \
-   $(SRC_PATH)/qapi/trace.json
+   $(SRC_PATH)/qapi/trace.json  $(SRC_PATH)/qapi/iothrottle.json
+ifdef CONFIG_VIRTFS
+qapi-modules += $(SRC_PATH)/qapi/fsdev.json
+endif
 
 qapi-types.c qapi-types.h :\
 $(qapi-modules) $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
diff --git a/fsdev/qemu-fsdev-throttle.c b/fsdev/qemu-fsdev-throttle.c
index 7ae4e86..e3ea095 100644
--- a/fsdev/qemu-fsdev-throttle.c
+++ b/fsdev/qemu-fsdev-throttle.c
@@ -29,6 +29,102 @@ static void fsdev_throttle_write_timer_cb(void *opaque)
 qemu_co_enter_next(&fst->throttled_reqs[true]);
 }
 
+void fsdev_set_io_throttle(IOThrottle *arg, FsThrottle *fst, Error **errp)
+{
+ThrottleConfig cfg;
+
+throttle_config_init(&cfg);
+cfg.buckets[THROTTLE_BPS_TOTAL].avg = arg->bps;
+cfg.buckets[THROTTLE_BPS_READ].avg  = arg->bps_rd;
+cfg.buckets[THROTTLE_BPS_WRITE].avg = arg->bps_wr;
+
+cfg.buckets[THROTTLE_OPS_TOTAL].avg = arg->iops;
+cfg.buckets[THROTTLE_OPS_READ].avg  = arg->iops_rd;
+cfg.buckets[THROTTLE_OPS_WRITE].avg = arg->iops_wr;
+
+if (arg->has_bps_max) {
+cfg.buckets[THROTTLE_BPS_TOTAL].max = arg->bps_max;
+}
+if (arg->has_bps_rd_max) {
+cfg.buckets[THROTTLE_BPS_READ].max = arg->bps_rd_max;
+}
+if (arg->has_bps_wr_max) {
+cfg.buckets[THROTTLE_BPS_WRITE].max = arg->bps_wr_max;
+}
+if (arg->has_iops_max) {
+cfg.buckets[THROTTLE_OPS_TOTAL].max = arg->iops_max;
+}
+if (arg->has_iops_rd_max) {
+cfg.buckets[THROTTLE_OPS_READ].max = arg->iops_rd_max;
+}
+if (arg->has_iops_wr_max) {
+cfg.buckets[THROTTLE_OPS_WRITE].max = arg->iops_wr_max;
+}
+
+if (arg->has_bps_max_length) {
+cfg.buckets[THROTTLE_BPS_TOTAL].burst_length = arg->bps_max_length;
+}
+if (arg->has_bps_rd_max_length) {
+cfg.buckets[THROTTLE_BPS_READ].burst_length = arg->bps_rd_max_length;
+}
+if (arg->has_bps_wr_max_length) {
+cfg.buckets[THROTTLE_BPS_WRITE].burst_length = arg->bps_wr_max_length;
+}
+if (arg->has_iops_max_length) {
+cfg.buckets[THROTTLE_OPS_TOTAL].burst_length = arg->iops_max_length;
+}
+if (arg->has_iops_rd_max_length) {
+cfg.buckets[THROTTLE_OPS_READ].burst_length = arg->iops_rd_max_length;
+}
+if (arg->has_iops_wr_max_length) {
+cfg.buckets[THROTTLE_OPS_WRITE].burst_length = arg->iops_wr_max_length;
+}
+
+if (arg->has_iops_size) {
+cfg.op_size = arg->iops_size;
+}
+
+if (throttle_is_valid(&cfg, errp)) {
+fst->cfg = cfg;
+fsdev_throttle_init(fst);
+}
+}
+
+void fsdev_get_io_throttle(FsThrottle *fst, IOThrottle **fs9pcfg,
+   char *fsdevice, Error **errp)
+{
+
+ThrottleConfig cfg = fst->cfg;
+IOThrottle *fscfg = g_malloc0(sizeof(*fscfg));
+
+fscfg->has_device = true;
+fscfg->device = g_strdup(fsdevice);
+fscfg->bps = cfg.buckets[THROTTLE_BPS_TOTAL].avg;
+fscfg->bps_rd = cfg.buckets[THROTTLE_BPS_READ].avg;
+fscfg->bps_wr = cfg.buckets[THROTTLE_BPS_WRITE].avg;
+
+fscfg->iops = cfg.buckets[THROTTLE_OPS_TOTAL].avg;
+fscfg->iops_rd = cfg.buckets[THROTTLE_OPS_READ].avg;
+fscfg->iops_wr = cfg.buckets[THROTTLE_OPS_WRITE].avg;
+
+fscfg->bps_max = cfg.buckets[THROTTLE_BPS_TOTAL].max;
+fscfg->bps_rd_max = cfg.buckets[THROTTLE_BPS

[Qemu-devel] [PATCH 2/2 v1] throttle: factor out the redundant code

[Pradeep Jagadeesh]

This patchset removes the throttle redundant code that was present
in block and fsdev files.

Signed-off-by: Pradeep Jagadeesh 
---
 blockdev.c  | 106 ---
 fsdev/Makefile.objs |   1 +
 fsdev/qemu-fsdev-throttle.c |  94 +-
 fsdev/qemu-fsdev-throttle.h |   3 +-
 hmp.c   |  39 +++
 include/qemu/throttle-options.h |   5 ++
 qapi/block-core.json|  76 +---
 qapi/iothrottle.json|   4 +-
 util/Makefile.objs  |   1 +
 util/throttle-options.c | 108 
 10 files changed, 151 insertions(+), 286 deletions(-)
 create mode 100644 util/throttle-options.c

diff --git a/blockdev.c b/blockdev.c
index c5b2c2c..2f4ec3a 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -386,49 +386,7 @@ static void extract_common_blockdev_options(QemuOpts 
*opts, int *bdrv_flags,
 }
 
 if (throttle_cfg) {
-throttle_config_init(throttle_cfg);
-throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg =
-qemu_opt_get_number(opts, "throttling.bps-total", 0);
-throttle_cfg->buckets[THROTTLE_BPS_READ].avg  =
-qemu_opt_get_number(opts, "throttling.bps-read", 0);
-throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg =
-qemu_opt_get_number(opts, "throttling.bps-write", 0);
-throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg =
-qemu_opt_get_number(opts, "throttling.iops-total", 0);
-throttle_cfg->buckets[THROTTLE_OPS_READ].avg =
-qemu_opt_get_number(opts, "throttling.iops-read", 0);
-throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg =
-qemu_opt_get_number(opts, "throttling.iops-write", 0);
-
-throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max =
-qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
-throttle_cfg->buckets[THROTTLE_BPS_READ].max  =
-qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
-throttle_cfg->buckets[THROTTLE_BPS_WRITE].max =
-qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
-throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max =
-qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
-throttle_cfg->buckets[THROTTLE_OPS_READ].max =
-qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
-throttle_cfg->buckets[THROTTLE_OPS_WRITE].max =
-qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
-
-throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length =
-qemu_opt_get_number(opts, "throttling.bps-total-max-length", 1);
-throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length  =
-qemu_opt_get_number(opts, "throttling.bps-read-max-length", 1);
-throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length =
-qemu_opt_get_number(opts, "throttling.bps-write-max-length", 1);
-throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length =
-qemu_opt_get_number(opts, "throttling.iops-total-max-length", 1);
-throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length =
-qemu_opt_get_number(opts, "throttling.iops-read-max-length", 1);
-throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length =
-qemu_opt_get_number(opts, "throttling.iops-write-max-length", 1);
-
-throttle_cfg->op_size =
-qemu_opt_get_number(opts, "throttling.iops-size", 0);
-
+parse_io_throttle_options(throttle_cfg, opts);
 if (!throttle_is_valid(throttle_cfg, errp)) {
 return;
 }
@@ -2635,9 +2593,12 @@ void qmp_block_set_io_throttle(BlockIOThrottle *arg, 
Error **errp)
 BlockDriverState *bs;
 BlockBackend *blk;
 AioContext *aio_context;
+IOThrottle *iothrottle;
+
+iothrottle = arg->iothrottle;
 
-blk = qmp_get_blk(arg->has_device ? arg->device : NULL,
-  arg->has_id ? arg->id : NULL,
+blk = qmp_get_blk(iothrottle->has_device ? iothrottle->device : NULL,
+  iothrottle->has_id ? iothrottle->id : NULL,
   errp);
 if (!blk) {
 return;
@@ -2652,56 +2613,7 @@ void qmp_block_set_io_throttle(BlockIOThrottle *arg, 
Error **errp)
 goto out;
 }
 
-throttle_config_init(&cfg);
-cfg.buckets[THROTTLE_BPS_TOTAL].avg = arg->bps;
-cfg.buckets[THROTTLE_BPS_READ].avg  = arg->bps_rd;
-cfg.buckets[THROTTLE_BPS_WRITE].avg = arg->bps_wr;
-
-cfg.buckets[THROTTLE_OPS_TOTAL].avg = arg->iops;
-cfg.buckets[THROTTLE_OPS_READ].avg  = arg->iops_rd;
-cfg.buckets[THROTTLE_OPS_WRITE].avg = arg->iops_wr;
-
-if (arg->has_bps_max) {
-cfg.buckets[THROTTLE_BPS_TOTAL].max = arg->bps_max;
-}
-if (arg->has_bps_rd_max) {
-cfg.buckets[THROTTLE_BPS_READ].max = arg->bps_rd_max;
-}
-if (arg->has_bps_wr_max) {
-cfg.buck

Re: [Qemu-devel] [PULL for-2.9 0/4] Block patches

[Peter Maydell]

On 22 March 2017 at 17:30, Jeff Cody  wrote:
> The following changes since commit 55a19ad8b2d0797e3a8fe90ab99a9bb713824059:
>
>   Update version for v2.9.0-rc1 release (2017-03-21 17:13:29 +)
>
> are available in the git repository at:
>
>   https://github.com/codyprime/qemu-kvm-jtc.git tags/block-pull-request
>
> for you to fetch changes up to 600ac6a0ef5c06418446ef2f37407bddcc51b21c:
>
>   blockjob: add devops to blockjob backends (2017-03-22 13:26:27 -0400)
>
> 
> Block patches for 2.9
> 
>
> John Snow (3):
>   blockjob: add block_job_start_shim
>   block-backend: add drained_begin / drained_end ops
>   blockjob: add devops to blockjob backends
>
> Paolo Bonzini (1):
>   blockjob: avoid recursive AioContext locking
>
>  block/block-backend.c  | 24 ++--
>  blockjob.c | 63 
> --
>  include/sysemu/block-backend.h |  8 ++
>  3 files changed, 79 insertions(+), 16 deletions(-)

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PATCH v2] qga: Add `guest-get-timezone` command

[sameeh jubran]

I have tested the patch on Windows 2012 R2 and it seems to be working good.

Please see some comments inline.


On 3/22/2017 7:16 PM, Vinzenz 'evilissimo' Feenstra wrote:

From: Vinzenz Feenstra 

Adds a new command `guest-get-timezone` reporting the currently
configured timezone on the system. The information on what timezone is
currently is configured is useful in case of Windows VMs where the
offset of the hardware clock is required to have the same offset. This
can be used for management systems like `oVirt` to detect the timezone
difference and warn administrators of the misconfiguration.

Signed-off-by: Vinzenz Feenstra 
---
  qga/commands.c   | 19 +++
  qga/qapi-schema.json | 26 ++
  2 files changed, 45 insertions(+)

diff --git a/qga/commands.c b/qga/commands.c
index 4d92946..83d7f99 100644
--- a/qga/commands.c
+++ b/qga/commands.c
@@ -499,3 +499,22 @@ int ga_parse_whence(GuestFileWhence *whence, Error **errp)
  error_setg(errp, "invalid whence code %"PRId64, whence->u.value);
  return -1;
  }
+
+GuestTimezone *qmp_guest_get_timezone(Error **errp)
+{
+GuestTimezone *info = g_new0(GuestTimezone, 1);

Check for null

+GTimeZone *tz = g_time_zone_new_local();

Check for null

+gint32 interval = g_time_zone_find_interval(tz, G_TIME_TYPE_STANDARD, 0);

Check if interval is -1

+gchar const *name = g_time_zone_get_abbreviation(tz, interval);
+if (name != NULL) {
+info->offset = g_time_zone_get_offset(tz, interval) / 60;
g_time_zone_get_offset returns the offset in seconds, dividing by 60 
will give us the offset in minutes.

Is there a reason to display it in minutes instead of hours?
Either way I think you should display the units in the output.

And 60 is a magic number, Even though it is very clear, I think it is 
better to add a define for that or check if glib already has one.

+info->zone = g_strdup(name);
+} else {
+error_setg(errp, "Timezone lookup failed");
+g_free(info);
+info = NULL;
+}
+g_time_zone_unref(tz);
+return info;
+}
+
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index a02dbf2..62d6909 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -1042,3 +1042,29 @@
'data':{ 'path': 'str', '*arg': ['str'], '*env': ['str'],
 '*input-data': 'str', '*capture-output': 'bool' },
'returns': 'GuestExec' }
+
+
+##
+# @GuestTimezone:
+#
+# @zone:Timezone name
+# @offset:  Offset to UTC in minutes. For western timezones the offset has a
+#   negative value and for eastern the offset is positive value
+#
+# Since: 2.10
+##
+{ 'struct': 'GuestTimezone',
+  'data':   { 'zone': 'str', 'offset': 'int' } }
+
+
+##
+# @guest-get-timezone:
+#
+# Retrieves the timezone information from the guest.
+#
+# Returns: A GuestTimezone dictionary.
+#
+# Since: 2.10
+##
+{ 'command': 'guest-get-timezone',
+  'returns': 'GuestTimezone' }

Re: [Qemu-devel] [PATCH RESEND 1/3] usb-ccid: make ccid_write_data_block() cope with null buffers

[Markus Armbruster]

Gerd Hoffmann  writes:

>   Hi,
>
>>  oops, there are hard-coded calls with NULL/0. I suppose to fix clang
>> warning, it would need to check if data != null for memcpy.
>
> I'd check for len > 0, and in that if branch we can also assert on data
> == NULL and thereby check that len and data are consistent.

If len is non-zero but data is null, memcpy() will crash just fine by
itself, so why bother asserting.  If len is zero, there's nothing to
assert.

I'd simply protect memcpy() with if (len) and call it a day.

[Qemu-devel] [PATCH for-2.9] disas/microblaze: Remove unused REG_PC define

[Peter Maydell]

The REG_PC define in disas/microblaze.c clashes with a define in
the Linux SPARC system headers:

/home/pm215/qemu/disas/microblaze.c:162:0: error: "REG_PC" redefined [-Werror]
 #define REG_PC  32 /* PC */

In file included from /usr/include/signal.h:326:0,
 from /home/pm215/qemu/include/qemu/osdep.h:86,
 from /home/pm215/qemu/disas/microblaze.c:36:
/usr/include/sparc64-linux-gnu/sys/ucontext.h:96:0: note: this is the location 
of the previous definition
 #define REG_PC  (1)

Since the code doesn't actually use the REG_PC define
anywhere, the simplest fix is just to remove it.

Signed-off-by: Peter Maydell 
---
 disas/microblaze.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/disas/microblaze.c b/disas/microblaze.c
index 407c0a3..7795a0b 100644
--- a/disas/microblaze.c
+++ b/disas/microblaze.c
@@ -159,7 +159,7 @@ enum microblaze_instr_type {
 #define MIN_PVR_REGNUM 0
 #define MAX_PVR_REGNUM 15
 
-#define REG_PC  32 /* PC */
+/* 32 is REG_PC */
 #define REG_MSR 33 /* machine status reg */
 #define REG_EAR 35 /* Exception reg */
 #define REG_ESR 37 /* Exception reg */
-- 
2.7.4

Re: [Qemu-devel] [PATCH v2] qga: Add `guest-get-timezone` command

[Marc-André Lureau]

Hi

On Thu, Mar 23, 2017 at 4:41 PM sameeh jubran  wrote:

> I have tested the patch on Windows 2012 R2 and it seems to be working good.
>
> Please see some comments inline.
>
>
> On 3/22/2017 7:16 PM, Vinzenz 'evilissimo' Feenstra wrote:
> > From: Vinzenz Feenstra 
> >
> > Adds a new command `guest-get-timezone` reporting the currently
> > configured timezone on the system. The information on what timezone is
> > currently is configured is useful in case of Windows VMs where the
> > offset of the hardware clock is required to have the same offset. This
> > can be used for management systems like `oVirt` to detect the timezone
> > difference and warn administrators of the misconfiguration.
> >
> > Signed-off-by: Vinzenz Feenstra 
> > ---
> >   qga/commands.c   | 19 +++
> >   qga/qapi-schema.json | 26 ++
> >   2 files changed, 45 insertions(+)
> >
> > diff --git a/qga/commands.c b/qga/commands.c
> > index 4d92946..83d7f99 100644
> > --- a/qga/commands.c
> > +++ b/qga/commands.c
> > @@ -499,3 +499,22 @@ int ga_parse_whence(GuestFileWhence *whence, Error
> **errp)
> >   error_setg(errp, "invalid whence code %"PRId64, whence->u.value);
> >   return -1;
> >   }
> > +
> > +GuestTimezone *qmp_guest_get_timezone(Error **errp)
> > +{
> > +GuestTimezone *info = g_new0(GuestTimezone, 1);
> Check for null
> > +GTimeZone *tz = g_time_zone_new_local();
> Check for null
> > +gint32 interval = g_time_zone_find_interval(tz,
> G_TIME_TYPE_STANDARD, 0);
> Check if interval is -1
> > +gchar const *name = g_time_zone_get_abbreviation(tz, interval);
> > +if (name != NULL) {
> > +info->offset = g_time_zone_get_offset(tz, interval) / 60;
> g_time_zone_get_offset returns the offset in seconds, dividing by 60
> will give us the offset in minutes.
> Is there a reason to display it in minutes instead of hours?
> Either way I think you should display the units in the output.
>
> And 60 is a magic number, Even though it is very clear, I think it is
> better to add a define for that or check if glib already has one.
>

Instead, why not follow glib API and return seconds?


> > +info->zone = g_strdup(name);
> > +} else {
> > +error_setg(errp, "Timezone lookup failed");
> > +g_free(info);
> > +info = NULL;
> > +}
> > +g_time_zone_unref(tz);
> > +return info;
> > +}
> > +
> > diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
> > index a02dbf2..62d6909 100644
> > --- a/qga/qapi-schema.json
> > +++ b/qga/qapi-schema.json
> > @@ -1042,3 +1042,29 @@
> > 'data':{ 'path': 'str', '*arg': ['str'], '*env': ['str'],
> >  '*input-data': 'str', '*capture-output': 'bool' },
> > 'returns': 'GuestExec' }
> > +
> > +
> > +##
> > +# @GuestTimezone:
> > +#
> > +# @zone:Timezone name
> > +# @offset:  Offset to UTC in minutes. For western timezones the offset
> has a
> > +#   negative value and for eastern the offset is positive value
> > +#
> > +# Since: 2.10
> > +##
> > +{ 'struct': 'GuestTimezone',
> > +  'data':   { 'zone': 'str', 'offset': 'int' } }
> > +
> > +
> > +##
> > +# @guest-get-timezone:
> > +#
> > +# Retrieves the timezone information from the guest.
> > +#
> > +# Returns: A GuestTimezone dictionary.
> > +#
> > +# Since: 2.10
> > +##
> > +{ 'command': 'guest-get-timezone',
> > +  'returns': 'GuestTimezone' }
>
> --
Marc-André Lureau

Re: [Qemu-devel] [PATCH v4 1/8] xen: import ring.h from xen

[Greg Kurz]

On Mon, 20 Mar 2017 11:19:05 -0700
Stefano Stabellini  wrote:

> Do not use the ring.h header installed on the system. Instead, import
> the header into the QEMU codebase. This avoids problems when QEMU is
> built against a Xen version too old to provide all the ring macros.
> 
> Signed-off-by: Stefano Stabellini 
> Reviewed-by: Greg Kurz 
> CC: anthony.per...@citrix.com
> CC: jgr...@suse.com
> ---
> NB: The new macros have not been committed to Xen yet. Do not apply this
> patch until they do.
> ---

Looking at your other series for the kernel part of this feature:

https://lkml.org/lkml/2017/3/22/761

I realize that the ring.h header from Xen also exists in the kernel tree... 

Shouldn't all the code that can be used in both kernel and userspace go to a
header file under include/uapi in the kernel tree ? And then we would import
it under include/standard-headers/linux in the QEMU tree and we could keep it
in sync using scripts/update-linux-headers.sh.

Cc'ing Paolo for insights.

> ---
>  hw/block/xen_blkif.h |   2 +-
>  hw/usb/xen-usb.c |   2 +-
>  include/hw/xen/io/ring.h | 455 
> +++
>  3 files changed, 457 insertions(+), 2 deletions(-)
>  create mode 100644 include/hw/xen/io/ring.h
> 
> diff --git a/hw/block/xen_blkif.h b/hw/block/xen_blkif.h
> index 3300b6f..3e6e1ea 100644
> --- a/hw/block/xen_blkif.h
> +++ b/hw/block/xen_blkif.h
> @@ -1,7 +1,7 @@
>  #ifndef XEN_BLKIF_H
>  #define XEN_BLKIF_H
>  
> -#include 
> +#include "hw/xen/io/ring.h"
>  #include 
>  #include 
>  
> diff --git a/hw/usb/xen-usb.c b/hw/usb/xen-usb.c
> index 8e676e6..370b3d9 100644
> --- a/hw/usb/xen-usb.c
> +++ b/hw/usb/xen-usb.c
> @@ -33,7 +33,7 @@
>  #include "qapi/qmp/qint.h"
>  #include "qapi/qmp/qstring.h"
>  
> -#include 
> +#include "hw/xen/io/ring.h"
>  #include 
>  
>  /*
> diff --git a/include/hw/xen/io/ring.h b/include/hw/xen/io/ring.h
> new file mode 100644
> index 000..cf01fc3
> --- /dev/null
> +++ b/include/hw/xen/io/ring.h
> @@ -0,0 +1,455 @@
> +/**
> + * ring.h
> + * 
> + * Shared producer-consumer ring macros.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a 
> copy
> + * of this software and associated documentation files (the "Software"), to
> + * deal in the Software without restriction, including without limitation the
> + * rights to use, copy, modify, merge, publish, distribute, sublicense, 
> and/or
> + * sell copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL 
> THE
> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS IN THE SOFTWARE.
> + *
> + * Tim Deegan and Andrew Warfield November 2004.
> + */
> +
> +#ifndef __XEN_PUBLIC_IO_RING_H__
> +#define __XEN_PUBLIC_IO_RING_H__
> +
> +#if __XEN_INTERFACE_VERSION__ < 0x00030208
> +#define xen_mb()  mb()
> +#define xen_rmb() rmb()
> +#define xen_wmb() wmb()
> +#endif
> +
> +typedef unsigned int RING_IDX;
> +
> +/* Round a 32-bit unsigned constant down to the nearest power of two. */
> +#define __RD2(_x)  (((_x) & 0x0002) ? 0x2  : ((_x) & 
> 0x1))
> +#define __RD4(_x)  (((_x) & 0x000c) ? __RD2((_x)>>2)<<2: __RD2(_x))
> +#define __RD8(_x)  (((_x) & 0x00f0) ? __RD4((_x)>>4)<<4: __RD4(_x))
> +#define __RD16(_x) (((_x) & 0xff00) ? __RD8((_x)>>8)<<8: __RD8(_x))
> +#define __RD32(_x) (((_x) & 0x) ? __RD16((_x)>>16)<<16 : __RD16(_x))
> +
> +/*
> + * Calculate size of a shared ring, given the total available space for the
> + * ring and indexes (_sz), and the name tag of the request/response 
> structure.
> + * A ring contains as many entries as will fit, rounded down to the nearest 
> + * power of two (so we can mask with (size-1) to loop around).
> + */
> +#define __CONST_RING_SIZE(_s, _sz) \
> +(__RD32(((_sz) - offsetof(struct _s##_sring, ring)) / \
> + sizeof(((struct _s##_sring *)0)->ring[0])))
> +/*
> + * The same for passing in an actual pointer instead of a name tag.
> + */
> +#define __RING_SIZE(_s, _sz) \
> +(__RD32(((_sz) - (long)(_s)->ring + (long)(_s)) / sizeof((_s)->ring[0])))
> +
> +/*
> + * Macros to make the correct C datatypes for a new kind of ring.
> + * 
> + * To make a new ring datatype, you need to have two message structures,
> + * let's say request_t, and respons

Re: [Qemu-devel] [PATCH for-2.10 15/23] QMP: include CpuInstanceProperties into query_cpus output output

[Eric Blake]

On 03/22/2017 08:32 AM, Igor Mammedov wrote:
> if board supports CpuInstanceProperties, report them for
> each CPU thread listed. Main motivation for this is to
> provide these properties introspection via QMP interface
> for using in test cases to verify numa node to cpu mapping,
> which includes not only boards that support cpu hotplug
> and have this info in query-hotpluggable-cpus (pc/spapr)
> but also for boards that don't not support hotpluggable-cpus
> but support numa mapping (virt-arm).
> 
> Signed-off-by: Igor Mammedov 
> ---

> @@ -1860,6 +1863,12 @@ CpuInfoList *qmp_query_cpus(Error **errp)
>  #else
>  info->value->arch = CPU_INFO_ARCH_OTHER;
>  #endif
> +if ((info->value->has_props = !!mc->cpu_index_to_instance_props)) {

checkpatch.pl doesn't flag that? We generally try to avoid side-effects
inside conditionals.

> +CpuInstanceProperties *props;
> +props = g_malloc0(sizeof(*props));
> +*props = mc->cpu_index_to_instance_props(ms, cpu->cpu_index);
> +info->value->props =  props;

Why two spaces after =?

With those cleaned up,
Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] slirp: tftp, copy sockaddr_size

[Samuel Thibault]

Marc-André Lureau, on jeu. 23 mars 2017 15:31:56 +0400, wrote:
> ASAN detects an "unknown-crash" when running pxe-test:
> 
> /ppc64/pxe/spapr-vlan: 
> =
> ==7143==ERROR: AddressSanitizer: unknown-crash on address 0x7f6dcd298d30 at 
> pc 0x55e22218830d bp 0x7f6dcd2989e0 sp 0x7f6dcd2989d0
> READ of size 128 at 0x7f6dcd298d30 thread T2
> #0 0x55e22218830c in tftp_session_allocate 
> /home/elmarco/src/qq/slirp/tftp.c:73
> #1 0x55e22218a1f8 in tftp_handle_rrq /home/elmarco/src/qq/slirp/tftp.c:289
> #2 0x55e22218b54c in tftp_input /home/elmarco/src/qq/slirp/tftp.c:446
> #3 0x55e2221833fe in udp6_input /home/elmarco/src/qq/slirp/udp6.c:82
> #4 0x55e222137b17 in ip6_input /home/elmarco/src/qq/slirp/ip6_input.c:67
> 
> Address 0x7f6dcd298d30 is located in stack of thread T2 at offset 96 in frame
> #0 0x55e222182420 in udp6_input /home/elmarco/src/qq/slirp/udp6.c:13
> 
>   This frame has 3 object(s):
> [32, 48) ''
> [96, 124) 'lhost' <== Memory access at offset 96 partially overflows this 
> variable
> [160, 200) 'save_ip' <== Memory access at offset 96 partially underflows 
> this variable
> 
> The sockaddr_storage pointer is the sockaddr_in6 lhost on the
> stack. Copy only the source addr size.
> 
> Signed-off-by: Marc-André Lureau 

Reviewed-by: Samuel Thibault 

> ---
>  slirp/tftp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/slirp/tftp.c b/slirp/tftp.c
> index 50e714807d..a9bc4bb1b6 100644
> --- a/slirp/tftp.c
> +++ b/slirp/tftp.c
> @@ -70,7 +70,7 @@ static int tftp_session_allocate(Slirp *slirp, struct 
> sockaddr_storage *srcsas,
>  
>   found:
>memset(spt, 0, sizeof(*spt));
> -  spt->client_addr = *srcsas;
> +  memcpy(&spt->client_addr, srcsas, sockaddr_size(srcsas));
>spt->fd = -1;
>spt->block_size = 512;
>spt->client_port = tp->udp.uh_sport;
> -- 
> 2.12.0.191.gc5d8de91d
> 

-- 
Samuel
gawk; talk; nice; date; wine; grep; touch; unzip; strip;  \
touch; gasp; finger; gasp; lyx; gasp; latex; mount; fsck; \
more; yes; gasp; umount; make clean; make mrproper; sleep

Re: [Qemu-devel] [PATCH for-2.10 22/23] numa: add '-numa cpu, ...' option for property based node mapping

[Eric Blake]

On 03/22/2017 08:32 AM, Igor Mammedov wrote:
> legacy cpu to node mapping is using cpu index values to map
> VCPU to node with help of '-numa node,nodeid=node,cpus=x[-y]'
> option. However cpu index is internal concept and QEMU users
> have to guess /reimplement qemu's logic/ to map it to
> a concrete cpu socket/core/thread to make sane CPUs
> placement across numa nodes.
> 
> This patch allows to map cpu objects to numa nodes using
> the same properties as used for cpus with -device/device_add
> (socket-id/core-id/thread-id/node-id).
> 
> At present valid properties/values to address CPUs could be
> fetched using hotpluggable-cpus monitor/qmp command, it will
> require user to start qemu twice when creating domain to fetch
> possible CPUs for a machine type/-smp layout first and
> then the second time with numa explicit mapping for actual
> usage. The first step results could be saved and reused to
> set/change mapping later as far as machine type/-smp stays
> the same.
> 
> Proposed impl. supports exact and wildcard matching to
> simplify CLI and allow to set mapping for a specific cpu
> or group of cpu objects specified by matched properties.
> 
> For example:
> 
># exact mapping x86
>-numa cpu,node-id=x,socket-id=y,core-id=z,thread-id=n
> 
># exact mapping SPAPR
>-numa cpu,node-id=x,core-id=y
> 
># wildcard mapping, all cpu objects that match socket-id=y
># are mapped to node-id=x
>-numa cpu,node-id=x,socket-id=y
> 
> Signed-off-by: Igor Mammedov 
> ---
>  numa.c   | 13 +
>  qapi-schema.json |  7 +--
>  qemu-options.hx  | 23 ++-
>  3 files changed, 40 insertions(+), 3 deletions(-)
> 

>  
> +@samp{cpu} option is new alternative to @samp{cpus} option

s/is/is a/

> +uses @samp{socket-id|core-id|thread-id} properties to assign

s/uses/which uses/

> +CPU objects to a @var{node} using topology layout properties of CPU.
> +Set of properties is machine specific, and depends on used machine

s/Set/The set/

> +type/@samp{smp} options. It could be queried with @samp{hotpluggable-cpus}
> +monitor command.
> +@samp{node-id} property specifies @var{node} to which CPU object
> +will be assigned, it's required for @var{node} to be declared
> +with @samp{node} option before it's used with @samp{cpu} option.
> +
> +For example:
> +@example
> +-M pc \
> +-smp 1,sockets=2,maxcpus=2 \
> +-numa node,nodeid=0 -numa node,nodeid=1 \
> +-numa cpu,node-id=0,socket-id=0 -numa cpu,node-id=1,socket-id=1
> +@end example
> +
>  @samp{mem} assigns a given RAM amount to a node. @samp{memdev}
>  assigns RAM from a given memory backend device to a node. If
>  @samp{mem} and @samp{memdev} are omitted in all nodes, RAM is
> 

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2] qga: Add `guest-get-timezone` command

[Vinzenz Feenstra]

> On Mar 23, 2017, at 1:51 PM, Marc-André Lureau  
> wrote:
> 
> Hi
> 
> On Thu, Mar 23, 2017 at 4:41 PM sameeh jubran  > wrote:
> I have tested the patch on Windows 2012 R2 and it seems to be working good.
> 
> Please see some comments inline.
> 
> 
> On 3/22/2017 7:16 PM, Vinzenz 'evilissimo' Feenstra wrote:
> > From: Vinzenz Feenstra mailto:vfeen...@redhat.com>>
> >
> > Adds a new command `guest-get-timezone` reporting the currently
> > configured timezone on the system. The information on what timezone is
> > currently is configured is useful in case of Windows VMs where the
> > offset of the hardware clock is required to have the same offset. This
> > can be used for management systems like `oVirt` to detect the timezone
> > difference and warn administrators of the misconfiguration.
> >
> > Signed-off-by: Vinzenz Feenstra  > >
> > ---
> >   qga/commands.c   | 19 +++
> >   qga/qapi-schema.json | 26 ++
> >   2 files changed, 45 insertions(+)
> >
> > diff --git a/qga/commands.c b/qga/commands.c
> > index 4d92946..83d7f99 100644
> > --- a/qga/commands.c
> > +++ b/qga/commands.c
> > @@ -499,3 +499,22 @@ int ga_parse_whence(GuestFileWhence *whence, Error 
> > **errp)
> >   error_setg(errp, "invalid whence code %"PRId64, whence->u.value);
> >   return -1;
> >   }
> > +
> > +GuestTimezone *qmp_guest_get_timezone(Error **errp)
> > +{
> > +GuestTimezone *info = g_new0(GuestTimezone, 1);
> Check for null
> > +GTimeZone *tz = g_time_zone_new_local();
> Check for null
> > +gint32 interval = g_time_zone_find_interval(tz, G_TIME_TYPE_STANDARD, 
> > 0);
> Check if interval is -1

Will add the checks above in v3

> > +gchar const *name = g_time_zone_get_abbreviation(tz, interval);
> > +if (name != NULL) {
> > +info->offset = g_time_zone_get_offset(tz, interval) / 60;
> g_time_zone_get_offset returns the offset in seconds, dividing by 60
> will give us the offset in minutes.
> Is there a reason to display it in minutes instead of hours?

Yes, there are offsets that are in minutes, some are 45 minutes, some 30 
minutes so hours
aren’t enough.

> Either way I think you should display the units in the output.
> 
> And 60 is a magic number, Even though it is very clear, I think it is
> better to add a define for that or check if glib already has one.
> 
> Instead, why not follow glib API and return seconds?

In an earlier iteration I did use a Windows API function and the offset value 
in Windows is returned
in minutes. See here the documentation for the  ‘Bias’ fields:
https://msdn.microsoft.com/en-us/library/windows/desktop/ms725481(v=vs.85).aspx 


That was for me the main motivation, plus the seconds granularity is not 
required for the offsets,
minutes is granular enough. However I could change it to seconds of course, 
seems more
reasonable in the light of the glib support for it.


>  
> > +info->zone = g_strdup(name);
> > +} else {
> > +error_setg(errp, "Timezone lookup failed");
> > +g_free(info);
> > +info = NULL;
> > +}
> > +g_time_zone_unref(tz);
> > +return info;
> > +}
> > +
> > diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
> > index a02dbf2..62d6909 100644
> > --- a/qga/qapi-schema.json
> > +++ b/qga/qapi-schema.json
> > @@ -1042,3 +1042,29 @@
> > 'data':{ 'path': 'str', '*arg': ['str'], '*env': ['str'],
> >  '*input-data': 'str', '*capture-output': 'bool' },
> > 'returns': 'GuestExec' }
> > +
> > +
> > +##
> > +# @GuestTimezone:
> > +#
> > +# @zone:Timezone name
> > +# @offset:  Offset to UTC in minutes. For western timezones the offset has 
> > a
> > +#   negative value and for eastern the offset is positive value
> > +#
> > +# Since: 2.10
> > +##
> > +{ 'struct': 'GuestTimezone',
> > +  'data':   { 'zone': 'str', 'offset': 'int' } }
> > +
> > +
> > +##
> > +# @guest-get-timezone:
> > +#
> > +# Retrieves the timezone information from the guest.
> > +#
> > +# Returns: A GuestTimezone dictionary.
> > +#
> > +# Since: 2.10
> > +##
> > +{ 'command': 'guest-get-timezone',
> > +  'returns': 'GuestTimezone' }
> 
> -- 
> Marc-André Lureau

[Qemu-devel] [RFC 1/1] qcow2: add ZSTD compression feature

[Denis V. Lunev]

ZSDT compression algorithm consumes 3-5 times less CPU power with a
comparable comression ratio with zlib. It would be wise to use it for
data compression f.e. for backups.

The patch adds incompatible ZSDT feature into QCOW2 header that indicates
that compressed clusters must be decoded using ZSTD.

Signed-off-by: Denis V. Lunev 
CC: Kevin Wolf 
CC: Max Reitz 
CC: Stefan Hajnoczi 
CC: Fam Zheng 
---
Actually this is very straightforward. May be we should implement 2 stage
scheme, i.e. add bit that indicates presence of the "compression
extension", which will actually define the compression algorithm. Though
at my opinion we will not have too many compression algorithms and proposed
one tier scheme is good enough.

 docs/specs/qcow2.txt | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt
index 80cdfd0..eb5c41b 100644
--- a/docs/specs/qcow2.txt
+++ b/docs/specs/qcow2.txt
@@ -85,7 +85,10 @@ in the description of a field.
 be written to (unless for regaining
 consistency).
 
-Bits 2-63:  Reserved (set to 0)
+Bits 2: ZSDT compression bit. ZSDT algorithm is used
+for cluster compression/decompression.
+
+Bits 3-63:  Reserved (set to 0)
 
  80 -  87:  compatible_features
 Bitmask of compatible features. An implementation can
-- 
2.7.4

Re: [Qemu-devel] [PULL v3 for-2.9 00/17] QAPI patches for 2017-03-22

[Peter Maydell]

On 22 March 2017 at 18:36, Markus Armbruster  wrote:
> v3 (third time's a charm)
> * Actually squash it into 16/17, not 17/17
> v2:
> * Leak fix squashed into 16/17
>
> The following changes since commit 940a8ce075e3408742a4edcabfd6c2a15e2539eb:
>
>   Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into 
> staging (2017-03-20 16:34:26 +)
>
> are available in the git repository at:
>
>   git://repo.or.cz/qemu/armbru.git tags/pull-qapi-2017-03-22-v3
>
> for you to fetch changes up to 21f88d021d0d2b4ecee8f6cd6ca63a943a3ce71d:
>
>   qapi: Fix QemuOpts visitor regression on unvisited input (2017-03-22 
> 19:24:34 +0100)
>
> 
> QAPI patches for 2017-03-22
>
> 

Applied, thanks.

-- PMM

Re: [Qemu-devel] (no subject)

[Eric Blake]

On 03/16/2017 09:50 AM, Vinzenz 'evilissimo' Feenstra wrote:
> In this version:

When sending a v2, it's best to send it as a new top-level thread
instead of burying it in-reply-to an older thread. Also, don't forget
the subject line on the header message.

> 
> - Changed the use of strdup to g_strdup and the use of sprintf with a local
>   buffer to use g_strdup_printf instead.
> - Made the majority of fields in the GuestOSInfo optional to allow 0 values
> - Used the right target version in the schema (2.10 vs 2.8 before)
> - Refactored the code for deciding which release/version file to use to use a
>   configuration struct and a while loop to iterate over the options.
> 
> I was looking into the usage of uname, as suggested by eric, however after
> looking into this I realized that there's no additional information to be
> gained from this. Therefore I decided that this is still a feasible approach.
> In most cases the code will break out of the loop after accessing the second
> file. For older systems there are some supported fallbacks available, but
> /etc/os-release and /usr/lib/os-release are already quite established.
> 
> 

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v3] Allow setting NUMA distance for different NUMA nodes

[Eric Blake]

On 03/23/2017 02:23 AM, He Chen wrote:

>>> -numa dist,src=1,dst=0,val=21 \

>>
>> And maybe should also point out how to set an unreachable node with 255.
>>
> Thanks for your careful review. Regarding setting unreachable node,
> which way is good? Set exact 255 as distance or distance that is larger
> than 255 is regarded as unreachable?

Given that you spec'd val as 'uint8', you can't pass a value for
distance that is larger than 255.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v4 1/8] xen: import ring.h from xen

[Juergen Gross]

On 23/03/17 14:00, Greg Kurz wrote:
> On Mon, 20 Mar 2017 11:19:05 -0700
> Stefano Stabellini  wrote:
> 
>> Do not use the ring.h header installed on the system. Instead, import
>> the header into the QEMU codebase. This avoids problems when QEMU is
>> built against a Xen version too old to provide all the ring macros.
>>
>> Signed-off-by: Stefano Stabellini 
>> Reviewed-by: Greg Kurz 
>> CC: anthony.per...@citrix.com
>> CC: jgr...@suse.com
>> ---
>> NB: The new macros have not been committed to Xen yet. Do not apply this
>> patch until they do.
>> ---
> 
> Looking at your other series for the kernel part of this feature:
> 
> https://lkml.org/lkml/2017/3/22/761
> 
> I realize that the ring.h header from Xen also exists in the kernel tree... 
> 
> Shouldn't all the code that can be used in both kernel and userspace go to a
> header file under include/uapi in the kernel tree ? And then we would import
> it under include/standard-headers/linux in the QEMU tree and we could keep it
> in sync using scripts/update-linux-headers.sh.
> 
> Cc'ing Paolo for insights.

As Xen isn't part of the kernel we don't want that. You can use and/or
build qemu with xen-9pfs backend support on an old Linux kernel without
the related frontend.

OTOH I don't see the advantage of not using the headers from Xen. This
is working for qdisk and pvusb backends and for all the Xen libraries.
Do you expect the 9pfs backend to be used for a qemu version built
against a Xen version not supporting that backend?


Juergen

Re: [Qemu-devel] [PATCH RESEND 1/3] usb-ccid: make ccid_write_data_block() cope with null buffers

[Gerd Hoffmann]

On Do, 2017-03-23 at 13:41 +0100, Markus Armbruster wrote:
> Gerd Hoffmann  writes:
> 
> >   Hi,
> >
> >>  oops, there are hard-coded calls with NULL/0. I suppose to fix clang
> >> warning, it would need to check if data != null for memcpy.
> >
> > I'd check for len > 0, and in that if branch we can also assert on data
> > == NULL and thereby check that len and data are consistent.
> 
> If len is non-zero but data is null, memcpy() will crash just fine by
> itself, so why bother asserting.

To make clang happy?  But maybe clang is clever enough to figure data
can't be null at that point in case we call memcpy with len != 0
only ...

cheers,
  Gerd

Re: [Qemu-devel] [PATCH] slirp: tftp, copy sockaddr_size

[Thomas Huth]

On 23.03.2017 12:31, Marc-André Lureau wrote:
> ASAN detects an "unknown-crash" when running pxe-test:
> 
> /ppc64/pxe/spapr-vlan: 
> =
> ==7143==ERROR: AddressSanitizer: unknown-crash on address 0x7f6dcd298d30 at 
> pc 0x55e22218830d bp 0x7f6dcd2989e0 sp 0x7f6dcd2989d0
> READ of size 128 at 0x7f6dcd298d30 thread T2
> #0 0x55e22218830c in tftp_session_allocate 
> /home/elmarco/src/qq/slirp/tftp.c:73
> #1 0x55e22218a1f8 in tftp_handle_rrq /home/elmarco/src/qq/slirp/tftp.c:289
> #2 0x55e22218b54c in tftp_input /home/elmarco/src/qq/slirp/tftp.c:446
> #3 0x55e2221833fe in udp6_input /home/elmarco/src/qq/slirp/udp6.c:82
> #4 0x55e222137b17 in ip6_input /home/elmarco/src/qq/slirp/ip6_input.c:67
> 
> Address 0x7f6dcd298d30 is located in stack of thread T2 at offset 96 in frame
> #0 0x55e222182420 in udp6_input /home/elmarco/src/qq/slirp/udp6.c:13
> 
>   This frame has 3 object(s):
> [32, 48) ''
> [96, 124) 'lhost' <== Memory access at offset 96 partially overflows this 
> variable
> [160, 200) 'save_ip' <== Memory access at offset 96 partially underflows 
> this variable
> 
> The sockaddr_storage pointer is the sockaddr_in6 lhost on the
> stack. Copy only the source addr size.
> 
> Signed-off-by: Marc-André Lureau 
> ---
>  slirp/tftp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/slirp/tftp.c b/slirp/tftp.c
> index 50e714807d..a9bc4bb1b6 100644
> --- a/slirp/tftp.c
> +++ b/slirp/tftp.c
> @@ -70,7 +70,7 @@ static int tftp_session_allocate(Slirp *slirp, struct 
> sockaddr_storage *srcsas,
>  
>   found:
>memset(spt, 0, sizeof(*spt));
> -  spt->client_addr = *srcsas;
> +  memcpy(&spt->client_addr, srcsas, sockaddr_size(srcsas));
>spt->fd = -1;
>spt->block_size = 512;
>spt->client_port = tp->udp.uh_sport;
> 

Makes sense.

Reviewed-by: Thomas Huth 

Re: [Qemu-devel] [PATCH RESEND 1/3] usb-ccid: make ccid_write_data_block() cope with null buffers

[Markus Armbruster]

Gerd Hoffmann  writes:

> On Do, 2017-03-23 at 13:41 +0100, Markus Armbruster wrote:
>> Gerd Hoffmann  writes:
>> 
>> >   Hi,
>> >
>> >>  oops, there are hard-coded calls with NULL/0. I suppose to fix clang
>> >> warning, it would need to check if data != null for memcpy.
>> >
>> > I'd check for len > 0, and in that if branch we can also assert on data
>> > == NULL and thereby check that len and data are consistent.
>> 
>> If len is non-zero but data is null, memcpy() will crash just fine by
>> itself, so why bother asserting.
>
> To make clang happy?  But maybe clang is clever enough to figure data
> can't be null at that point in case we call memcpy with len != 0
> only ...

If Clang needs another hint to become happy, then an assertion is a fine
way to provide it.

[Qemu-devel] [PATCH v3] qga: Add `guest-get-timezone` command

[Vinzenz 'evilissimo' Feenstra]

From: Vinzenz Feenstra 

Changes since v2:

- Changed offset from reporting minutes to reporting seconds like it is
  returned from the used glib API
- Applied more checks and better error handling in the implementation

Vinzenz Feenstra (1):
  qga: Add `guest-get-timezone` command

 qga/commands.c   | 42 ++
 qga/qapi-schema.json | 26 ++
 2 files changed, 68 insertions(+)

-- 
2.9.3