Re: [Qemu-devel] [PATCH] Use siginfo_t instead of struct siginfo.

[Andreas Jaeger]

On Monday, July 30, 2012 22:38:32 Peter Maydell wrote:
> On 30 July 2012 22:33, Andreas Färber  wrote:
> > Am 30.07.2012 23:30, schrieb Alexander Graf:
> >> On 30.07.2012, at 09:21, Andreas Jaeger wrote:
> >>> glibc 2.16 does not export the undocumented struct siginfo
> >>> anymore.
> >>> qemu uses already in most cases siginfo_t, this patch fixes the
> >>> last
> >>> three occurences.
> > 
> > Wasn't there already another patch doing the same thing a few weeks
> > ago?
> Yes: http://patchwork.ozlabs.org/patch/169170/
> 
> (It's also more complete than this patch, which misses one
> occurrence in user-exec.c and one comment in linux-user/signal.c.)

Yes, let's apply that patch instead of mine,

Andreas
-- 
 Andreas Jaeger aj@{suse.com,opensuse.org} Twitter/Identica: jaegerandi
  SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
   GF: Jeff Hawn,Jennifer Guild,Felix Imendörffer,HRB16746 (AG Nürnberg)
GPG fingerprint = 93A3 365E CE47 B889 DF7F  FED1 389A 563C C272 A126

Re: [Qemu-devel] [PATCH] ATAPI: Add support for ASCQ in sense codes

[Paolo Bonzini]

Il 31/07/2012 04:07, Ronnie Sahlberg ha scritto:
> Add support for setting the ASCQ for SCSI sense codes in the ATAPI driver.
> Use this to set ASCQ==2 for the medium removal prevention that is recommended 
> in MMC for this condition.
> 
> asc:0x53 ascq:0x02 is the recommended error for MEDIUM_REMOVAL_PREVENTED and 
> is listed in Annex F in MMC

You also need to cover migration.

You could either add a subsection, or something like this:

diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index f7f714c..89c0157 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -1143,3 +1143,20 @@ void ide_atapi_cmd(IDEState *s)
 
 ide_atapi_cmd_error(s, ILLEGAL_REQUEST, ASC_ILLEGAL_OPCODE);
 }
+
+void ide_atapi_post_load(IDEState *s, int version_id)
+{
+if (version_id < 3) {
+if (s->sense_key == UNIT_ATTENTION &&
+s->asc == ASC_MEDIUM_MAY_HAVE_CHANGED) {
+s->cdrom_changed = 1;
+}
+}
+
+/* This is simpler than adding a subsection just for the ascq.  */
+if (s->asc == ASC_MEDIA_REMOVAL_PREVENTED) {
+s->ascq = 2;
+} else {
+s->ascq = 0;
+}
+}
diff --git a/hw/ide/core.c b/hw/ide/core.c
index cb5ca4b..959ac48 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2154,12 +2154,7 @@ static int ide_drive_post_load(void *opaque, int 
version_id)
 {
 IDEState *s = opaque;
 
-if (version_id < 3) {
-if (s->sense_key == UNIT_ATTENTION &&
-s->asc == ASC_MEDIUM_MAY_HAVE_CHANGED) {
-s->cdrom_changed = 1;
-}
-}
+ide_atapi_post_load(s, version_id);
 if (s->identify_set) {
 bdrv_set_enable_write_cache(s->bs, !!(s->identify_data[85] & (1 << 
5)));
 }
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 7170bd9..2572461 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -572,6 +572,7 @@ BlockDriverAIOCB *ide_issue_trim(BlockDriverState *bs,
 /* hw/ide/atapi.c */
 void ide_atapi_cmd(IDEState *s);
 void ide_atapi_cmd_reply_end(IDEState *s);
+void ide_atapi_post_load(IDEState *s, int version_id);
 
 /* hw/ide/qdev.c */
 void ide_bus_new(IDEBus *idebus, DeviceState *dev, int bus_id);


In fact, I wonder if it is simpler to "make up" the ascq directly in
cmd_request_sense, instead of changing all invocations of
ide_atapi_cmd_error.  Kevin, any preferences?

Paolo

Re: [Qemu-devel] 9p broken?

[Aneesh Kumar K.V]

Avi Kivity  writes:

> Having an annoying bug on i386 kvm I decided to debug it buy running an
> i386 guest on my x86_64 host, use 9p to access a guest image, and run it
> using nested kvm.
>
> However, 9p appears to be broken: first, the configure test fails (patch
> sent).  Second, while mount works, ls on the mount point causes qemu to
> crash with
>
> (gdb) bt
> #0  error_set (errp=0x7fffe95fb128, fmt=0x558d4568 "{ 'class':
> 'VirtFSFeatureBlocksMigration', 'data': { 'path': %s, 'tag': %s } }") at
> /home/tlv/akivity/qemu/error.c:32
> #1  0x5567cb06 in v9fs_attach (opaque=0x7fffe95e3020) at
> /home/tlv/akivity/qemu/hw/9pfs/virtio-9p.c:988
> #2  0x5561d19f in coroutine_trampoline (i0=1449767888, i1=21845)
> at /home/tlv/akivity/qemu/coroutine-ucontext.c:138
> #3  0x75a93ef0 in ?? ()   from /lib64/libc.so.6
> #4  0x7fffce00 in ?? ()
> #5  0x in ?? (
>
> **errp already points to a VirtFSFeatureBlocksMigration error;
> v9fs_attach() has been called a second time (the first time,
> understandably, on mount; the second on ls).
>

Ok we mount with fid uid = (unsigned)~0. So we need to attach again when
we do ls as root. I guess we can do that migration block in attach, or
we have to do it conditionally. We also can hit the same if we do a
path lookup as a different user.

-aneesh

[Qemu-devel] buildbot failure in qemu on xen40

[qemu]

The Buildbot has detected a new failure on builder xen40 while building qemu.
Full details are available at:
 http://buildbot.b1-systems.de/qemu/builders/xen40/builds/80

Buildbot URL: http://buildbot.b1-systems.de/qemu/

Buildslave for this Build: anthony_xen

Build Reason: The Nightly scheduler named 'nightly_xen40' triggered this build
Build Source Stamp: [branch master] HEAD
Blamelist: 

BUILD FAILED: failed compile

sincerely,
 -The Buildbot

[Qemu-devel] [PATCH] hw/9pfs: Fix assert when disabling migration

[Aneesh Kumar K.V]

From: "Aneesh Kumar K.V" 

For 9p we can get the attach request multiple times for the
same export. So don't adding migration blocker for every
attach request.

Signed-off-by: Aneesh Kumar K.V 
---
 hw/9pfs/virtio-9p.c |   15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index f4a7026..4b52540 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -983,11 +983,16 @@ static void v9fs_attach(void *opaque)
 err += offset;
 trace_v9fs_attach_return(pdu->tag, pdu->id,
  qid.type, qid.version, qid.path);
-s->root_fid = fid;
-/* disable migration */
-error_set(&s->migration_blocker, QERR_VIRTFS_FEATURE_BLOCKS_MIGRATION,
-  s->ctx.fs_root ? s->ctx.fs_root : "NULL", s->tag);
-migrate_add_blocker(s->migration_blocker);
+/*
+ * disable migration if we haven't done already.
+ * attach could get called multiple times for the same export.
+ */
+if (!s->migration_blocker) {
+s->root_fid = fid;
+error_set(&s->migration_blocker, QERR_VIRTFS_FEATURE_BLOCKS_MIGRATION,
+  s->ctx.fs_root ? s->ctx.fs_root : "NULL", s->tag);
+migrate_add_blocker(s->migration_blocker);
+}
 out:
 put_fid(pdu, fidp);
 out_nofid:
-- 
1.7.10

Re: [Qemu-devel] [PATCH 3/5] qapi: avoid reserved word restrict

[Paolo Bonzini]

Il 30/07/2012 18:04, blauwir...@gmail.com ha scritto:
> From: Blue Swirl 
> 
> Clang compiler complained about use of reserved word 'restrict' in SLIRP
> and QAPI.
> 
> Rename 'restrict' to 'restricted' which also matches other SLIRP code.

Can't do it, this changes the command-line option.

Luiz, Michael, any ideas?

Paolo

> Signed-off-by: Blue Swirl 
> ---
>  net/slirp.c  |6 +++---
>  qapi-schema.json |4 ++--
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/net/slirp.c b/net/slirp.c
> index 5c2e6b2..8c42b53 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -722,9 +722,9 @@ int net_init_slirp(const NetClientOptions *opts, const 
> char *name,
>  net_init_slirp_configs(user->hostfwd, SLIRP_CFG_HOSTFWD);
>  net_init_slirp_configs(user->guestfwd, 0);
>  
> -ret = net_slirp_init(vlan, "user", name, user->restrict, vnet, 
> user->host,
> - user->hostname, user->tftp, user->bootfile,
> - user->dhcpstart, user->dns, user->smb,
> +ret = net_slirp_init(vlan, "user", name, user->restricted, vnet,
> + user->host, user->hostname, user->tftp,
> + user->bootfile, user->dhcpstart, user->dns, 
> user->smb,
>   user->smbserver);
>  
>  while (slirp_configs) {
> diff --git a/qapi-schema.json b/qapi-schema.json
> index bc55ed2..3912430 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1925,7 +1925,7 @@
>  #
>  # @hostname: #optional client hostname reported by the builtin DHCP server
>  #
> -# @restrict: #optional isolate the guest from the host
> +# @restricted: #optional isolate the guest from the host
>  #
>  # @ip: #optional legacy parameter, use net= instead
>  #
> @@ -1956,7 +1956,7 @@
>  { 'type': 'NetdevUserOptions',
>'data': {
>  '*hostname':  'str',
> -'*restrict':  'bool',
> +'*restricted':'bool',
>  '*ip':'str',
>  '*net':   'str',
>  '*host':  'str',
> 

Re: [Qemu-devel] [PATCH 2/5] pseries: Instantiate USB if requested

[Benjamin Herrenschmidt]

On Tue, 2012-07-31 at 16:09 +1000, David Gibson wrote:
> The pseries machine currently ignores the -usb command line option.
> This patch corrects the problem by having it instantiate a PCI OHCI
> USB host controller when -usb is specified.
> 
> Signed-off-by: David Gibson 

Same comment as the -vga patch, Li's been dealing with that lately,
Li, can you update us on the status of those patches ?

Cheers,
Ben.

> ---
>  hw/spapr.c |7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/hw/spapr.c b/hw/spapr.c
> index ab5a0c2..740881b 100644
> --- a/hw/spapr.c
> +++ b/hw/spapr.c
> @@ -45,6 +45,7 @@
>  #include "kvm.h"
>  #include "kvm_ppc.h"
>  #include "pci.h"
> +#include "usb.h"
>  
>  #include "exec-memory.h"
>  
> @@ -710,6 +711,12 @@ static void ppc_spapr_init(ram_addr_t ram_size,
>  spapr_vscsi_create(spapr->vio_bus);
>  }
>  
> +/* USB */
> +if (usb_enabled) {
> +pci_create_simple(QLIST_FIRST(&spapr->phbs)->host_state.bus,
> +  -1, "pci-ohci");
> +}
> +
>  if (rma_size < (MIN_RMA_SLOF << 20)) {
>  fprintf(stderr, "qemu: pSeries SLOF firmware requires >= "
>  "%ldM guest RMA (Real Mode Area memory)\n", MIN_RMA_SLOF);

Re: [Qemu-devel] [PATCH 02/11] Add migrate_set_parameter and query-migrate-parameters

[Orit Wasserman]

On 07/30/2012 08:41 PM, Luiz Capitulino wrote:
> On Sun, 29 Jul 2012 12:42:54 +0300
> Orit Wasserman  wrote:
> 
>> The management can enable/disable a capability for the next migration by 
>> using
>> migrate_set_parameter command.
>> The management can query the current migration capabilities using
>> query-migrate-parameters
> 
> In general looks good to me, I have a question and a few nitpick comments
> below.
> 
>>
>> Signed-off-by: Orit Wasserman 
>> Signed-off-by: Juan Quintela 
>> ---
>>  hmp-commands.hx  |   16 
>>  hmp.c|   71 
>> ++
>>  hmp.h|2 +
>>  migration.c  |   42 +++-
>>  migration.h  |2 +
>>  monitor.c|7 +
>>  qapi-schema.json |   32 
>>  qmp-commands.hx  |   60 -
>>  8 files changed, 229 insertions(+), 3 deletions(-)
>>
>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>> index 8786148..3e15338 100644
>> --- a/hmp-commands.hx
>> +++ b/hmp-commands.hx
>> @@ -861,6 +861,20 @@ Set maximum tolerated downtime (in seconds) for 
>> migration.
>>  ETEXI
>>  
>>  {
>> +.name   = "migrate_set_parameter",
>> +.args_type  = "capability:s,state:b",
>> +.params = "capability state",
>> +.help   = "Enable/Disable the usage of a capability for 
>> migration",
>> +.mhandler.cmd = hmp_migrate_set_parameter,
>> +},
>> +
>> +STEXI
>> +@item migrate_set_parameter @var{capability} @var{state}
>> +@findex migrate_set_parameter
>> +Enable/Disable the usage of a capability @var{capability} for migration.
>> +ETEXI
>> +
>> +{
>>  .name   = "client_migrate_info",
>>  .args_type  = 
>> "protocol:s,hostname:s,port:i?,tls-port:i?,cert-subject:s?",
>>  .params = "protocol hostname port tls-port cert-subject",
>> @@ -1419,6 +1433,8 @@ show user network stack connection states
>>  show migration status
>>  @item info migration_capabilities
>>  show migration capabilities
>> +@item info migrate_parameters
>> +show current migration parameters
>>  @item info balloon
>>  show balloon information
>>  @item info qtree
>> diff --git a/hmp.c b/hmp.c
>> index 5c7d0be..f2f63fd 100644
>> --- a/hmp.c
>> +++ b/hmp.c
>> @@ -131,8 +131,22 @@ void hmp_info_mice(Monitor *mon)
>>  void hmp_info_migrate(Monitor *mon)
>>  {
>>  MigrationInfo *info;
>> +MigrationCapabilityStatusList *cap;
>> +MigrationParameters *params;
>>  
>>  info = qmp_query_migrate(NULL);
>> +params = qmp_query_migrate_parameters(NULL);
>> +
>> +/* do not display parameters during setup */
>> +if (info->has_status && params->capabilities) {
>> +monitor_printf(mon, "capabilities: ");
>> +for (cap = params->capabilities; cap; cap = cap->next) {
>> +monitor_printf(mon, "%s: %s ",
>> +   
>> MigrationCapability_lookup[cap->value->capability],
>> +   cap->value->state ? "on" : "off");
>> +}
>> +monitor_printf(mon, "\n");
>> +}
>>  
>>  if (info->has_status) {
>>  monitor_printf(mon, "Migration status: %s\n", info->status);
>> @@ -159,6 +173,7 @@ void hmp_info_migrate(Monitor *mon)
>>  }
>>  
>>  qapi_free_MigrationInfo(info);
>> +qapi_free_MigrationParameters(params);
>>  }
>>  
>>  void hmp_info_migration_capabilities(Monitor *mon)
>> @@ -180,6 +195,27 @@ void hmp_info_migration_capabilities(Monitor *mon)
>>  qapi_free_MigrationCapabilityStatusList(caps_list);
>>  }
>>  
>> +void hmp_info_migrate_parameters(Monitor *mon)
>> +{
>> +
>> +MigrationCapabilityStatusList *cap;
>> +MigrationParameters *params;
>> +
>> +params = qmp_query_migrate_parameters(NULL);
>> +
>> +if (params->capabilities) {
>> +monitor_printf(mon, "capabilities: ");
>> +for (cap = params->capabilities; cap; cap = cap->next) {
>> +monitor_printf(mon, "%s: %s ",
>> +   
>> MigrationCapability_lookup[cap->value->capability],
>> +   cap->value->state ? "on" : "off");
>> +}
>> +monitor_printf(mon, "\n");
>> +}
>> +
>> +qapi_free_MigrationParameters(params);
>> +}
>> +
>>  void hmp_info_cpus(Monitor *mon)
>>  {
>>  CpuInfoList *cpu_list, *cpu;
>> @@ -754,6 +790,41 @@ void hmp_migrate_set_speed(Monitor *mon, const QDict 
>> *qdict)
>>  qmp_migrate_set_speed(value, NULL);
>>  }
>>  
>> +void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
>> +{
>> +const char *cap = qdict_get_str(qdict, "capability");
>> +bool state = qdict_get_bool(qdict, "state");
>> +Error *err = NULL;
>> +MigrationCapabilityStatusList *params = NULL;
>> +int i;
>> +
>> +for (i = 0; i < MIGRATION_CAPABILITY_MAX; i++) {
>> +if (strcmp(cap, MigrationCapability_lookup[i]) == 0) {
>> +if (!params) {
>> + 

Re: [Qemu-devel] [PATCH 02/11] Add migrate_set_parameter and query-migrate-parameters

[Orit Wasserman]

On 07/30/2012 08:41 PM, Luiz Capitulino wrote:
> On Sun, 29 Jul 2012 12:42:54 +0300
> Orit Wasserman  wrote:
> 
>> The management can enable/disable a capability for the next migration by 
>> using
>> migrate_set_parameter command.
>> The management can query the current migration capabilities using
>> query-migrate-parameters
> 
> In general looks good to me, I have a question and a few nitpick comments
> below.
> 
>>
>> Signed-off-by: Orit Wasserman 
>> Signed-off-by: Juan Quintela 
>> ---
>>  hmp-commands.hx  |   16 
>>  hmp.c|   71 
>> ++
>>  hmp.h|2 +
>>  migration.c  |   42 +++-
>>  migration.h  |2 +
>>  monitor.c|7 +
>>  qapi-schema.json |   32 
>>  qmp-commands.hx  |   60 -
>>  8 files changed, 229 insertions(+), 3 deletions(-)
>>
>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>> index 8786148..3e15338 100644
>> --- a/hmp-commands.hx
>> +++ b/hmp-commands.hx
>> @@ -861,6 +861,20 @@ Set maximum tolerated downtime (in seconds) for 
>> migration.
>>  ETEXI
>>  
>>  {
>> +.name   = "migrate_set_parameter",
>> +.args_type  = "capability:s,state:b",
>> +.params = "capability state",
>> +.help   = "Enable/Disable the usage of a capability for 
>> migration",
>> +.mhandler.cmd = hmp_migrate_set_parameter,
>> +},
>> +
>> +STEXI
>> +@item migrate_set_parameter @var{capability} @var{state}
>> +@findex migrate_set_parameter
>> +Enable/Disable the usage of a capability @var{capability} for migration.
>> +ETEXI
>> +
>> +{
>>  .name   = "client_migrate_info",
>>  .args_type  = 
>> "protocol:s,hostname:s,port:i?,tls-port:i?,cert-subject:s?",
>>  .params = "protocol hostname port tls-port cert-subject",
>> @@ -1419,6 +1433,8 @@ show user network stack connection states
>>  show migration status
>>  @item info migration_capabilities
>>  show migration capabilities
>> +@item info migrate_parameters
>> +show current migration parameters
>>  @item info balloon
>>  show balloon information
>>  @item info qtree
>> diff --git a/hmp.c b/hmp.c
>> index 5c7d0be..f2f63fd 100644
>> --- a/hmp.c
>> +++ b/hmp.c
>> @@ -131,8 +131,22 @@ void hmp_info_mice(Monitor *mon)
>>  void hmp_info_migrate(Monitor *mon)
>>  {
>>  MigrationInfo *info;
>> +MigrationCapabilityStatusList *cap;
>> +MigrationParameters *params;
>>  
>>  info = qmp_query_migrate(NULL);
>> +params = qmp_query_migrate_parameters(NULL);
>> +
>> +/* do not display parameters during setup */
>> +if (info->has_status && params->capabilities) {
>> +monitor_printf(mon, "capabilities: ");
>> +for (cap = params->capabilities; cap; cap = cap->next) {
>> +monitor_printf(mon, "%s: %s ",
>> +   
>> MigrationCapability_lookup[cap->value->capability],
>> +   cap->value->state ? "on" : "off");
>> +}
>> +monitor_printf(mon, "\n");
>> +}
>>  
>>  if (info->has_status) {
>>  monitor_printf(mon, "Migration status: %s\n", info->status);
>> @@ -159,6 +173,7 @@ void hmp_info_migrate(Monitor *mon)
>>  }
>>  
>>  qapi_free_MigrationInfo(info);
>> +qapi_free_MigrationParameters(params);
>>  }
>>  
>>  void hmp_info_migration_capabilities(Monitor *mon)
>> @@ -180,6 +195,27 @@ void hmp_info_migration_capabilities(Monitor *mon)
>>  qapi_free_MigrationCapabilityStatusList(caps_list);
>>  }
>>  
>> +void hmp_info_migrate_parameters(Monitor *mon)
>> +{
>> +
>> +MigrationCapabilityStatusList *cap;
>> +MigrationParameters *params;
>> +
>> +params = qmp_query_migrate_parameters(NULL);
>> +
>> +if (params->capabilities) {
>> +monitor_printf(mon, "capabilities: ");
>> +for (cap = params->capabilities; cap; cap = cap->next) {
>> +monitor_printf(mon, "%s: %s ",
>> +   
>> MigrationCapability_lookup[cap->value->capability],
>> +   cap->value->state ? "on" : "off");
>> +}
>> +monitor_printf(mon, "\n");
>> +}
>> +
>> +qapi_free_MigrationParameters(params);
>> +}
>> +
>>  void hmp_info_cpus(Monitor *mon)
>>  {
>>  CpuInfoList *cpu_list, *cpu;
>> @@ -754,6 +790,41 @@ void hmp_migrate_set_speed(Monitor *mon, const QDict 
>> *qdict)
>>  qmp_migrate_set_speed(value, NULL);
>>  }
>>  
>> +void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
>> +{
>> +const char *cap = qdict_get_str(qdict, "capability");
>> +bool state = qdict_get_bool(qdict, "state");
>> +Error *err = NULL;
>> +MigrationCapabilityStatusList *params = NULL;
>> +int i;
>> +
>> +for (i = 0; i < MIGRATION_CAPABILITY_MAX; i++) {
>> +if (strcmp(cap, MigrationCapability_lookup[i]) == 0) {
>> +if (!params) {
>> + 

Re: [Qemu-devel] [GIT PULL (PATCH 0/4)] VFIO driver for v3.6

[Linus Torvalds]

On Mon, Jul 30, 2012 at 4:17 PM, Alex Williamson
 wrote:
>
> I'm pretty anxious to find out as well.  Linus, ping, any thoughts on
> including this in 3.6?  Thanks,

I just pulled it, but then I unpulled again when I realized it's not a
signed tag and it's on github.

Please, people. Do tagged releases with proper signatures if you're
not using kernel.org or other controlled servers. In fact, I prefer
signed tags even if you *do* use kernel.org etc.

 Linus

Re: [Qemu-devel] Cirrus bugs vs endian: how two bugs cancel each other out

[Alon Levy]

On Tue, Jul 31, 2012 at 08:24:23AM +1000, Benjamin Herrenschmidt wrote:
> On Mon, 2012-07-30 at 18:24 +0200, Alon Levy wrote:
> > On Mon, Jul 30, 2012 at 10:08:07PM +1000, Benjamin Herrenschmidt wrote:
> > > On Mon, 2012-07-30 at 14:58 +0300, Avi Kivity wrote:
> > > > Let's balkanize some more then?
> > > > 
> > > > No, qxl is our paravirt vga, we should improve it instead of spawning
> > > > new ones (which will be horrible in the eyes of the next person to look
> > > > at them).  You should also be getting the drm driver for free.
> > > > 
> > > > http://spice-space.org/page/DRM
> > > 
> > > "Free" is a big word here, I wouldn't be surprised if it was totally
> > > endian busted :-)
> > > 
> > > Why so much effort into a bad design on top of a crack transport btw ?
> > Could you elaborate about the design and transport issues that you see?
> 
> So Anthony listed a few, the transport being inconsistent with all our
> other paravirt model is also part of the problem, and the spice code
> base just hurts my eyes. The fact that it's essentially GDI centric
> makes it a non starter for me anyway.
> 
> > > Is it just RH politics of there's a good reason hiding somewhere ? Some
> > No politics AFAIK. Spice code may suck but it's doing a good job while
> > sucking, so we prefer to fix it unless a good design that makes it
> > easier to rebuild from scratch comes along. I'm all ears.
> > 
> > > kind of morbid fascination for anything Windows ?
> > We do have bad linux performance but the is work going to improve it, by
> > adding RENDER implementation to our driver and protocol additions, among
> > others.
> 
> Which still makes me feel like we should be doing something better
> targeted at Linux exclusively.

Using virtio - +1 - I'm not actively working on it because of priorities
issues, but if it comes along I'll be happy to help making it work as
well as the existing device.

There is really not much different between virtio + linear framebuffer
and the qxl pci device, so I really don't see a lot of issues in
porting.

But other then this issue, I'm not sure what you think is windows
specific: The protocol is not the transport. GDI over a wire is
accurate, but adding RENDER makes it X over the wire (the wire here
being both qxl/future-virtio and the tcp spice protocol). And for the
future we will be passing bo's and TGSI command streams probably to the
host, rendering it and pushing video to the client. But that's still a
way off. Nothing window's specific there either (except the driver part
of course).

> 
> > > People I've talked to around in the community seem to agree that some
> > > minor improvements on qemu-vga are worthwhile, so I'm doing them,
> > > nothing drastic, mostly having a mirror of the legacy IO registers in an
> > > MMIO BAR so it's more usable for non-x86 platforms, and I'm about to add
> > > some simplistic 2D blit facility so we can have a semi-decent fb console
> > > over vnc. I can trivially do an equivalent of cirrusdrmfb for it so we
> > > get X mode setting / RandR.
> > > 
> > > That gives us a baseline for mostly unaccelerated 2D.
> > > 
> > > Something tells me that getting that spice/qxl gunk will be more than a
> > > trivial effort (but I might be wrong) and I'm reluctant to start
> > > committing effort on it since so far I yet have to see it being actually
> > > picked up by people.
> > 
> > I'll be happy to elaborate on any part of qxl/spice that I understand
> > and help you help us improve it.
> 
> Well, the main thing to begin with from my perspective would be to make
> it work on powerpc, and thus handle all the endianness issues etc... but
> at this stage, it seems pretty far out.

I'm sure we can make spice endianess clean. If you look at how it works,
server/red_parse.c reads from the device (so this is the single point to
change if the transport moves from QXLPHYSICAL meaning a encoded
slot/offset into a BAR to some other location identifier like a handle
when using virtio) and writes to spice specific structs, which are then,
like Anthony pointed out, stored in a tree for dropping operations that
have become obsolete before being sent to the client, and also put on a
pipe (queue) of messages to be sent to the client.

That would be the place to do any endianess convertions between guest
and host. The network protocol is LE (yes, counter to the norm, I know.
It is possible to add a capability to spice to change this though if we
really want. So only the initial handshake would have to be LE and the
rest would be BE).

> 
> Cheers,
> Ben.
> 
> 

Re: [Qemu-devel] [PATCH v3 0/2] GlusterFS support in QEMU - v3

[Stefan Hajnoczi]

On Wed, Jul 25, 2012 at 6:58 AM, Bharata B Rao
 wrote:
> Apart from cleanups, the major change in this version is to expose all
> the gluster configuration options to QEMU user. With this, the gluster
> specification looks like this:
>
> -drive file=gluster:server:[port]:[transport]:volname:image
>
> - Here 'gluster' is the protocol.
> - 'server' specifies the server where the volume file specification for
>   the given volume resides.

Works fine for hostnames and IPv4 addresses.  It seems like the ':'
separator may prevent users from giving IPv6 addresses unless there is
a way to escape ':'.

http://en.wikipedia.org/wiki/IPv6_address

Stefan

Re: [Qemu-devel] Cirrus bugs vs endian: how two bugs cancel each other out

[Alon Levy]

On Mon, Jul 30, 2012 at 09:29:01AM -0500, Anthony Liguori wrote:
> Avi Kivity  writes:
> 
> >>> Virtio makes sense for qxl, but for now we have the original pci model
> >>> which I don't see a reason why it can't work for ppc.
> >> 
> >> I'm sure it can work for PPC given enough effort.  But I think the
> >> question becomes, why not invest that effort in moving qxl to the
> >> standard transport that the rest of our PV devices use.
> >
> > The drm drivers for the current model are needed anyway; so moving to
> > virtio is extra effort, not an alternative.
> 
> This is just a point in time statement.  If we were serious about using
> virtio then we could quickly introduce a virtio transport and only
> target the DRM drivers at the virtio transport.
> 
> > Note virtio doesn't support mapping framebuffers yet
> 
> Yes.  I haven't seen a good proposal yet on how to handle this.  I think
> this is the main problem to solve.

The only thing I can add here is perhaps we could use scatter-gather
lists of pages instead of large framebuffer. When just passing commands
from guest->host->client this would mean guests constructs list -> host
reads from list to socket -> client unchanged. For rendering locally on
the host (client wants a screenshot, host wants a screenshot) this would
be a lot of work, basically having a non linear framebuffer version of
pixman. The upside is that you don't need to modify virtio and can use
guest ram.

> 
> > or the entire vga compatibility stuff
> 
> This is actually independent of virtio.  A virtio-pci device could
> expose it's class code as a VGA adapter and also handle I/O accesses for
> the legacy region.  This is strictly a PC-ism.
> 
> For non-virtio-pci versions of the device, the legacy I/O area would not 
> exist.
> 
> > so the pc-oriented card will have to be a mix of
> > virtio and stdvga multiplexed on one pci card (maybe two functions, but
> > I'd rather avoid that).
> 
> Yes.  We could modify stdvga to expose the VGA ram area as the second
> bar and make the first bar a virtio-pci compatible area.  This would
> require modifying the VGA bios to understand the change but otherwise,
> should be compatible.
> 
> It would take modeling VGACommonState as a proper device and then it's a
> pretty simple process of embedding a VGACommonState within a virtio-pci
> device.  It should work fairly well.
> 
> It gets a little complicated in terms of who owns the DisplayState but
> that's a solvable problem.
> 
> Regards,
> 
> Anthony Liguori
> 
> >
> > -- 
> > error compiling committee.c: too many arguments to function
> 

Re: [Qemu-devel] [PATCH 2/5] pseries: Instantiate USB if requested

[Benjamin Herrenschmidt]

On Tue, 2012-07-31 at 16:09 +1000, David Gibson wrote:
> The pseries machine currently ignores the -usb command line option.
> This patch corrects the problem by having it instantiate a PCI OHCI
> USB host controller when -usb is specified.
> 
> Signed-off-by: David Gibson 

Li Zhang  has been owning that patch for a while and
I believe has a better version.

Li, what's the status with it ? Alex ?

Cheers,
Ben.


> ---
>  hw/spapr.c |7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/hw/spapr.c b/hw/spapr.c
> index ab5a0c2..740881b 100644
> --- a/hw/spapr.c
> +++ b/hw/spapr.c
> @@ -45,6 +45,7 @@
>  #include "kvm.h"
>  #include "kvm_ppc.h"
>  #include "pci.h"
> +#include "usb.h"
>  
>  #include "exec-memory.h"
>  
> @@ -710,6 +711,12 @@ static void ppc_spapr_init(ram_addr_t ram_size,
>  spapr_vscsi_create(spapr->vio_bus);
>  }
>  
> +/* USB */
> +if (usb_enabled) {
> +pci_create_simple(QLIST_FIRST(&spapr->phbs)->host_state.bus,
> +  -1, "pci-ohci");
> +}
> +
>  if (rma_size < (MIN_RMA_SLOF << 20)) {
>  fprintf(stderr, "qemu: pSeries SLOF firmware requires >= "
>  "%ldM guest RMA (Real Mode Area memory)\n", MIN_RMA_SLOF);

Re: [Qemu-devel] [PATCH 10/11] Add XBZRLE statistics

[Orit Wasserman]

On 07/30/2012 10:37 PM, Luiz Capitulino wrote:
> On Sun, 29 Jul 2012 12:43:02 +0300
> Orit Wasserman  wrote:
> 
>> Signed-off-by: Benoit Hudzia 
>> Signed-off-by: Petter Svard 
>> Signed-off-by: Aidan Shribman 
>> Signed-off-by: Orit Wasserman 
>> Signed-off-by: Juan Quintela 
>> ---
>>  arch_init.c  |   28 
>>  hmp.c|   21 +
>>  migration.c  |   28 
>>  migration.h  |4 
>>  qapi-schema.json |   32 +++-
>>  qmp-commands.hx  |   35 +++
>>  6 files changed, 147 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch_init.c b/arch_init.c
>> index 7f12317..9833d54 100644
>> --- a/arch_init.c
>> +++ b/arch_init.c
>> @@ -203,7 +203,11 @@ int64_t xbzrle_cache_resize(int64_t new_size)
>>  typedef struct AccountingInfo {
>>  uint64_t dup_pages;
>>  uint64_t norm_pages;
>> +uint64_t xbzrle_bytes;
>> +uint64_t xbzrle_pages;
>> +uint64_t xbzrle_cache_miss;
>>  uint64_t iterations;
>> +uint64_t xbzrle_overflows;
>>  } AccountingInfo;
>>  
>>  static AccountingInfo acct_info;
>> @@ -233,6 +237,26 @@ uint64_t norm_mig_pages_transferred(void)
>>  return acct_info.norm_pages;
>>  }
>>  
>> +uint64_t xbzrle_mig_bytes_transferred(void)
>> +{
>> +return acct_info.xbzrle_bytes;
>> +}
>> +
>> +uint64_t xbzrle_mig_pages_transferred(void)
>> +{
>> +return acct_info.xbzrle_pages;
>> +}
>> +
>> +uint64_t xbzrle_mig_pages_cache_miss(void)
>> +{
>> +return acct_info.xbzrle_cache_miss;
>> +}
>> +
>> +uint64_t xbzrle_mig_pages_overflow(void)
>> +{
>> +return acct_info.xbzrle_overflows;
>> +}
>> +
>>  static void save_block_hdr(QEMUFile *f, RAMBlock *block, ram_addr_t offset,
>>  int cont, int flag)
>>  {
>> @@ -257,6 +281,7 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t 
>> *current_data,
>>  if (!cache_is_cached(XBZRLE.cache, current_addr)) {
>>  cache_insert(XBZRLE.cache, current_addr,
>>   g_memdup(current_data, TARGET_PAGE_SIZE));
>> +acct_info.xbzrle_cache_miss++;
>>  return -1;
>>  }
>>  
>> @@ -274,6 +299,7 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t 
>> *current_data,
>>  return 0;
>>  } else if (encoded_len == -1) {
>>  DPRINTF("Overflow\n");
>> +acct_info.xbzrle_overflows++;
>>  /* update data in the cache */
>>  memcpy(prev_cached_page, current_data, TARGET_PAGE_SIZE);
>>  return -1;
>> @@ -288,6 +314,8 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t 
>> *current_data,
>>  qemu_put_be16(f, encoded_len);
>>  qemu_put_buffer(f, XBZRLE.encoded_buf, encoded_len);
>>  bytes_sent = encoded_len + 1 + 2;
>> +acct_info.xbzrle_pages++;
>> +acct_info.xbzrle_bytes += bytes_sent;
>>  
>>  return bytes_sent;
>>  }
>> diff --git a/hmp.c b/hmp.c
>> index 9770d7b..383d5b1 100644
>> --- a/hmp.c
>> +++ b/hmp.c
>> @@ -172,6 +172,27 @@ void hmp_info_migrate(Monitor *mon)
>> info->disk->total >> 10);
>>  }
>>  
>> +if (info->has_xbzrle_cache) {
>> +monitor_printf(mon, "cache size: %" PRIu64 " bytes\n",
>> +   info->xbzrle_cache->cache_size);
>> +if (info->xbzrle_cache->has_xbzrle_bytes) {
>> +monitor_printf(mon, "xbzrle transferred: %" PRIu64 " kbytes\n",
>> +   info->xbzrle_cache->xbzrle_bytes >> 10);
>> +}
>> +if (info->xbzrle_cache->has_xbzrle_pages) {
>> +monitor_printf(mon, "xbzrle pages: %" PRIu64 " pages\n",
>> +   info->xbzrle_cache->xbzrle_pages);
>> +}
>> +if (info->xbzrle_cache->has_xbzrle_cache_miss) {
>> +monitor_printf(mon, "xbzrle cache miss: %" PRIu64 "\n",
>> +   info->xbzrle_cache->xbzrle_cache_miss);
>> +}
>> +if (info->xbzrle_cache->has_xbzrle_overflow) {
>> +monitor_printf(mon, "xbzrle overflow : %" PRIu64 "\n",
>> +   info->xbzrle_cache->xbzrle_overflow);
>> +}
>> +}
>> +
>>  qapi_free_MigrationInfo(info);
>>  qapi_free_MigrationParameters(params);
>>  }
>> diff --git a/migration.c b/migration.c
>> index 4dc99ba..fb802bc 100644
>> --- a/migration.c
>> +++ b/migration.c
>> @@ -136,6 +136,23 @@ MigrationParameters *qmp_query_migrate_parameters(Error 
>> **errp)
>>  return params;
>>  }
>>  
>> +static void get_xbzrle_cache_stats(MigrationInfo *info)
>> +{
>> +if (migrate_use_xbzrle()) {
>> +info->has_xbzrle_cache = true;
>> +info->xbzrle_cache = g_malloc0(sizeof(*info->xbzrle_cache));
>> +info->xbzrle_cache->cache_size = migrate_xbzrle_cache_size();
>> +info->xbzrle_cache->has_xbzrle_bytes = true;
>> +info->xbzrle_cache->xbzrle_bytes = xbzrle_mig_bytes_transferred();
>> +info->xbzrle_cache->has_xbzrle_pages = true;
>> +info->xbzr

Re: [Qemu-devel] [PATCH 09/11] Add migration accounting for normal and duplicate pages

[Orit Wasserman]

On 07/30/2012 10:30 PM, Luiz Capitulino wrote:
> On Sun, 29 Jul 2012 12:43:01 +0300
> Orit Wasserman  wrote:
> 
>> Signed-off-by: Benoit Hudzia 
>> Signed-off-by: Petter Svard 
>> Signed-off-by: Aidan Shribman 
>> Signed-off-by: Orit Wasserman 
>> Signed-off-by: Juan Quintela 
>> ---
>>  arch_init.c  |   38 ++
>>  migration.c  |4 
>>  migration.h  |5 +
>>  qapi-schema.json |8 ++--
>>  qmp-commands.hx  |   14 +++---
>>  5 files changed, 64 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch_init.c b/arch_init.c
>> index d709ccb..7f12317 100644
>> --- a/arch_init.c
>> +++ b/arch_init.c
>> @@ -199,6 +199,40 @@ int64_t xbzrle_cache_resize(int64_t new_size)
>>  return pow2floor(new_size);
>>  }
>>  
>> +/* accounting for migration statistics */
>> +typedef struct AccountingInfo {
>> +uint64_t dup_pages;
>> +uint64_t norm_pages;
>> +uint64_t iterations;
>> +} AccountingInfo;
>> +
>> +static AccountingInfo acct_info;
>> +
>> +static void acct_clear(void)
>> +{
>> +memset(&acct_info, 0, sizeof(acct_info));
>> +}
>> +
>> +uint64_t dup_mig_bytes_transferred(void)
>> +{
>> +return acct_info.dup_pages * TARGET_PAGE_SIZE;
>> +}
>> +
>> +uint64_t dup_mig_pages_transferred(void)
>> +{
>> +return acct_info.dup_pages;
>> +}
>> +
>> +uint64_t norm_mig_bytes_transferred(void)
>> +{
>> +return acct_info.norm_pages * TARGET_PAGE_SIZE;
>> +}
>> +
>> +uint64_t norm_mig_pages_transferred(void)
>> +{
>> +return acct_info.norm_pages;
>> +}
>> +
>>  static void save_block_hdr(QEMUFile *f, RAMBlock *block, ram_addr_t offset,
>>  int cont, int flag)
>>  {
>> @@ -293,6 +327,7 @@ static int ram_save_block(QEMUFile *f)
>>  p = memory_region_get_ram_ptr(mr) + offset;
>>  
>>  if (is_dup_page(p)) {
>> +acct_info.dup_pages++;
>>  save_block_hdr(f, block, offset, cont, 
>> RAM_SAVE_FLAG_COMPRESS);
>>  qemu_put_byte(f, *p);
>>  bytes_sent = 1;
>> @@ -308,6 +343,7 @@ static int ram_save_block(QEMUFile *f)
>>  save_block_hdr(f, block, offset, cont, RAM_SAVE_FLAG_PAGE);
>>  qemu_put_buffer(f, p, TARGET_PAGE_SIZE);
>>  bytes_sent = TARGET_PAGE_SIZE;
>> +acct_info.norm_pages++;
>>  }
>>  
>>  /* if page is unmodified, continue to the next */
>> @@ -429,6 +465,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
>>  }
>>  XBZRLE.encoded_buf = g_malloc0(TARGET_PAGE_SIZE);
>>  XBZRLE.current_buf = g_malloc(TARGET_PAGE_SIZE);
>> +acct_clear();
>>  }
>>  
>>  /* Make sure all dirty bits are set */
>> @@ -477,6 +514,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
>>  break;
>>  }
>>  bytes_transferred += bytes_sent;
>> +acct_info.iterations++;
>>  /* we want to check in the 1st loop, just in case it was the 1st 
>> time
>> and we had to sync the dirty bitmap.
>> qemu_get_clock_ns() is a bit expensive, so we only check each 
>> some
>> diff --git a/migration.c b/migration.c
>> index bc2231d..4dc99ba 100644
>> --- a/migration.c
>> +++ b/migration.c
>> @@ -156,6 +156,8 @@ MigrationInfo *qmp_query_migrate(Error **errp)
>>  info->ram->total = ram_bytes_total();
>>  info->ram->total_time = qemu_get_clock_ms(rt_clock)
>>  - s->total_time;
>> +info->ram->duplicate = dup_mig_pages_transferred();
>> +info->ram->normal = norm_mig_pages_transferred();
>>  
>>  if (blk_mig_active()) {
>>  info->has_disk = true;
>> @@ -175,6 +177,8 @@ MigrationInfo *qmp_query_migrate(Error **errp)
>>  info->ram->remaining = 0;
>>  info->ram->total = ram_bytes_total();
>>  info->ram->total_time = s->total_time;
>> +info->ram->duplicate = dup_mig_pages_transferred();
>> +info->ram->normal = norm_mig_pages_transferred();
>>  break;
>>  case MIG_STATE_ERROR:
>>  info->has_status = true;
>> diff --git a/migration.h b/migration.h
>> index 337e225..e4a7cd7 100644
>> --- a/migration.h
>> +++ b/migration.h
>> @@ -87,6 +87,11 @@ uint64_t ram_bytes_total(void);
>>  
>>  extern SaveVMHandlers savevm_ram_handlers;
>>  
>> +uint64_t dup_mig_bytes_transferred(void);
>> +uint64_t dup_mig_pages_transferred(void);
>> +uint64_t norm_mig_bytes_transferred(void);
>> +uint64_t norm_mig_pages_transferred(void);
>> +
>>  /**
>>   * @migrate_add_blocker - prevent migration from proceeding
>>   *
>> diff --git a/qapi-schema.json b/qapi-schema.json
>> index 04adcee..a936714 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -264,11 +264,15 @@
>>  #migration has ended, it returns the total migration
>>  #time. (since 1.2)
>>  #
>> -# Since: 0.14.0.
>> +# @duplicate: number of duplicate pages (since 1.2)
>> +#
>> +# @normal : number of normal

[Qemu-devel] Tracing drivers commands to an emulated device

[ffdevel]

Hello to all,
I would trace the commands issued by the driver to an emulated device. I've 
read in docs/tracing that it is possible but it is not completely clear how 
doing that. Is it possible to place hooks inside the emulated device code? Or 
is better tracing the in/out operations and the access to the memory mapped 
zone?
Thank you for any suggestion or help.

Francesco
Inviato dal mio smartphone BlackBerry® 
www.blackberry.com

Re: [Qemu-devel] Cirrus bugs vs endian: how two bugs cancel each other out

[ronnie sahlberg]

On Mon, Jul 30, 2012 at 4:24 PM, Benjamin Herrenschmidt
 wrote:
> So I got cirrus working on ppc with cirrusdrmfb...
>
> The fun part is that it works :-)
>
> Basically, the issue is that normally, for it to work, one would have to
> access the framebuffer using the appropriate aperture for byteswapping
> based on the bpp.
>
> However, qemu doesn't emulate those apertures ... and cirrusdrmfb
> either.
>
> In fact, qemu cirrus model is just dumb and assumes guest native
> byteorder for the framebuffer.
>
> The good thing is that this makes it work... the bad thing is that it's
> a completely incorrect HW model and if the linux driver wasn't also
> buggy it wouldn't work.
>
> However it's also pretty much unfixable without making it also unusable
> in terms of performance so I want to check with you guys if it's ok to
> just leave it as-is.
>
> Basically, if the fb was LE as it's supposed to be, one would have to
> use the byteswapped apertures. But those can only be emulated by
> trapping on every access to turn it into MMIO emulation, which means
> unusable performances.
>
> So we end up with what is effectively a BE framebuffer thanks to qemu
> hard coding what it thinks the guest endian is (btw, this is quite
> busted in theory as well since PPC can be bi-endian for example).
>
> Anyways, it works today, it's just that the HW model is wrong... and I
> don't want to fix it. Any objection ?
>
> As for the work I'm doing to brush up pci-vga a bit, I'm tempted to add
> an MMIO reg or a VBE config reg bit to allow configuring the endianness
> of the underlying fb with a default to what qemu does today.
>
> Cheers,
> Ben.
>
>
>

I use lots of guests that will never ever get virtio drivers.
So for those guests, any work on making sure bog standard vga keeps
working or even improving it
gets two thumbs up from me!

Re: [Qemu-devel] [PATCH 04/47] block: add block_job_query

[Kevin Wolf]

Am 30.07.2012 17:05, schrieb Paolo Bonzini:
> Il 30/07/2012 16:47, Kevin Wolf ha scritto:
 +BlockJobInfo *block_job_query(BlockJob *job)
 +{
 +BlockJobInfo *info = g_new(BlockJobInfo, 1);
 +info->type   = g_strdup(job->job_type->job_type);
 +info->device = g_strdup(bdrv_get_device_name(job->bs));
 +info->len= job->len;
 +info->offset = job->offset;
 +info->speed  = job->speed;
 +return info;
 +}
>> Why did you convert the initialisation to separate statement? If you
>> really want to do this, I think using g_new0 would be safer now, but I
>> actually like compound literals better.
> 
> Later on I will have some more initialization beyond the list of fields,
> so I preferred an explicit list.  I can change it back if you prefer.

What I'm really interested in is having zero-initialisation for any not
explicitly initialised fields, just to be on the safe side. You can do
that with g_new0() or with compound literals, that's a matter of taste.
My taste happens to prefer the latter, but I won't criticise a patch
based on taste as long as it's doing the same thing functionally.

Kevin

Re: [Qemu-devel] [PATCH v3 0/2] GlusterFS support in QEMU - v3

[Stefan Hajnoczi]

On Mon, Jul 30, 2012 at 5:46 AM, Bharata B Rao
 wrote:
> On Wed, Jul 25, 2012 at 11:28:06AM +0530, Bharata B Rao wrote:
>> Hi,
>>
>> This is the v3 of the patchset to support GlusterFS backend from QEMU.
>
> I am planning a v4 post to address a few minor cleanups suggested by
> Blue Swirl. I would like to know if there are any other comments or test
> scenarios that I need to take care before this GlusterFS support in
> QEMU can be considered for inclusion.
>
> I have tested these patches in the following scenarios:
>
> - local gluster volume (QEMU and gluster volume residing on the same m/c)
> - remote gluster volume
> - local distributed+replicated volume
> - remote distributed volume
> - use of 2 gluster drives
> - live migration (VM image resides on gluster volume consisting of bricks on
>   machine A and B, migrating QEMU from machine C to D)
>
> When I say "tested", I mean that I have created VM images using qemu-img
> on these volumes, installed VM, booted from it and run fio benchmark
> with various cache options (none, writeback, writethrough)

Besides the cleanups that have been suggested and the IPv6 issue:

Reviewed-by: Stefan Hajnoczi 

Re: [Qemu-devel] [PATCH 04/47] block: add block_job_query

[Paolo Bonzini]

Il 31/07/2012 10:47, Kevin Wolf ha scritto:
>>> >> Why did you convert the initialisation to separate statement? If you
>>> >> really want to do this, I think using g_new0 would be safer now, but I
>>> >> actually like compound literals better.
>> > 
>> > Later on I will have some more initialization beyond the list of fields,
>> > so I preferred an explicit list.  I can change it back if you prefer.
> What I'm really interested in is having zero-initialisation for any not
> explicitly initialised fields, just to be on the safe side. You can do
> that with g_new0() or with compound literals, that's a matter of taste.

Yes, and in fact I even have a change to g_new0 later in the series.
I'll squash that change in this patch.

Paolo

Re: [Qemu-devel] [PATCH v2 00/16] net: Move legacy QEMU VLAN code into net/hub.c

[Stefan Hajnoczi]

On Tue, Jul 31, 2012 at 10:22:37AM +0800, Zhi Yong Wu wrote:
> On Mon, Jul 30, 2012 at 11:49 PM, Stefan Hajnoczi  wrote:
> > On Tue, Jul 24, 2012 at 4:35 PM, Stefan Hajnoczi
> >  wrote:
> >> [These patches are based on the net tree at git://github.com/stefanha/net]
> >>
> >> The QEMU net subsystem has the concept of separate network segments, called
> >> "VLANs".  Each VLAN is a broadcast domain so all net clients connected to 
> >> the
> >> same VLAN can communicate with each other.
> >>
> >> Today this feature is mostly used with the "dump" backend, which saves 
> >> packet
> >> captures to .pcap files.  It can still come in useful in other rare cases.
> >>
> >> This patch series moves the VLAN code out of net.c and into net/hub.c.  The
> >> idea is to introduce a software network hub along with hub port net 
> >> clients.
> >> This way we can implement the same semantics of QEMU VLANs using just 
> >> -netdev
> >> peer.  Then -netdev peer becomes the single method of connecting net 
> >> clients.
> >>
> >> The end result of this series is that the net subsystem core is simplified.
> >> Now is a good time to do this because it saves us from modeling QEMU VLANs 
> >> when
> >> we convert the net subsystem to QOM.
> >>
> >> Please note that this series preserves command-line compatibility with the 
> >> QEMU
> >> VLAN feature.  No existing QEMU command-lines should break.
> >>
> >> I have tested the following configurations:
> >>  * -net user -net nic,model=virtio
> >>  * -net user,vlan=1 -net nic,model=virtio,vlan=1
> >>  * -net user,vlan=1 -net nic,model=virtio,vlan=1 -net user,vlan=2 -net 
> >> nic,model=virtio,vlan=2
> >>  * -netdev user,id=netdev0 -device virtio-net-pci,netdev=netdev0
> >>  * -netdev user,id=netdev0 -device virtio-net-pci,netdev=netdev0 -net 
> >> user,vlan=2 -net nic,model=virtio,vlan=2
> >>
> >> v2:
> >>  * Change int64_t and unsigned int mess to int which is what VLAN IDs are 
> >> today [Laszlo]
> >>  * Remove bogus error_set() -> qerror_report() merge artifact [Laszlo]
> >>  * Use net_hub_id_for_client(nc, NULL) == 0 instead of adding 
> >> net_hub_port_peer_nc() [Laszlo]
> >>  * Fix stray print_net_client(..., NetClientState *vc) argument name 
> >> [Laszlo]
> >>  * Consistently update vlan -> peer argument name [Laszlo]
> >>  * Drop spurious closesocket(fd), probably merge conflict [Stefan]
> >>
> >> Stefan Hajnoczi (11):
> >>   net: Add a hub net client
> >>   net: Use hubs for the vlan feature
> >>   net: Look up 'vlan' net clients using hubs
> >>   hub: Check that hubs are configured correctly
> >>   net: Drop vlan argument to qemu_new_net_client()
> >>   net: Remove vlan code from net.c
> >>   net: Remove VLANState
> >>   net: Rename non_vlan_clients to net_clients
> >>   net: Rename VLANClientState to NetClientState
> >>   net: Rename vc local variables to nc
> >>   net: Rename qemu_del_vlan_client() to qemu_del_net_client()
> >>
> >> Zhi Yong Wu (5):
> >>   net: Convert qdev_prop_vlan to peer with hub
> >>   net: Make "info network" output more readable info
> >>   net: cleanup deliver/deliver_iov func pointers
> >>   net: determine if packets can be sent before net queue deliver
> >> packets
> >>   hub: add the support for hub own flow control
> >>
> >>  hw/cadence_gem.c|8 +-
> >>  hw/dp8393x.c|7 +-
> >>  hw/e1000.c  |   10 +-
> >>  hw/eepro100.c   |8 +-
> >>  hw/etraxfs_eth.c|8 +-
> >>  hw/exynos4_boards.c |2 +-
> >>  hw/highbank.c   |2 +-
> >>  hw/integratorcp.c   |2 +-
> >>  hw/kzm.c|2 +-
> >>  hw/lan9118.c|8 +-
> >>  hw/lance.c  |2 +-
> >>  hw/mcf5208.c|2 +-
> >>  hw/mcf_fec.c|7 +-
> >>  hw/milkymist-minimac2.c |6 +-
> >>  hw/mips_mipssim.c   |2 +-
> >>  hw/mips_r4k.c   |2 +-
> >>  hw/mipsnet.c|6 +-
> >>  hw/musicpal.c   |6 +-
> >>  hw/ne2000-isa.c |2 +-
> >>  hw/ne2000.c |8 +-
> >>  hw/ne2000.h |4 +-
> >>  hw/opencores_eth.c  |8 +-
> >>  hw/pcnet-pci.c  |4 +-
> >>  hw/pcnet.c  |6 +-
> >>  hw/pcnet.h  |6 +-
> >>  hw/qdev-properties.c|   54 +++--
> >>  hw/qdev.c   |2 -
> >>  hw/qdev.h   |7 +-
> >>  hw/rtl8139.c|   10 +-
> >>  hw/smc91c111.c  |6 +-
> >>  hw/spapr_llan.c |4 +-
> >>  hw/stellaris_enet.c |6 +-
> >>  hw/usb/dev-network.c|8 +-
> >>  hw/vexpress.c   |2 +-
> >>  hw/vhost_net.c  |   24 +-
> >>  hw/vhost_net.h  |2 +-
> >>  hw/virtio-net.c |   12 +-
> >>  hw/xen_nic.c|7 +-
> >>  hw/xgmac.c  |6 +-
> >>  hw/xilinx_axienet.c |6 +-
> >>  hw/xilinx_ethlite.c |6 +-
> >>  hw/xtensa_lx60.c|2 +-
> >>  net.c   |  616 
> >> ++-

Re: [Qemu-devel] [PATCH 28/47] qmp: add drive-mirror command

[Kevin Wolf]

Am 24.07.2012 13:04, schrieb Paolo Bonzini:
> This adds the monitor commands that start the mirroring job.
> 
> Signed-off-by: Paolo Bonzini 

[ Moving the discussion upstream ]

> Why make all of it inaccessible?  Everything except target device access
> does have a stable API.  The target device access can be delayed to 1.3,
> together with the much-needed QMP schema introspection.

I'm not even sure about the QMP mirror command itself.

I don't really like it, it does too many things at once: It can create
the target image file, it opens the target and it actually starts the
mirroring. It's rather bad at the first two steps, because it doesn't
take any options. This means that it can't create qcow2v3 images, for
example. Or you can't mirror into a backup with cache=unsafe while
running your real VM on cache=writethrough.

Having an all-in-one mirror command is a nice feature for HMP, but for
QMP it's more like a design problem.

Now I see you have called it drive-mirror, so that kind of implies that
it's not the final blockdev-mirror but just a QMP version of a command
primarily designed for HMP. As such this restricted functionality may be
acceptable, but it's not like everything is already perfect and there's
no room for discussion.

Kevin

Re: [Qemu-devel] [PATCH V4 04/12] hw/sd.c: favour SD card type (SDSC or SDHC) when performing erase

[Markus Armbruster]

Igor Mitsyanko  writes:

> Standard capacity cards SDSC use byte unit address while SDHC and SDXC Cards 
> use
> block unit address (512 bytes) when setting ERASE_START and ERASE_END with 
> CMD32
> and CMD33, we have to account for this.
>
> Signed-off-by: Igor Mitsyanko 
> ---
>  hw/sd.c |6 --
>  1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/hw/sd.c b/hw/sd.c
> index d0674d5..f7aa580 100644
> --- a/hw/sd.c
> +++ b/hw/sd.c
> @@ -482,8 +482,10 @@ static void sd_erase(SDState *sd)
>  return;
>  }
>  
> -start = sd_addr_to_wpnum(sd->erase_start);
> -end = sd_addr_to_wpnum(sd->erase_end);
> +start = sd_addr_to_wpnum(sd->ocr & (1 << 30) ?
> +(uint64_t)sd->erase_start * 512 : sd->erase_start);
> +end = sd_addr_to_wpnum(sd->ocr & (1 << 30) ?
> +(uint64_t)sd->erase_end * 512 : sd->erase_end);
>  sd->erase_start = 0;
>  sd->erase_end = 0;
>  sd->csd[14] |= 0x40;

Is this a bug fix?

If yes, I recommend to state that clearly in the subject, say "hw/sd.c:
Fix erase for high capacity cards".

Re: [Qemu-devel] [PATCH V4 08/12] hw/sd.c: add SD card save/load support

[Markus Armbruster]

Igor Mitsyanko  writes:

> This patch updates SD card model to support save/load of card's state.
>
> Signed-off-by: Igor Mitsyanko 
> ---
>  hw/sd.c |   88 +-
>  1 files changed, 64 insertions(+), 24 deletions(-)
>
> diff --git a/hw/sd.c b/hw/sd.c
> index 20ebd8e..f8ab045 100644
> --- a/hw/sd.c
> +++ b/hw/sd.c
> @@ -55,24 +55,28 @@ typedef enum {
>  sd_illegal = -2,
>  } sd_rsp_type_t;
>  
> +enum {
> +sd_inactive,
> +sd_card_identification_mode,
> +sd_data_transfer_mode,
> +};
> +
> +enum {
> +sd_inactive_state = -1,
> +sd_idle_state = 0,
> +sd_ready_state,
> +sd_identification_state,
> +sd_standby_state,
> +sd_transfer_state,
> +sd_sendingdata_state,
> +sd_receivingdata_state,
> +sd_programming_state,
> +sd_disconnect_state,
> +};
> +
>  struct SDState {
> -enum {
> -sd_inactive,
> -sd_card_identification_mode,
> -sd_data_transfer_mode,
> -} mode;
> -enum {
> -sd_inactive_state = -1,
> -sd_idle_state = 0,
> -sd_ready_state,
> -sd_identification_state,
> -sd_standby_state,
> -sd_transfer_state,
> -sd_sendingdata_state,
> -sd_receivingdata_state,
> -sd_programming_state,
> -sd_disconnect_state,
> -} state;
> +uint32_t mode;
> +int32_t state;

Comment pointing to the related enum?

>  uint32_t ocr;
>  uint8_t scr[8];
>  uint8_t cid[16];

[...]

Re: [Qemu-devel] [PATCH 28/47] qmp: add drive-mirror command

[Paolo Bonzini]

Il 31/07/2012 11:26, Kevin Wolf ha scritto:
> Am 24.07.2012 13:04, schrieb Paolo Bonzini:
>> This adds the monitor commands that start the mirroring job.
>>
>> Signed-off-by: Paolo Bonzini 
> 
> [ Moving the discussion upstream ]
> 
>> Why make all of it inaccessible?  Everything except target device access
>> does have a stable API.  The target device access can be delayed to 1.3,
>> together with the much-needed QMP schema introspection.
> 
> I'm not even sure about the QMP mirror command itself.
> 
> I don't really like it, it does too many things at once: It can create
> the target image file, it opens the target and it actually starts the
> mirroring. It's rather bad at the first two steps, because it doesn't
> take any options. This means that it can't create qcow2v3 images, for
> example. Or you can't mirror into a backup with cache=unsafe while
> running your real VM on cache=writethrough.

Yes, though this can be worked around with mode: 'existing'.

> Having an all-in-one mirror command is a nice feature for HMP, but for
> QMP it's more like a design problem.
> 
> Now I see you have called it drive-mirror

I thought this was your idea. :)

> , so that kind of implies that
> it's not the final blockdev-mirror but just a QMP version of a command
> primarily designed for HMP. As such this restricted functionality may be
> acceptable, but it's not like everything is already perfect and there's
> no room for discussion.

We keep going back to the same point that we do not have -blockdev, but
it's becoming a bit frustrating to always rehash this same point...

Paolo

Re: [Qemu-devel] [PATCH V4 09/12] hw/sd.c: convert SD state to QOM object

[Markus Armbruster]

Igor Mitsyanko  writes:

> A straightforward conversion of SD card implementation to a proper QEMU 
> object.
> Wrapper functions were introduced for SDClass methods in order to avoid SD 
> card
> users modification. Because of this, name change for several functions in 
> hw/sd.c
> was required.
>
> Signed-off-by: Igor Mitsyanko 
> ---
>  hw/sd.c |   56 ++--
>  hw/sd.h |   67 +++---
>  2 files changed, 100 insertions(+), 23 deletions(-)
>
> diff --git a/hw/sd.c b/hw/sd.c
> index f8ab045..fe2c138 100644
> --- a/hw/sd.c
> +++ b/hw/sd.c
> @@ -75,6 +75,8 @@ enum {
>  };
>  
>  struct SDState {
> +Object parent_obj;
> +
>  uint32_t mode;
>  int32_t state;
>  uint32_t ocr;
> @@ -489,11 +491,8 @@ static const VMStateDescription sd_vmstate = {
> whether card should be in SSI or MMC/SD mode.  It is also up to the
> board to ensure that ssi transfers only occur when the chip select
> is asserted.  */
> -SDState *sd_init(BlockDriverState *bs, bool is_spi)
> +static void sd_init_card(SDState *sd, BlockDriverState *bs, bool is_spi)
>  {
> -SDState *sd;
> -
> -sd = (SDState *) g_malloc0(sizeof(SDState));
>  sd->buf = qemu_blockalign(bs, 512);
>  sd->spi = is_spi;
>  sd->enable = true;
> @@ -503,10 +502,9 @@ SDState *sd_init(BlockDriverState *bs, bool is_spi)
>  bdrv_set_dev_ops(sd->bdrv, &sd_block_ops, sd);
>  }
>  vmstate_register(NULL, -1, &sd_vmstate, sd);
> -return sd;
>  }
>  
> -void sd_set_cb(SDState *sd, qemu_irq readonly, qemu_irq insert)
> +static void sd_set_callbacks(SDState *sd, qemu_irq readonly, qemu_irq insert)
>  {
>  sd->readonly_cb = readonly;
>  sd->inserted_cb = insert;
> @@ -1334,7 +1332,7 @@ static int cmd_valid_while_locked(SDState *sd, 
> SDRequest *req)
>  return sd_cmd_class[req->cmd] == 0 || sd_cmd_class[req->cmd] == 7;
>  }
>  
> -int sd_do_command(SDState *sd, SDRequest *req,
> +static int sd_send_command(SDState *sd, SDRequest *req,
>uint8_t *response) {
>  int last_state;
>  sd_rsp_type_t rtype;
> @@ -1502,7 +1500,7 @@ static void sd_blk_write(SDState *sd, uint64_t addr, 
> uint32_t len)
>  #define APP_READ_BLOCK(a, len)   memset(sd->data, 0xec, len)
>  #define APP_WRITE_BLOCK(a, len)
>  
> -void sd_write_data(SDState *sd, uint8_t value)
> +static void sd_write_card_data(SDState *sd, uint8_t value)
>  {
>  int i;
>  
> @@ -1510,7 +1508,7 @@ void sd_write_data(SDState *sd, uint8_t value)
>  return;
>  
>  if (sd->state != sd_receivingdata_state) {
> -fprintf(stderr, "sd_write_data: not in Receiving-Data state\n");
> +fprintf(stderr, "sd_write_card_data: not in Receiving-Data state\n");

Outside this patch's scope, but here goes anyway: what kind of condition
is reported here?  Programming error that should never happen?  Guest
doing something weird?

Same for all the other fprintf(stderr, ...) in this file.

>  return;
>  }
>  
> @@ -1621,12 +1619,12 @@ void sd_write_data(SDState *sd, uint8_t value)
>  break;
>  
>  default:
> -fprintf(stderr, "sd_write_data: unknown command\n");
> +fprintf(stderr, "sd_write_card_data: unknown command\n");
>  break;
>  }
>  }
>  
> -uint8_t sd_read_data(SDState *sd)
> +static uint8_t sd_read_card_data(SDState *sd)
>  {
>  /* TODO: Append CRCs */
>  uint8_t ret;
> @@ -1636,7 +1634,7 @@ uint8_t sd_read_data(SDState *sd)
>  return 0x00;
>  
>  if (sd->state != sd_sendingdata_state) {
> -fprintf(stderr, "sd_read_data: not in Sending-Data state\n");
> +fprintf(stderr, "sd_read_card_data: not in Sending-Data state\n");
>  return 0x00;
>  }
>  
> @@ -1738,19 +1736,47 @@ uint8_t sd_read_data(SDState *sd)
>  break;
>  
>  default:
> -fprintf(stderr, "sd_read_data: unknown command\n");
> +fprintf(stderr, "sd_read_card_data: unknown command\n");
>  return 0x00;
>  }
>  
>  return ret;
>  }
>  
> -bool sd_data_ready(SDState *sd)
> +static bool sd_is_data_ready(SDState *sd)
>  {
>  return sd->state == sd_sendingdata_state;
>  }
>  
> -void sd_enable(SDState *sd, bool enable)
> +static void sd_enable_card(SDState *sd, bool enable)
>  {
>  sd->enable = enable;
>  }
> +
> +static void sd_class_init(ObjectClass *klass, void *data)
> +{
> +SDClass *k = SD_CLASS(klass);
> +
> +k->init = sd_init_card;
> +k->set_cb = sd_set_callbacks;
> +k->do_command = sd_send_command;
> +k->data_ready = sd_is_data_ready;
> +k->read_data = sd_read_card_data;
> +k->write_data = sd_write_card_data;
> +k->enable = sd_enable_card;
> +}
> +
> +static const TypeInfo sd_type_info = {
> +.name = TYPE_SD_CARD,
> +.parent = TYPE_OBJECT,

Possibly ignorant question: why TYPE_OBJECT, not TYPE_DEVICE?

> +.instance_size = sizeof(SDState),
> +.class_init = sd_class_

Re: [Qemu-devel] [PATCH 28/47] qmp: add drive-mirror command

[Kevin Wolf]

Am 31.07.2012 11:33, schrieb Paolo Bonzini:
> Il 31/07/2012 11:26, Kevin Wolf ha scritto:
>> Am 24.07.2012 13:04, schrieb Paolo Bonzini:
>>> This adds the monitor commands that start the mirroring job.
>>>
>>> Signed-off-by: Paolo Bonzini 
>>
>> [ Moving the discussion upstream ]
>>
>>> Why make all of it inaccessible?  Everything except target device access
>>> does have a stable API.  The target device access can be delayed to 1.3,
>>> together with the much-needed QMP schema introspection.
>>
>> I'm not even sure about the QMP mirror command itself.
>>
>> I don't really like it, it does too many things at once: It can create
>> the target image file, it opens the target and it actually starts the
>> mirroring. It's rather bad at the first two steps, because it doesn't
>> take any options. This means that it can't create qcow2v3 images, for
>> example. Or you can't mirror into a backup with cache=unsafe while
>> running your real VM on cache=writethrough.
> 
> Yes, though this can be worked around with mode: 'existing'.

True. Only the problem with image creation, though, not the one with
bdrv_open() flags, right?

>> Having an all-in-one mirror command is a nice feature for HMP, but for
>> QMP it's more like a design problem.
>>
>> Now I see you have called it drive-mirror
> 
> I thought this was your idea. :)

Hm, then probably we discussed similar things before. :-)

>> , so that kind of implies that
>> it's not the final blockdev-mirror but just a QMP version of a command
>> primarily designed for HMP. As such this restricted functionality may be
>> acceptable, but it's not like everything is already perfect and there's
>> no room for discussion.
> 
> We keep going back to the same point that we do not have -blockdev, but
> it's becoming a bit frustrating to always rehash this same point...

The question is whether we need it at all. We do have a drive_add
if=none, and for creating a mirror target that should actually be enough.

Kevin

Re: [Qemu-devel] [PATCH] kvm: Check if smp_cpus exceeds max cpus supported by kvm

[Stefan Hajnoczi]

On Mon, Jul 30, 2012 at 7:22 PM,   wrote:
> From: Dunrong Huang 
>
> Add a helper function for fetching max cpus supported by kvm.
>
> Make QEMU exit with an error message if smp_cpus exceeds limit
> of VCPU count retrieved by invoking this helper function.
>
> Signed-off-by: Dunrong Huang 
> ---
>  kvm-all.c |   25 +
>  1 files changed, 25 insertions(+), 0 deletions(-)
>
> diff --git a/kvm-all.c b/kvm-all.c
> index 2148b20..8cb4b92 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -1207,6 +1207,23 @@ static int kvm_irqchip_create(KVMState *s)
>  return 0;
>  }
>
> +static int kvm_max_vcpus(KVMState *s)
> +{
> +int max_vcpus = 4;
> +int ret;
> +ret = kvm_check_extension(s, KVM_CAP_MAX_VCPUS);
> +if (ret) {
> +max_vcpus = ret;
> +} else {
> +   ret = kvm_check_extension(s, KVM_CAP_NR_VCPUS);
> +if (ret) {
> +max_vcpus = ret;
> +}
> +   }

The indentation is off here.  It should be 4 spaces.

Otherwise looks fine.

Stefan

[Qemu-devel] [PATCH] Fix ALSA configure check

[Paul Brook]

Recent gcc notice that the ASLA configure check uses an uninitialized
variable, causing spurious failures.  Adjust the testcase to avoid this.

Signed-off-by: Paul Brook 
---
 configure |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure b/configure
index c65b5f6..9152798 100755
--- a/configure
+++ b/configure
@@ -1890,7 +1890,7 @@ for drv in $audio_drv_list; do
 case $drv in
 alsa)
 audio_drv_probe $drv alsa/asoundlib.h -lasound \
-"snd_pcm_t **handle; return snd_pcm_close(*handle);"
+"snd_pcm_t *handle = NULL; return snd_pcm_close(handle);"
 libs_softmmu="-lasound $libs_softmmu"
 ;;
 
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH V4 11/12] SD card: introduce "spi" property for SD card objects

[Markus Armbruster]

Igor Mitsyanko  writes:

> And drop passing is_spi argument to SDCardClass::init function.
> "spi" property could be set only while SD card object is not
> attached to any BlockDriverState.
> It defaults to "false".
>
> Signed-off-by: Igor Mitsyanko 
> Cc: Paul Brook 
> ---
>  hw/sd.c |   33 +++--
>  hw/sd.h |8 ++--
>  2 files changed, 37 insertions(+), 4 deletions(-)
>
> diff --git a/hw/sd.c b/hw/sd.c
> index fe2c138..611e1f3 100644
> --- a/hw/sd.c
> +++ b/hw/sd.c
> @@ -491,10 +491,9 @@ static const VMStateDescription sd_vmstate = {
> whether card should be in SSI or MMC/SD mode.  It is also up to the
> board to ensure that ssi transfers only occur when the chip select
> is asserted.  */
> -static void sd_init_card(SDState *sd, BlockDriverState *bs, bool is_spi)
> +static void sd_init_card(SDState *sd, BlockDriverState *bs)
>  {
>  sd->buf = qemu_blockalign(bs, 512);
> -sd->spi = is_spi;
>  sd->enable = true;
>  sd_reset(sd, bs);
>  if (sd->bdrv) {
> @@ -1766,10 +1765,40 @@ static void sd_class_init(ObjectClass *klass, void 
> *data)
>  k->enable = sd_enable_card;
>  }
>  
> +static void sd_is_spi(Object *obj, Visitor *v, void *opaque,
> + const char *name, Error **errp)
> +{
> +SDState *sd = SD_CARD(obj);
> +
> +visit_type_bool(v, &sd->spi, name, errp);
> +}
> +
> +static void sd_set_spimode(Object *obj, Visitor *v, void *opaque,
> + const char *name, Error **errp)
> +{
> +SDState *sd = SD_CARD(obj);
> +
> +if (sd->bdrv) {
> +error_set(errp, QERR_DEVICE_IN_USE, bdrv_get_device_name(sd->bdrv));
> +} else {
> +visit_type_bool(v, &sd->spi, name, errp);
> +}
> +}
> +
> +static void sd_initfn(Object *obj)
> +{
> +SDState *sd = SD_CARD(obj);
> +
> +sd->spi = false;
> +object_property_add(obj, "spi", "boolean", sd_is_spi, sd_set_spimode,
> +NULL, NULL, NULL);
> +}
> +
>  static const TypeInfo sd_type_info = {
>  .name = TYPE_SD_CARD,
>  .parent = TYPE_OBJECT,
>  .instance_size = sizeof(SDState),
> +.instance_init = sd_initfn,
>  .class_init = sd_class_init,
>  .class_size = sizeof(SDClass)
>  };

I suspect this would be much simpler the declarative way qdevs normally
use.  For an example, check out scsi_hd_properties[] and its use in
hw/scsi-disk.c.

> diff --git a/hw/sd.h b/hw/sd.h
> index f84e863..c78eaa1 100644
> --- a/hw/sd.h
> +++ b/hw/sd.h
> @@ -31,6 +31,7 @@
>  
>  #include "qemu-common.h"
>  #include "qemu/object.h"
> +#include "qapi/qapi-visit-core.h"
>  
>  #define OUT_OF_RANGE (1 << 31)
>  #define ADDRESS_ERROR(1 << 30)
> @@ -73,7 +74,7 @@ typedef struct SDState SDState;
>  typedef struct SDClass {
>  ObjectClass parent_class;
>  
> -void (*init)(SDState *sd, BlockDriverState *bs, bool is_spi);
> +void (*init)(SDState *sd, BlockDriverState *bdrv);
>  int (*do_command)(SDState *sd, SDRequest *req, uint8_t *response);
>  void (*write_data)(SDState *sd, uint8_t value);
>  uint8_t (*read_data)(SDState *sd);
> @@ -93,7 +94,10 @@ typedef struct SDClass {
>  static inline SDState *sd_init(BlockDriverState *bs, bool is_spi)
>  {
>  SDState *sd = SD_CARD(object_new(TYPE_SD_CARD));
> -SD_GET_CLASS(sd)->init(sd, bs, is_spi);
> +Error *errp = NULL;
> +object_property_set_bool(OBJECT(sd), is_spi, "spi", &errp);
> +assert_no_error(errp);
> +SD_GET_CLASS(sd)->init(sd, bs);
>  return sd;
>  }

Re: [Qemu-devel] [PATCH] Fix ALSA configure check

[malc]

On Tue, 31 Jul 2012, Paul Brook wrote:

> Recent gcc notice that the ASLA configure check uses an uninitialized
> variable, causing spurious failures.  Adjust the testcase to avoid this.

http://lists.nongnu.org/archive/html/qemu-devel/2012-07/msg02704.html

> 
> Signed-off-by: Paul Brook 
> ---
>  configure |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/configure b/configure
> index c65b5f6..9152798 100755
> --- a/configure
> +++ b/configure
> @@ -1890,7 +1890,7 @@ for drv in $audio_drv_list; do
>  case $drv in
>  alsa)
>  audio_drv_probe $drv alsa/asoundlib.h -lasound \
> -"snd_pcm_t **handle; return snd_pcm_close(*handle);"
> +"snd_pcm_t *handle = NULL; return snd_pcm_close(handle);"
>  libs_softmmu="-lasound $libs_softmmu"
>  ;;
>  
> 

-- 
mailto:av1...@comtv.ru

Re: [Qemu-devel] [PATCH V4 09/12] hw/sd.c: convert SD state to QOM object

[Peter Maydell]

On 31 July 2012 10:45, Markus Armbruster  wrote:
> Igor Mitsyanko  writes:
>> @@ -1510,7 +1508,7 @@ void sd_write_data(SDState *sd, uint8_t value)
>>  return;
>>
>>  if (sd->state != sd_receivingdata_state) {
>> -fprintf(stderr, "sd_write_data: not in Receiving-Data state\n");
>> +fprintf(stderr, "sd_write_card_data: not in Receiving-Data 
>> state\n");
>
> Outside this patch's scope, but here goes anyway: what kind of condition
> is reported here?  Programming error that should never happen?  Guest
> doing something weird?

This particular case I think is "SD card controller model tried
to do something wrong".

> Same for all the other fprintf(stderr, ...) in this file.

Various uses:
 * guest legitimately did something we feel like telling the user
   about (eg "Card force-erased by CMD42")
 * guest did something dubious but with defined semantics
   ("Unknown CMD", trying to do something when the card is locked)
 * guest did something legitimate but unimplemented
 * underlying block layer read/write failed (and we are about
   to throw away the error rather than reporting it anywhere else!)

These would all be worth tidying up some day when we have a
sensible logging infrastructure to tidy them up into.

-- PMM

Re: [Qemu-devel] [PATCH 28/47] qmp: add drive-mirror command

[Paolo Bonzini]

Il 31/07/2012 11:46, Kevin Wolf ha scritto:
>>> I'm not even sure about the QMP mirror command itself.
>>>
>>> I don't really like it, it does too many things at once: It can create
>>> the target image file, it opens the target and it actually starts the
>>> mirroring. It's rather bad at the first two steps, because it doesn't
>>> take any options. This means that it can't create qcow2v3 images, for
>>> example. Or you can't mirror into a backup with cache=unsafe while
>>> running your real VM on cache=writethrough.
>>
>> Yes, though this can be worked around with mode: 'existing'.
> 
> True. Only the problem with image creation, though, not the one with
> bdrv_open() flags, right?

Yeah, but do you really care about for example io=threads vs. io=native?
 The only interesting one is cache=unsafe; the mirror should enable
writeback caching on the target (bdrv_swap will disable it if needed;
I'll change this in the next submission), so cache=writethrough vs.
writeback doesn't matter.

>>> Having an all-in-one mirror command is a nice feature for HMP, but for
>>> QMP it's more like a design problem.
>>>
>>> Now I see you have called it drive-mirror
>>
>> I thought this was your idea. :)
> 
> Hm, then probably we discussed similar things before. :-)
> 
>>> , so that kind of implies that
>>> it's not the final blockdev-mirror but just a QMP version of a command
>>> primarily designed for HMP. As such this restricted functionality may be
>>> acceptable, but it's not like everything is already perfect and there's
>>> no room for discussion.
>>
>> We keep going back to the same point that we do not have -blockdev, but
>> it's becoming a bit frustrating to always rehash this same point...
> 
> The question is whether we need it at all. We do have a drive_add
> if=none, and for creating a mirror target that should actually be enough.

But not for creating images.  That would require qemu-img invocation.

If you're okay with always using an existing image in the QMP case (and
moving image creation to the HMP implementation), we can do it.  But I'm
not sure I like it, I think it's excessive in the other direction.

Paolo

Re: [Qemu-devel] [PATCH] kvm: Check if smp_cpus exceeds max cpus supported by kvm

[Peter Maydell]

On 30 July 2012 19:22,   wrote:
> +static int kvm_max_vcpus(KVMState *s)
> +{
> +int max_vcpus = 4;
> +int ret;
> +ret = kvm_check_extension(s, KVM_CAP_MAX_VCPUS);
> +if (ret) {
> +max_vcpus = ret;
> +} else {
> +   ret = kvm_check_extension(s, KVM_CAP_NR_VCPUS);
> +if (ret) {
> +max_vcpus = ret;
> +}
> +   }
> +
> +return max_vcpus;
> +}

A small thing, but I think having code flow like:

/* Find number of supported CPUs using the recommended
 * procedure from the kernel API documentation to cope with
 * older kernels that may be missing capabilities.
 */
ret = kvm_check_extension(s, KVM_CAP_MAX_VCPUS);
if (ret) {
return ret;
}
ret = kvm_check_extension(s, KVM_CAP_NR_VCPUS);
if (ret) {
return ret;
}
return 4;

would be clearer.

(also I think a comment helps suggest that 4 isn't a magic
number we made up ourselves :-))

-- PMM

[Qemu-devel] [PATCH v2] kvm: Check if smp_cpus exceeds max cpus supported by kvm

[riegamaths]

From: Dunrong Huang 

Add a helper function for fetching max cpus supported by kvm.

Make QEMU exit with an error message if smp_cpus exceeds limit
of VCPU count retrieved by invoking this helper function.

Signed-off-by: Dunrong Huang 
---
v1 -> v2:
   * Fix indentation
 kvm-all.c |   25 +
 1 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index 2148b20..d1f7d72 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -1207,6 +1207,23 @@ static int kvm_irqchip_create(KVMState *s)
 return 0;
 }
 
+static int kvm_max_vcpus(KVMState *s)
+{
+int max_vcpus = 4;
+int ret;
+ret = kvm_check_extension(s, KVM_CAP_MAX_VCPUS);
+if (ret) {
+max_vcpus = ret;
+} else {
+ret = kvm_check_extension(s, KVM_CAP_NR_VCPUS);
+if (ret) {
+max_vcpus = ret;
+}
+}
+
+return max_vcpus;
+}
+
 int kvm_init(void)
 {
 static const char upgrade_note[] =
@@ -1216,6 +1233,7 @@ int kvm_init(void)
 const KVMCapabilityInfo *missing_cap;
 int ret;
 int i;
+int max_vcpus;
 
 s = g_malloc0(sizeof(KVMState));
 
@@ -1256,6 +1274,13 @@ int kvm_init(void)
 goto err;
 }
 
+max_vcpus = kvm_max_vcpus(s);
+if (smp_cpus > max_vcpus) {
+fprintf(stderr, "Number of SMP cpus requested (%d) exceeds max cpus "
+"supported by KVM (%d)\n", smp_cpus, max_vcpus);
+exit(1);
+}
+
 s->vmfd = kvm_ioctl(s, KVM_CREATE_VM, 0);
 if (s->vmfd < 0) {
 #ifdef TARGET_S390X
-- 
1.7.8.6

Re: [Qemu-devel] [PATCH V4 04/12] hw/sd.c: favour SD card type (SDSC or SDHC) when performing erase

[Igor Mitsyanko]

On 07/31/2012 01:29 PM, Markus Armbruster wrote:

Igor Mitsyanko  writes:


Standard capacity cards SDSC use byte unit address while SDHC and SDXC Cards use
block unit address (512 bytes) when setting ERASE_START and ERASE_END with CMD32
and CMD33, we have to account for this.

Signed-off-by: Igor Mitsyanko 
---
  hw/sd.c |6 --
  1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/sd.c b/hw/sd.c
index d0674d5..f7aa580 100644
--- a/hw/sd.c
+++ b/hw/sd.c
@@ -482,8 +482,10 @@ static void sd_erase(SDState *sd)
  return;
  }
  
-start = sd_addr_to_wpnum(sd->erase_start);

-end = sd_addr_to_wpnum(sd->erase_end);
+start = sd_addr_to_wpnum(sd->ocr & (1 << 30) ?
+(uint64_t)sd->erase_start * 512 : sd->erase_start);
+end = sd_addr_to_wpnum(sd->ocr & (1 << 30) ?
+(uint64_t)sd->erase_end * 512 : sd->erase_end);
  sd->erase_start = 0;
  sd->erase_end = 0;
  sd->csd[14] |= 0x40;

Is this a bug fix?

If yes, I recommend to state that clearly in the subject, say "hw/sd.c:
Fix erase for high capacity cards".


Yes it is, I'll change subject in next version then.

[Qemu-devel] [RFC] Factor out fifos / circular buffers

[Peter Crosthwaite]

Hi All,

A lot of devices have little internal fifos that are often implemented
as circular buffers in the device state. Any reason to not factor that
out into a helper module? Was thinkin just a struct defintion
containing the key elements (the uint8_t *data buffer, head/tail
pointers, capacity). and helper functions to create/destroy, put/get,
test for full/empty etc. And VMSD support - which will make devices a
lot cleaner if theres just one VM_STATE_FIFO instead of all these
little bits of implementation detail in the device state.

Regards,
Peter

Re: [Qemu-devel] [PATCH v2] kvm: Check if smp_cpus exceeds max cpus supported by kvm

[Peter Maydell]

On 31 July 2012 11:14,   wrote:
>
> @@ -1256,6 +1274,13 @@ int kvm_init(void)
>  goto err;
>  }
>
> +max_vcpus = kvm_max_vcpus(s);
> +if (smp_cpus > max_vcpus) {
> +fprintf(stderr, "Number of SMP cpus requested (%d) exceeds max cpus "
> +"supported by KVM (%d)\n", smp_cpus, max_vcpus);
> +exit(1);

Don't exit here, just use 'goto err' like all the other checks in
this function. The caller will handle this and exit (or downgrade
to TCG if the user wanted that).

-- PMM

Re: [Qemu-devel] [PATCH 28/47] qmp: add drive-mirror command

[Kevin Wolf]

Am 31.07.2012 12:02, schrieb Paolo Bonzini:
> Il 31/07/2012 11:46, Kevin Wolf ha scritto:
 I'm not even sure about the QMP mirror command itself.

 I don't really like it, it does too many things at once: It can create
 the target image file, it opens the target and it actually starts the
 mirroring. It's rather bad at the first two steps, because it doesn't
 take any options. This means that it can't create qcow2v3 images, for
 example. Or you can't mirror into a backup with cache=unsafe while
 running your real VM on cache=writethrough.
>>>
>>> Yes, though this can be worked around with mode: 'existing'.
>>
>> True. Only the problem with image creation, though, not the one with
>> bdrv_open() flags, right?
> 
> Yeah, but do you really care about for example io=threads vs. io=native?
>  The only interesting one is cache=unsafe; the mirror should enable
> writeback caching on the target (bdrv_swap will disable it if needed;
> I'll change this in the next submission), so cache=writethrough vs.
> writeback doesn't matter.

Can we really make it writeback unconditionally? For a passive mirror it
probably doesn't make a difference, but what happens when the user stops
the mirroring and switches to the target? Will it stay writeback?

The same goes for aio=native/threads. Probably not interesting for the
mirror, but well afterwards.

Another interesting thing is I/O throttling. The mirror currently
implements rate limiting itself, but is there really a reason why we
can't reuse regular I/O throttling on the target?

 Having an all-in-one mirror command is a nice feature for HMP, but for
 QMP it's more like a design problem.

 Now I see you have called it drive-mirror
>>>
>>> I thought this was your idea. :)
>>
>> Hm, then probably we discussed similar things before. :-)
>>
 , so that kind of implies that
 it's not the final blockdev-mirror but just a QMP version of a command
 primarily designed for HMP. As such this restricted functionality may be
 acceptable, but it's not like everything is already perfect and there's
 no room for discussion.
>>>
>>> We keep going back to the same point that we do not have -blockdev, but
>>> it's becoming a bit frustrating to always rehash this same point...
>>
>> The question is whether we need it at all. We do have a drive_add
>> if=none, and for creating a mirror target that should actually be enough.
> 
> But not for creating images.  That would require qemu-img invocation.

Yeah, either qemu-img or another monitor command. I believe that in
practice libvirt will do this anyway if this is the only way to specify
image creation options.

> If you're okay with always using an existing image in the QMP case (and
> moving image creation to the HMP implementation), we can do it.  But I'm
> not sure I like it, I think it's excessive in the other direction.

If you think it's helpful, we could make it optional and have a mode
'blockdev' where you don't specify a file name but a blockdev name. But
this is an approach that feels a bit HMPish...

Kevin

Re: [Qemu-devel] [PATCH V4 08/12] hw/sd.c: add SD card save/load support

[Igor Mitsyanko]

On 07/31/2012 01:33 PM, Markus Armbruster wrote:

Igor Mitsyanko  writes:


This patch updates SD card model to support save/load of card's state.

Signed-off-by: Igor Mitsyanko 
---
  hw/sd.c |   88 +-
  1 files changed, 64 insertions(+), 24 deletions(-)

diff --git a/hw/sd.c b/hw/sd.c
index 20ebd8e..f8ab045 100644
--- a/hw/sd.c
+++ b/hw/sd.c
@@ -55,24 +55,28 @@ typedef enum {
  sd_illegal = -2,
  } sd_rsp_type_t;
  
+enum {

+sd_inactive,
+sd_card_identification_mode,
+sd_data_transfer_mode,
+};
+
+enum {
+sd_inactive_state = -1,
+sd_idle_state = 0,
+sd_ready_state,
+sd_identification_state,
+sd_standby_state,
+sd_transfer_state,
+sd_sendingdata_state,
+sd_receivingdata_state,
+sd_programming_state,
+sd_disconnect_state,
+};
+
  struct SDState {
-enum {
-sd_inactive,
-sd_card_identification_mode,
-sd_data_transfer_mode,
-} mode;
-enum {
-sd_inactive_state = -1,
-sd_idle_state = 0,
-sd_ready_state,
-sd_identification_state,
-sd_standby_state,
-sd_transfer_state,
-sd_sendingdata_state,
-sd_receivingdata_state,
-sd_programming_state,
-sd_disconnect_state,
-} state;
+uint32_t mode;
+int32_t state;

Comment pointing to the related enum?


Ok, I'll have to give them names then.


  uint32_t ocr;
  uint8_t scr[8];
  uint8_t cid[16];

[...]

Re: [Qemu-devel] Tracing drivers commands to an emulated device

[Stefan Hajnoczi]

On Tue, Jul 31, 2012 at 9:43 AM,   wrote:
> I would trace the commands issued by the driver to an emulated device. I've 
> read in docs/tracing that it is possible but it is not completely clear how 
> doing that. Is it possible to place hooks inside the emulated device code? Or 
> is better tracing the in/out operations and the access to the memory mapped 
> zone?

Device-specific trace events can provide you more information like the
device's operating state.

Generic trace events like pio/mmio only tell you the address and data
that the guest is reading/writing.

Which is best depends on what you are doing.  If you want to observe a
specific device I recommend enabling its device-specific trace events
or adding new ones.

If you add trace events and think they may be useful to others, please
submit patches.  For more information, see
http://wiki.qemu.org/Contribute/SubmitAPatch.

Stefan

Re: [Qemu-devel] Cirrus bugs vs endian: how two bugs cancel each other out

[Benjamin Herrenschmidt]

On Tue, 2012-07-31 at 18:44 +1000, ronnie sahlberg wrote:
> 
> I use lots of guests that will never ever get virtio drivers.
> So for those guests, any work on making sure bog standard vga keeps
> working or even improving it
> gets two thumbs up from me!

So I've been essentially restarting my work to make it a virtio-vga...
however, you don't have to use the virtio channel to set modes & do VBE
etc... so it should work like vga std for a dumb x86 guest using
VBE/BIOS.

I'm hoping to have something to show later this week.

I've also done it like the other virtio drivers in that the PCI
interface is split from the core so it can accomodate a non-PCI virtio
base.

The main difference with the other virtio ones is that it has APIs to
return MemoryRegion's for the linear fb and registers, which the
virtio-pci stub hooks up to BARs but that can be hooked up elsewhere
easily.  

The main thing to do right now to have parity of functionality with vga
std is the modified vbe bios, which I'll toy with tomorrow (ugh .. x86
asm :-) From there, I can start doing things with the virtio channel
(right now it exists but I don't have any request implemented).

I'm thinking about a trivial 2D blitter (based on earlier work posted in
2009 by Alex Graf), maybe a HW cursor, see if I can get that used by
SLOF and do a fbdev driver (or rather a drmfb driver).

>From there, I think the right approach is to sync with the folks working
on virtio-qxl so they can use that pipe for spice & we don't re-invent
that wheel.

Cheers,
Ben.

Re: [Qemu-devel] [PATCH v2] kvm: Check if smp_cpus exceeds max cpus supported by kvm

[Dunrong Huang]

2012/7/31 Peter Maydell :
> On 31 July 2012 11:14,   wrote:
>>
>> @@ -1256,6 +1274,13 @@ int kvm_init(void)
>>  goto err;
>>  }
>>
>> +max_vcpus = kvm_max_vcpus(s);
>> +if (smp_cpus > max_vcpus) {
>> +fprintf(stderr, "Number of SMP cpus requested (%d) exceeds max cpus 
>> "
>> +"supported by KVM (%d)\n", smp_cpus, max_vcpus);
>> +exit(1);
>
> Don't exit here, just use 'goto err' like all the other checks in
> this function. The caller will handle this and exit (or downgrade
> to TCG if the user wanted that).
>
Thanks for your review.
I agree with you, I should use "goto err" like other checks.
> -- PMM



-- 
Best Regards,

Dunrong Huang

Re: [Qemu-devel] [PATCH] Makefile: Remove generated hw/usb files in 'clean' target

[Stefan Hajnoczi]

On Mon, Jul 30, 2012 at 5:46 PM, Peter Maydell  wrote:
> On 30 July 2012 17:42, Stefan Hajnoczi  wrote:
>> On Sun, Jul 29, 2012 at 03:48:49PM +0200, Stefan Weil wrote:
>>> Commit f1ae32a1ecda8aaff7a355c9030c0d8c363f3a70 moved the
>>> usb directory to hw, so the files which should be removed
>>> are now hw/usb/*.{d,o}.
>>>
>>> The new code removes these files and also any other generated
>>> *.o and *.d files in directories below hw.
>>>
>>> Signed-off-by: Stefan Weil 
>>> ---
>>>  Makefile |2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/Makefile b/Makefile
>>> index e4c754d..30f9e91 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -215,7 +215,7 @@ clean:
>>>   rm -Rf .libs
>>>   rm -f slirp/*.o slirp/*.d audio/*.o audio/*.d block/*.o block/*.d 
>>> net/*.o net/*.d fsdev/*.o fsdev/*.d ui/*.o ui/*.d qapi/*.o qapi/*.d qga/*.o 
>>> qga/*.d
>>>   rm -f qom/*.o qom/*.d
>>> - rm -f usb/*.o usb/*.d hw/*.o hw/*.d
>>> + rm -f hw/*/*.o hw/*/*.d hw/*.o hw/*.d
>>>   rm -f qemu-img-cmds.h
>>>   rm -f trace/*.o trace/*.d
>>>   rm -f trace-dtrace.dtrace trace-dtrace.dtrace-timestamp
>>> --
>>> 1.7.10
>>
>> Thanks.  Jan Kiszka already submitted an equivalent patch which I have
>> merged.  He used hw/usb/*.o hw/usb/*.d instead of hw/*/
>
> We should probably just use
> rm -f $(shell find . -name '*.[od]')
>
> for consistency with Makefile.target and future proofing anyway...

Cool, want to submit a patch? :)

Stefan

Re: [Qemu-devel] [PATCH 28/47] qmp: add drive-mirror command

[Paolo Bonzini]

Il 31/07/2012 12:25, Kevin Wolf ha scritto:
>> Yeah, but do you really care about for example io=threads vs. io=native?
>>  The only interesting one is cache=unsafe; the mirror should enable
>> writeback caching on the target (bdrv_swap will disable it if needed;
>> I'll change this in the next submission), so cache=writethrough vs.
>> writeback doesn't matter.
> 
> Can we really make it writeback unconditionally? For a passive mirror it
> probably doesn't make a difference, but what happens when the user stops
> the mirroring and switches to the target? Will it stay writeback?

bdrv_swap takes care of it just fine.

> The same goes for aio=native/threads. Probably not interesting for the
> mirror, but well afterwards.

Actually it is interesting for the mirror.  Passive mirroring can only
benefit from lower latency.

But yes, bdrv_swap would not copy this one.  Right now we always use the
same aio method as the source (at worst it is ignored by the protocol),
so it is not a problem.

> Another interesting thing is I/O throttling. The mirror currently
> implements rate limiting itself, but is there really a reason why we
> can't reuse regular I/O throttling on the target?

I thought about it, but I'm worried of what happens when I/O throttling
kicks in, and how it interacts with pause/resume/cancel.

> Having an all-in-one mirror command is a nice feature for HMP, but for
> QMP it's more like a design problem.
>
> Now I see you have called it drive-mirror

 I thought this was your idea. :)
>>>
>>> Hm, then probably we discussed similar things before. :-)
>>>
> , so that kind of implies that
> it's not the final blockdev-mirror but just a QMP version of a command
> primarily designed for HMP. As such this restricted functionality may be
> acceptable, but it's not like everything is already perfect and there's
> no room for discussion.

 We keep going back to the same point that we do not have -blockdev, but
 it's becoming a bit frustrating to always rehash this same point...
>>>
>>> The question is whether we need it at all. We do have a drive_add
>>> if=none, and for creating a mirror target that should actually be enough.
>>
>> But not for creating images.  That would require qemu-img invocation.
> 
> Yeah, either qemu-img or another monitor command. I believe that in
> practice libvirt will do this anyway if this is the only way to specify
> image creation options.

Playing devil's advocate because you've almost convinced me, we have the
same problem for blockdev-snapshot-sync.  Now drive-mirror is a bit
different because you can use it to "reshape" an image to something
else, but the same could be done with snapshot + streaming in many cases.

>> If you're okay with always using an existing image in the QMP case (and
>> moving image creation to the HMP implementation), we can do it.  But I'm
>> not sure I like it, I think it's excessive in the other direction.
> 
> If you think it's helpful, we could make it optional and have a mode
> 'blockdev' where you don't specify a file name but a blockdev name. But
> this is an approach that feels a bit HMPish...

I think having a few limited knobs for image creation make some sense
(not all QMP clients need to be as sophisticated as libvirt), but that's
actually an interesting idea (as it is in general to piggyback on
drive_add).

Still, it leaves something to be desired.  It's not that it feels
HMP-ish, it's that it's overloading target a bit too much.  I would
prefer to keep drive-mirror for simple clients, and have a separate
blockdev-mirror that must have a blockdev target.  But doing the same
with blockdev-snapshot-sync will always look like duct-tape, because the
blockdev name is already taken. :(  Man, sometimes it feels like we're
not getting one thing right.

Paolo

Re: [Qemu-devel] [PATCH 28/47] qmp: add drive-mirror command

[Kevin Wolf]

Am 31.07.2012 12:51, schrieb Paolo Bonzini:
> Il 31/07/2012 12:25, Kevin Wolf ha scritto:
>>> Yeah, but do you really care about for example io=threads vs. io=native?
>>>  The only interesting one is cache=unsafe; the mirror should enable
>>> writeback caching on the target (bdrv_swap will disable it if needed;
>>> I'll change this in the next submission), so cache=writethrough vs.
>>> writeback doesn't matter.
>>
>> Can we really make it writeback unconditionally? For a passive mirror it
>> probably doesn't make a difference, but what happens when the user stops
>> the mirroring and switches to the target? Will it stay writeback?
> 
> bdrv_swap takes care of it just fine.

Ah, the switch uses bdrv_swap? Then that one is fine indeed.

>> The same goes for aio=native/threads. Probably not interesting for the
>> mirror, but well afterwards.
> 
> Actually it is interesting for the mirror.  Passive mirroring can only
> benefit from lower latency.
> 
> But yes, bdrv_swap would not copy this one.  Right now we always use the
> same aio method as the source (at worst it is ignored by the protocol),
> so it is not a problem.

Fair enough, though there may be cases where you'd really want to
switch, like migrating from a block device to a file on NFS.

>> Another interesting thing is I/O throttling. The mirror currently
>> implements rate limiting itself, but is there really a reason why we
>> can't reuse regular I/O throttling on the target?
> 
> I thought about it, but I'm worried of what happens when I/O throttling
> kicks in, and how it interacts with pause/resume/cancel.

bdrv_co_write won't return until the request has been throttled, so it
should be mostly transparent. The effect that it could have is that
pausing the mirror could take a little bit longer to complete (though
not much, as there is only one mirror request at the same time). But
iirc, pausing a block job was async anyway.

Any other aspect I'm missing?

>> Having an all-in-one mirror command is a nice feature for HMP, but for
>> QMP it's more like a design problem.
>>
>> Now I see you have called it drive-mirror
>
> I thought this was your idea. :)

 Hm, then probably we discussed similar things before. :-)

>> , so that kind of implies that
>> it's not the final blockdev-mirror but just a QMP version of a command
>> primarily designed for HMP. As such this restricted functionality may be
>> acceptable, but it's not like everything is already perfect and there's
>> no room for discussion.
>
> We keep going back to the same point that we do not have -blockdev, but
> it's becoming a bit frustrating to always rehash this same point...

 The question is whether we need it at all. We do have a drive_add
 if=none, and for creating a mirror target that should actually be enough.
>>>
>>> But not for creating images.  That would require qemu-img invocation.
>>
>> Yeah, either qemu-img or another monitor command. I believe that in
>> practice libvirt will do this anyway if this is the only way to specify
>> image creation options.
> 
> Playing devil's advocate because you've almost convinced me, we have the
> same problem for blockdev-snapshot-sync.  Now drive-mirror is a bit
> different because you can use it to "reshape" an image to something
> else, but the same could be done with snapshot + streaming in many cases.

Yes, blockdev-snapshot-sync is more or less the same case. We were aware
from the beginning that it's not the right command, but apparently
didn't think of drive_add.

>>> If you're okay with always using an existing image in the QMP case (and
>>> moving image creation to the HMP implementation), we can do it.  But I'm
>>> not sure I like it, I think it's excessive in the other direction.
>>
>> If you think it's helpful, we could make it optional and have a mode
>> 'blockdev' where you don't specify a file name but a blockdev name. But
>> this is an approach that feels a bit HMPish...
> 
> I think having a few limited knobs for image creation make some sense
> (not all QMP clients need to be as sophisticated as libvirt), but that's
> actually an interesting idea (as it is in general to piggyback on
> drive_add).
> 
> Still, it leaves something to be desired.  It's not that it feels
> HMP-ish, it's that it's overloading target a bit too much.  I would
> prefer to keep drive-mirror for simple clients, and have a separate
> blockdev-mirror that must have a blockdev target.  But doing the same
> with blockdev-snapshot-sync will always look like duct-tape, because the
> blockdev name is already taken. :(  Man, sometimes it feels like we're
> not getting one thing right.

blockdev-snapshot isn't taken yet. However, having the two side by side
would imply that blockdev-snapshot is async, which I believe is
currently not the most urgent of our concerns...

Or actually, it might not even matter any more, because the thing that
really takes some time is creating and ope

[Qemu-devel] [PATCH v3] kvm: Check if smp_cpus exceeds max cpus supported by kvm

[riegamaths]

From: Dunrong Huang 

Add a helper function for fetching max cpus supported by kvm.

Make QEMU exit with an error message if smp_cpus exceeds limit
of VCPU count retrieved by invoking this helper function.

Signed-off-by: Dunrong Huang 
---
v1 -> v2:
   * Fix indentation(thanks to Stefan Hajnoczi for his review)
   
v2 -> v3(Thanks to Peter Maydell for this hint)
   * Add a comment for helper function
   * Use "goto err" instead of "exit(1)"
   
 kvm-all.c |   29 +
 1 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index 2148b20..bf64761 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -1207,6 +1207,26 @@ static int kvm_irqchip_create(KVMState *s)
 return 0;
 }
 
+static int kvm_max_vcpus(KVMState *s)
+{
+int ret;
+
+/* Find number of supported CPUs using the recommended
+ * procedure from the kernel API documentation to cope with
+ * older kernels that may be missing capabilities.
+ */
+ret = kvm_check_extension(s, KVM_CAP_MAX_VCPUS);
+if (ret) {
+return ret;
+}
+ret = kvm_check_extension(s, KVM_CAP_NR_VCPUS);
+if (ret) {
+return ret;
+}
+
+return 4;
+}
+
 int kvm_init(void)
 {
 static const char upgrade_note[] =
@@ -1216,6 +1236,7 @@ int kvm_init(void)
 const KVMCapabilityInfo *missing_cap;
 int ret;
 int i;
+int max_vcpus;
 
 s = g_malloc0(sizeof(KVMState));
 
@@ -1256,6 +1277,14 @@ int kvm_init(void)
 goto err;
 }
 
+max_vcpus = kvm_max_vcpus(s);
+if (smp_cpus > max_vcpus) {
+ret = -EINVAL;
+fprintf(stderr, "Number of SMP cpus requested (%d) exceeds max cpus "
+"supported by KVM (%d)\n", smp_cpus, max_vcpus);
+goto err;
+}
+
 s->vmfd = kvm_ioctl(s, KVM_CREATE_VM, 0);
 if (s->vmfd < 0) {
 #ifdef TARGET_S390X
-- 
1.7.8.6

Re: [Qemu-devel] [PATCH 28/47] qmp: add drive-mirror command

[Paolo Bonzini]

Il 31/07/2012 13:13, Kevin Wolf ha scritto:
> Am 31.07.2012 12:51, schrieb Paolo Bonzini:
>> Il 31/07/2012 12:25, Kevin Wolf ha scritto:
 Yeah, but do you really care about for example io=threads vs. io=native?
  The only interesting one is cache=unsafe; the mirror should enable
 writeback caching on the target (bdrv_swap will disable it if needed;
 I'll change this in the next submission), so cache=writethrough vs.
 writeback doesn't matter.
>>>
>>> Can we really make it writeback unconditionally? For a passive mirror it
>>> probably doesn't make a difference, but what happens when the user stops
>>> the mirroring and switches to the target? Will it stay writeback?
>>
>> bdrv_swap takes care of it just fine.
> 
> Ah, the switch uses bdrv_swap? Then that one is fine indeed.
> 
>>> The same goes for aio=native/threads. Probably not interesting for the
>>> mirror, but well afterwards.
>>
>> Actually it is interesting for the mirror.  Passive mirroring can only
>> benefit from lower latency.
>>
>> But yes, bdrv_swap would not copy this one.  Right now we always use the
>> same aio method as the source (at worst it is ignored by the protocol),
>> so it is not a problem.
> 
> Fair enough, though there may be cases where you'd really want to
> switch, like migrating from a block device to a file on NFS.
> 
>>> Another interesting thing is I/O throttling. The mirror currently
>>> implements rate limiting itself, but is there really a reason why we
>>> can't reuse regular I/O throttling on the target?
>>
>> I thought about it, but I'm worried of what happens when I/O throttling
>> kicks in, and how it interacts with pause/resume/cancel.
> 
> bdrv_co_write won't return until the request has been throttled, so it
> should be mostly transparent.

At the end of this series I use bdrv_aio_readv/writev.

> The effect that it could have is that
> pausing the mirror could take a little bit longer to complete (though
> not much, as there is only one mirror request at the same time).

Not anymore. :)

> But iirc, pausing a block job was async anyway.

Yes, it is, and job->busy nicely abstracts the hairy parts.

> Any other aspect I'm missing?

No, that should be ok.  Though I'm not sure if it's so useful to apply
throttling on the target.  It's more useful to throttle the source
(making writes slower than reads will help the job's convergence) and
copy at full steam to the target.

 If you're okay with always using an existing image in the QMP case (and
 moving image creation to the HMP implementation), we can do it.  But I'm
 not sure I like it, I think it's excessive in the other direction.
>>>
>>> If you think it's helpful, we could make it optional and have a mode
>>> 'blockdev' where you don't specify a file name but a blockdev name. But
>>> this is an approach that feels a bit HMPish...
>>
>> I think having a few limited knobs for image creation make some sense
>> (not all QMP clients need to be as sophisticated as libvirt), but that's
>> actually an interesting idea (as it is in general to piggyback on
>> drive_add).
>>
>> Still, it leaves something to be desired.  It's not that it feels
>> HMP-ish, it's that it's overloading target a bit too much.  I would
>> prefer to keep drive-mirror for simple clients, and have a separate
>> blockdev-mirror that must have a blockdev target.  But doing the same
>> with blockdev-snapshot-sync will always look like duct-tape, because the
>> blockdev name is already taken. :(  Man, sometimes it feels like we're
>> not getting one thing right.
> 
> blockdev-snapshot isn't taken yet. However, having the two side by side
> would imply that blockdev-snapshot is async, which I believe is
> currently not the most urgent of our concerns...
> 
> Or actually, it might not even matter any more, because the thing that
> really takes some time is creating and opening the image. Once you have
> the blockdev, there's no point in making things async any more.

Right, blockdev-snapshot would really be just a bdrv_append operation.
/me smiles. :)  So let's keep drive-mirror as is, and later add
blockdev-mirror.

Paolo

Re: [Qemu-devel] [PATCH 0/2] net: Make -netdev socket,listen= work

[Laszlo Ersek]

(in reply to
 --
In-Reply-To set, but that may not be enough for the web archive)

Looks good to me. A minor nit: 2/2 keeps the close(s->fd) call (instead
of calling closesocket(s->fd), like in eoc handling) in
net_socket_cleanup().

Reviewed-by: Laszlo Ersek 

Re: [Qemu-devel] 9p broken?

[Avi Kivity]

On 07/31/2012 09:51 AM, Aneesh Kumar K.V wrote:
> Avi Kivity  writes:
> 
>> Having an annoying bug on i386 kvm I decided to debug it buy running an
>> i386 guest on my x86_64 host, use 9p to access a guest image, and run it
>> using nested kvm.
>>
>> However, 9p appears to be broken: first, the configure test fails (patch
>> sent).  Second, while mount works, ls on the mount point causes qemu to
>> crash with
> 
> I missed that you have already sent a patch for configure fix. That
> looks better that what i sent. I will ack that patch 
> 
>>
>> (gdb) bt
>> #0  error_set (errp=0x7fffe95fb128, fmt=0x558d4568 "{ 'class':
>> 'VirtFSFeatureBlocksMigration', 'data': { 'path': %s, 'tag': %s } }") at
>> /home/tlv/akivity/qemu/error.c:32
>> #1  0x5567cb06 in v9fs_attach (opaque=0x7fffe95e3020) at
>> /home/tlv/akivity/qemu/hw/9pfs/virtio-9p.c:988
>> #2  0x5561d19f in coroutine_trampoline (i0=1449767888, i1=21845)
>> at /home/tlv/akivity/qemu/coroutine-ucontext.c:138
>> #3  0x75a93ef0 in ?? ()  from /lib64/libc.so.6
>> #4  0x7fffce00 in ?? ()
>> #5  0x in ?? (
>>
>> **errp already points to a VirtFSFeatureBlocksMigration error;
>> v9fs_attach() has been called a second time (the first time,
>> understandably, on mount; the second on ls).
>>
> 
> Why are we calling attach a second time ?. I am also not able to reproduce 
> this
> 
> root@qemu-img-64:~# mount -t 9p -otrans=virtio,version=9p2000.L v_tmp /mnt
> root@qemu-img-64:~# ls /mnt/a.c
> /mnt/a.c
> 

I'm just doing ls /mnt (even tab completion: ls /mn crashes qemu).

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 28/47] qmp: add drive-mirror command

[Kevin Wolf]

Am 31.07.2012 13:25, schrieb Paolo Bonzini:
> Il 31/07/2012 13:13, Kevin Wolf ha scritto:
>> Am 31.07.2012 12:51, schrieb Paolo Bonzini:
>>> Il 31/07/2012 12:25, Kevin Wolf ha scritto:
 Another interesting thing is I/O throttling. The mirror currently
 implements rate limiting itself, but is there really a reason why we
 can't reuse regular I/O throttling on the target?
>>>
>>> I thought about it, but I'm worried of what happens when I/O throttling
>>> kicks in, and how it interacts with pause/resume/cancel.
>>
>> bdrv_co_write won't return until the request has been throttled, so it
>> should be mostly transparent.
> 
> At the end of this series I use bdrv_aio_readv/writev.
> 
>> The effect that it could have is that
>> pausing the mirror could take a little bit longer to complete (though
>> not much, as there is only one mirror request at the same time).
> 
> Not anymore. :)

Hm, I see. Makes it a bit more involved, but then the logic should be
almost the same as you already need to complete the mirror.

>> But iirc, pausing a block job was async anyway.
> 
> Yes, it is, and job->busy nicely abstracts the hairy parts.
> 
>> Any other aspect I'm missing?
> 
> No, that should be ok.  Though I'm not sure if it's so useful to apply
> throttling on the target.  It's more useful to throttle the source
> (making writes slower than reads will help the job's convergence) and
> copy at full steam to the target.

But doesn't the rate limiting of the mirror already throttle the target?
Which isn't too bad, because I think at least in the initial phase
you'll want to have both source and target throttled (later the target
is automatically throttled to the level of the source, except for bitmap
granularity artefacts).

> If you're okay with always using an existing image in the QMP case (and
> moving image creation to the HMP implementation), we can do it.  But I'm
> not sure I like it, I think it's excessive in the other direction.

 If you think it's helpful, we could make it optional and have a mode
 'blockdev' where you don't specify a file name but a blockdev name. But
 this is an approach that feels a bit HMPish...
>>>
>>> I think having a few limited knobs for image creation make some sense
>>> (not all QMP clients need to be as sophisticated as libvirt), but that's
>>> actually an interesting idea (as it is in general to piggyback on
>>> drive_add).
>>>
>>> Still, it leaves something to be desired.  It's not that it feels
>>> HMP-ish, it's that it's overloading target a bit too much.  I would
>>> prefer to keep drive-mirror for simple clients, and have a separate
>>> blockdev-mirror that must have a blockdev target.  But doing the same
>>> with blockdev-snapshot-sync will always look like duct-tape, because the
>>> blockdev name is already taken. :(  Man, sometimes it feels like we're
>>> not getting one thing right.
>>
>> blockdev-snapshot isn't taken yet. However, having the two side by side
>> would imply that blockdev-snapshot is async, which I believe is
>> currently not the most urgent of our concerns...
>>
>> Or actually, it might not even matter any more, because the thing that
>> really takes some time is creating and opening the image. Once you have
>> the blockdev, there's no point in making things async any more.
> 
> Right, blockdev-snapshot would really be just a bdrv_append operation.
> /me smiles. :)  So let's keep drive-mirror as is, and later add
> blockdev-mirror.

Ok, that's fair enough.

Kevin

Re: [Qemu-devel] [PATCH V4 11/12] SD card: introduce "spi" property for SD card objects

[Andreas Färber]

Am 31.07.2012 11:54, schrieb Markus Armbruster:
> Igor Mitsyanko  writes:
> 
>> And drop passing is_spi argument to SDCardClass::init function.
>> "spi" property could be set only while SD card object is not
>> attached to any BlockDriverState.
>> It defaults to "false".
>>
>> Signed-off-by: Igor Mitsyanko 
>> Cc: Paul Brook 
>> ---
>>  hw/sd.c |   33 +++--
>>  hw/sd.h |8 ++--
>>  2 files changed, 37 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/sd.c b/hw/sd.c
>> index fe2c138..611e1f3 100644
>> --- a/hw/sd.c
>> +++ b/hw/sd.c
>> @@ -491,10 +491,9 @@ static const VMStateDescription sd_vmstate = {
>> whether card should be in SSI or MMC/SD mode.  It is also up to the
>> board to ensure that ssi transfers only occur when the chip select
>> is asserted.  */
>> -static void sd_init_card(SDState *sd, BlockDriverState *bs, bool is_spi)
>> +static void sd_init_card(SDState *sd, BlockDriverState *bs)
>>  {
>>  sd->buf = qemu_blockalign(bs, 512);
>> -sd->spi = is_spi;
>>  sd->enable = true;
>>  sd_reset(sd, bs);
>>  if (sd->bdrv) {
>> @@ -1766,10 +1765,40 @@ static void sd_class_init(ObjectClass *klass, void 
>> *data)
>>  k->enable = sd_enable_card;
>>  }
>>  
>> +static void sd_is_spi(Object *obj, Visitor *v, void *opaque,
>> + const char *name, Error **errp)
>> +{
>> +SDState *sd = SD_CARD(obj);
>> +
>> +visit_type_bool(v, &sd->spi, name, errp);
>> +}
>> +
>> +static void sd_set_spimode(Object *obj, Visitor *v, void *opaque,
>> + const char *name, Error **errp)
>> +{
>> +SDState *sd = SD_CARD(obj);
>> +
>> +if (sd->bdrv) {
>> +error_set(errp, QERR_DEVICE_IN_USE, bdrv_get_device_name(sd->bdrv));
>> +} else {
>> +visit_type_bool(v, &sd->spi, name, errp);
>> +}
>> +}
>> +
>> +static void sd_initfn(Object *obj)
>> +{
>> +SDState *sd = SD_CARD(obj);
>> +
>> +sd->spi = false;
>> +object_property_add(obj, "spi", "boolean", sd_is_spi, sd_set_spimode,
>> +NULL, NULL, NULL);
>> +}
>> +
>>  static const TypeInfo sd_type_info = {
>>  .name = TYPE_SD_CARD,
>>  .parent = TYPE_OBJECT,
>>  .instance_size = sizeof(SDState),
>> +.instance_init = sd_initfn,
>>  .class_init = sd_class_init,
>>  .class_size = sizeof(SDClass)
>>  };
> 
> I suspect this would be much simpler the declarative way qdevs normally
> use.  For an example, check out scsi_hd_properties[] and its use in
> hw/scsi-disk.c.
[snip]

For static properties bool support was missing some time ago...

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [RFC PATCH] vfio: VFIO PCI driver for Qemu

[Avi Kivity]

On 07/31/2012 01:29 AM, Alex Williamson wrote:
>> 
>> If the region size is zero, then both memory_region_del_subregion()
>> (assuming the region is parented) and munmap() do nothing.  So you could
>> call this unconditionally.
> 
> I suppose parenting them is the key.  I'm counting on memory_region_size
> of zero for an uninitialized, g_malloc0() MemoryRegion.

That's a no-no.  We have APIs for a reason.  Maybe I'll start encrypting
the contents by xoring with a private variable.

>  Initializing
> them just to have a parent so we can unconditionally remove them here
> seems like it's just shifting complexity from one function to another.
> The majority of BARs aren't even implemented, so we'd actually be
> setting up a lot of dummy infrastructure for a slightly cleaner unmap
> function.  I'll keep looking at this, but I'm not optimistic there's an
> overall simplification here.

Ok.  But use your own bool, don't overload an something from MemoryRegion.


>  
>> >> > +
>> >> > +if (vdev->msix && vdev->msix->table_bar == nr) {
>> >> > +size = memory_region_size(&vdev->msix->mmap_mem);
>> >> > +if (size) {
>> >> > +memory_region_del_subregion(&bar->mem, 
>> >> > &vdev->msix->mmap_mem);
>> >> > +munmap(vdev->msix->mmap, size);
>> >> > +}
>> >> > +}
>> > 
>> > And this one potentially unmaps the overlap after the vector table if
>> > there's any space for one.
>> > 
>> >> Are the three size checks needed? Everything should work without them
>> >> from the memory core point of view.
>> > 
>> > I haven't tried, but I strongly suspect I shouldn't be munmap'ing
>> > NULL... no?
>> 
>> NULL isn't the problem (well some kernels protect against mmaping NULL
>> to avoid kernel exploits), but it seems the kernel doesn't like a zero
>> length.
> 
> in mm/mmap.c:do_munmap() I see:
> 
> if ((len = PAGE_ALIGN(len)) == 0)
> return -EINVAL;
> 
> Before anything scary happens, so that should be ok.  It's not really
> worthwhile to call the munmaps unconditionally if we already have the
> condition tests because the subregions are unparented though.

Yeah.

> 
>> >> > +
>> >> > +/*
>> >> > + * We can't mmap areas overlapping the MSIX vector table, so we
>> >> > + * potentially insert a direct-mapped subregion before and after 
>> >> > it.
>> >> > + */
>> >> 
>> >> This splitting is what the memory core really enjoys.  You can just
>> >> place the MSIX page over the RAM page and let it do the cut-n-paste.
>> > 
>> > Sure, but VFIO won't allow us to mmap over the MSI-X table for security
>> > reasons.  It might be worthwhile to someday make VFIO insert an
>> > anonymous page over the MSI-X table to allow this, but it didn't look
>> > trivial for my novice mm abilities.  Easy to add a flag from the VFIO
>> > kernel structure where we learn about this BAR if we add it in the
>> > future.
>> 
>> I meant due it purely in qemu.  Instead of an emulated region overlaid
>> by two assigned regions, have an assigned region overlaid by the
>> emulated region.  The regions seen by the vfio listener will be the same.
> 
> Sure, that's what KVM device assignment does, but it requires being able
> to mmap the whole BAR, including an MSI-X table.  The VFIO kernel side
> can't assume userspace isn't malicious so it has to prevent this.

I wonder whether it should prevent the mmap(), or let it though and just
SIGBUS on accesses.

>> > 
>> > This is actually kind of complicated.  Opening /dev/vfio/vfio gives us
>> > an instance of a container in the kernel.  A group can only be attached
>> > to one container.  So whoever calls us with passed fds needs to track
>> > this very carefully.  This is also why I've dropped any kind of shared
>> > IOMMU option to give us a hint whether to try to cram everything in the
>> > same container (~= iommu domain).  It's too easy to pass conflicting
>> > info to share a container for one device, but not another... yet they
>> > may be in the same group.  I'll work on the fd passing though and try to
>> > come up with a reasonable model.
>> 
>> I didn't really follow the container stuff so I can't comment here.  But
>> suppose all assigned devices are done via fd passing, isn't it
>> sufficient to just pass the fd for the device (and keep the iommu group
>> fd in the managment tool)?
> 
> Nope.
> 
> containerfd = open(/dev/vfio/vfio)
> groupfd = open(/dev/vfio/$GROUPID)
> devicefd  = ioctl(groupfd, VFIO_GROUP_GET_DEVICE_FD)
> 
> The container provides access to the iommu, the group is the unit of
> ownership and privilege, and device cannot be accessed without iommu
> protection.  Therefore to get to a devicefd, we first need to privilege
> the container by attaching a group to it, that let's us initialize the
> iommu, which allows us to get the device fd.  At a minimum, we'd need
> both container and device fds, which means libvirt would be responsible
> for determining what type of iommu interface to initialize.  Doin

Re: [Qemu-devel] [PATCH v3 0/2] GlusterFS support in QEMU - v3

[Bharata B Rao]

On Tue, Jul 31, 2012 at 09:12:53AM +0100, Stefan Hajnoczi wrote:
> On Wed, Jul 25, 2012 at 6:58 AM, Bharata B Rao
>  wrote:
> > Apart from cleanups, the major change in this version is to expose all
> > the gluster configuration options to QEMU user. With this, the gluster
> > specification looks like this:
> >
> > -drive file=gluster:server:[port]:[transport]:volname:image
> >
> > - Here 'gluster' is the protocol.
> > - 'server' specifies the server where the volume file specification for
> >   the given volume resides.
> 
> Works fine for hostnames and IPv4 addresses.  It seems like the ':'
> separator may prevent users from giving IPv6 addresses unless there is
> a way to escape ':'.

I feel its better to go the URI way than to escape ':'. I will have this
specification in v4:

gluster://server:[port]/volname/path/to/image?transport=socket

As per http://tools.ietf.org/html/rfc3986#section-3.2.2, ipv6 addresses
are specified within square brackets in a URI which makes it easy to parse
the port after :

Regards,
Bharata.

[Qemu-devel] R: Re: Tracing drivers commands to an emulated device

[ffdevel]

Hi Stefan,
thanks for the support i think that the better way for me is to combine the two 
approaches. At this time i want start profiling the disk and network driver 
activity. I will submit patches in the case that the trace events that i add 
can be useful for others.

Francesco 
--Messaggio originale--
Da: Stefan Hajnoczi
A: ffde...@gmail.com
Cc: qemu-devel@nongnu.org
Oggetto: Re: [Qemu-devel] Tracing drivers commands to an emulated device
Inviato: 31 lug 2012 12:29 pm

On Tue, Jul 31, 2012 at 9:43 AM,   wrote:
> I would trace the commands issued by the driver to an emulated device. I've 
> read in docs/tracing that it is possible but it is not completely clear how 
> doing that. Is it possible to place hooks inside the emulated device code? Or 
> is better tracing the in/out operations and the access to the memory mapped 
> zone?

Device-specific trace events can provide you more information like the
device's operating state.

Generic trace events like pio/mmio only tell you the address and data
that the guest is reading/writing.

Which is best depends on what you are doing.  If you want to observe a
specific device I recommend enabling its device-specific trace events
or adding new ones.

If you add trace events and think they may be useful to others, please
submit patches.  For more information, see
http://wiki.qemu.org/Contribute/SubmitAPatch.

Stefan

Inviato dal mio smartphone BlackBerry® 
www.blackberry.com

Re: [Qemu-devel] [PATCH v4 07/07] s390: make sclp ascii console the default

[Christian Borntraeger]

On 30/07/12 16:05, Alexander Graf wrote:
> 
> On 26.07.2012, at 10:59, Christian Borntraeger wrote:
> 
>> This patch makes the sclp ascii default for S390. It requires a guest
>> kernel that autodetects the console and which not blindly assumes
>> that kvm means virtio console.
>> (commit cd1834591fe9564720ac4b0193bf1c790fe89f0d
>>KVM: s390: Perform early event mask processing during boot)
>> Otherwise the guest admin has to explicitely add console=ttyS1 to the
>> command line.
> 
> Hrm. Could we make this the non-default for the time being with a simple 
> -machine option to switch it to the default? When enough kernels are aware of 
> the switch, we could then make it the default.

No problem, we can defer that patch. It was merely a testing aid for those who 
want to test the
sclp code. Later on maybe we can make that decision based on other thing.

Re: [Qemu-devel] [PATCH 28/47] qmp: add drive-mirror command

[Paolo Bonzini]

Il 31/07/2012 14:17, Kevin Wolf ha scritto:
>> No, that should be ok.  Though I'm not sure if it's so useful to apply
>> throttling on the target.  It's more useful to throttle the source
>> (making writes slower than reads will help the job's convergence) and
>> copy at full steam to the target.
> 
> But doesn't the rate limiting of the mirror already throttle the target?

Of course whatever you throttle (any of job, source, target) will have
an effect on the other two as well.

IMO, the target is perhaps the least useful to throttle.  It is more
interesting to play with the source, because that's guest visible.
Slowing down the target, while letting the guest run at full speed is
unlikely to help convergence of the job.

On the other hand, the job and target speeds are really duplicates of
each other, so the job speed is really just as useless.

So it sounds like removing the job speed is a good idea.  If needed,
libvirt can implement it later with a named block device for the target
+ I/O throttling.

> Which isn't too bad, because I think at least in the initial phase
> you'll want to have both source and target throttled (later the target
> is automatically throttled to the level of the source, except for bitmap
> granularity artefacts).

The target is always throttled to the level of the source and vice
versa.  The target can never be written faster than you read the source;
and slowing down the target will keep buffers busy so you cannot read
more from the source.

Paolo

Re: [Qemu-devel] [PATCH v4 07/07] s390: make sclp ascii console the default

[Alexander Graf]

On 07/31/2012 02:44 PM, Christian Borntraeger wrote:

On 30/07/12 16:05, Alexander Graf wrote:

On 26.07.2012, at 10:59, Christian Borntraeger wrote:


This patch makes the sclp ascii default for S390. It requires a guest
kernel that autodetects the console and which not blindly assumes
that kvm means virtio console.
(commit cd1834591fe9564720ac4b0193bf1c790fe89f0d
KVM: s390: Perform early event mask processing during boot)
Otherwise the guest admin has to explicitely add console=ttyS1 to the
command line.

Hrm. Could we make this the non-default for the time being with a simple 
-machine option to switch it to the default? When enough kernels are aware of 
the switch, we could then make it the default.

No problem, we can defer that patch. It was merely a testing aid for those who 
want to test the
sclp code. Later on maybe we can make that decision based on other thing.


Well, I'd like to have the logic around and easy to choose. Basically 
what I envision is that one day we do a flag day that introduces a 
compatibility machine type which only differs from the current one in 
that it sets said machine opt to "use ascii console as default".


That way for the time being, people who want to play with it only need 
to do -machine ascii_console=on and everything else would happen 
automatically. It'd definitely make it easier for everyone involved.



Alex

Re: [Qemu-devel] [PATCH V4 11/12] SD card: introduce "spi" property for SD card objects

[Paolo Bonzini]

Il 31/07/2012 14:19, Andreas Färber ha scritto:
>>> >> +sd->spi = false;
>>> >> +object_property_add(obj, "spi", "boolean", sd_is_spi, 
>>> >> sd_set_spimode,
>>> >> +NULL, NULL, NULL);
>>> >> +}
>>> >> +
>>> >>  static const TypeInfo sd_type_info = {
>>> >>  .name = TYPE_SD_CARD,
>>> >>  .parent = TYPE_OBJECT,
>>> >>  .instance_size = sizeof(SDState),
>>> >> +.instance_init = sd_initfn,
>>> >>  .class_init = sd_class_init,
>>> >>  .class_size = sizeof(SDClass)
>>> >>  };
>> > 
>> > I suspect this would be much simpler the declarative way qdevs normally
>> > use.  For an example, check out scsi_hd_properties[] and its use in
>> > hw/scsi-disk.c.
> [snip]
> 
> For static properties bool support was missing some time ago...

There are bitfields, which are really the same thing except they expect
an u32 field.

Paolo

Re: [Qemu-devel] [PATCH v3 0/2] GlusterFS support in QEMU - v3

[Stefan Hajnoczi]

On Tue, Jul 31, 2012 at 1:38 PM, Bharata B Rao
 wrote:
> On Tue, Jul 31, 2012 at 09:12:53AM +0100, Stefan Hajnoczi wrote:
>> On Wed, Jul 25, 2012 at 6:58 AM, Bharata B Rao
>>  wrote:
>> > Apart from cleanups, the major change in this version is to expose all
>> > the gluster configuration options to QEMU user. With this, the gluster
>> > specification looks like this:
>> >
>> > -drive file=gluster:server:[port]:[transport]:volname:image
>> >
>> > - Here 'gluster' is the protocol.
>> > - 'server' specifies the server where the volume file specification for
>> >   the given volume resides.
>>
>> Works fine for hostnames and IPv4 addresses.  It seems like the ':'
>> separator may prevent users from giving IPv6 addresses unless there is
>> a way to escape ':'.
>
> I feel its better to go the URI way than to escape ':'. I will have this
> specification in v4:
>
> gluster://server:[port]/volname/path/to/image?transport=socket
>
> As per http://tools.ietf.org/html/rfc3986#section-3.2.2, ipv6 addresses
> are specified within square brackets in a URI which makes it easy to parse
> the port after :

Okay.  Did you check that volnames cannot contain '/'?

Stefan

Re: [Qemu-devel] [PATCH 3/5] qapi: avoid reserved word restrict

[Luiz Capitulino]

On Tue, 31 Jul 2012 09:28:43 +0200
Paolo Bonzini  wrote:

> Il 30/07/2012 18:04, blauwir...@gmail.com ha scritto:
> > From: Blue Swirl 
> > 
> > Clang compiler complained about use of reserved word 'restrict' in SLIRP
> > and QAPI.
> > 
> > Rename 'restrict' to 'restricted' which also matches other SLIRP code.
> 
> Can't do it, this changes the command-line option.
> 
> Luiz, Michael, any ideas?

I'm not sure how complicated it would be to implement this, but we could add
a 'bind' keyword to the type dict to control mapping between protocol names
and generated variable names. Like this:

{ 'type': 'NetdevUserOptions',
  'data': {
'*hostname':  'str',
'*restrict':  'bool',
...
'*hostfwd':   ['String'],
'*guestfwd':  ['String'] },

  'bind': { 'restrict': 'restricted' } }

Re: [Qemu-devel] [PATCH 4/7] s390: sclp event support

[Andreas Färber]

Am 24.07.2012 09:37, schrieb Christian Borntraeger:
> From: Heinz Graalfs 
> 
> Several SCLP features are considered to be events. Those events don't
> provide SCLP commands on their own, instead they are all based on
> Read Event Data, Write Event Data, Write Event Mask and the service
> interrupt. Follow-on patches will provide SCLP's Signal Quiesce (via
> system_powerdown) and the ASCII console.
> Further down the road the sclp line mode console and configuration
> change events (e.g. cpu hotplug) can be implemented.
> 
> Signed-off-by: Heinz Graalfs 
> Signed-off-by: Christian Borntraeger 
> ---
>  hw/s390x/Makefile.objs|1 +
>  hw/s390x/event-facility.c |  390 
> +
>  hw/s390x/event-facility.h |  107 
>  hw/s390x/sclp.c   |   48 +-
>  hw/s390x/sclp.h   |   44 +
>  5 files changed, 587 insertions(+), 3 deletions(-)
>  create mode 100644 hw/s390x/event-facility.c
>  create mode 100644 hw/s390x/event-facility.h
> 
> diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs
> index 1c14b96..b32fc52 100644
> --- a/hw/s390x/Makefile.objs
> +++ b/hw/s390x/Makefile.objs
> @@ -2,3 +2,4 @@ obj-y = s390-virtio-bus.o s390-virtio.o
>  
>  obj-y := $(addprefix ../,$(obj-y))
>  obj-y += sclp.o
> +obj-y += event-facility.o
> diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
> new file mode 100644
> index 000..74a3514
> --- /dev/null
> +++ b/hw/s390x/event-facility.c
> @@ -0,0 +1,390 @@
> +/*
> + * SCLP
> + *Event Facility
> + *   handles SCLP event types
> + *  - Signal Quiesce - system power down
> + *  - ASCII Console Data - VT220 read and write
> + *
> + * Copyright IBM, Corp. 2012
> + *
> + * Authors:
> + *  Heinz Graalfs 
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or (at 
> your
> + * option) any later version.  See the COPYING file in the top-level 
> directory.
> + *
> + */
> +
> +#include "monitor.h"
> +#include "sysemu.h"
> +
> +#include "sclp.h"
> +#include "event-facility.h"
> +
> +typedef struct EventTypes {
> +BusState qbus;
> +SCLPEventFacility *event_facility;
> +} EventTypes;
> +
> +struct SCLPEventFacility {
> +EventTypes sbus;
> +DeviceState *qdev;
> +/* guest' receive mask */
> +unsigned int receive_mask;
> +};

The naming here strikes me as particularly odd...

IIUC this Event Facility is a device sitting on the SCLP bus.

But why does it expose a bus named "EventTypes"? Busses are usually
named ...Bus (PCIBus, IDEBus, etc.). So is this actually a bus? If not,
and if all you need is an SCLP-specific list API, maybe compare the
sPAPR hypercall registration API.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

[Qemu-devel] [PATCH] Makefile: Avoid explicit list of directories in clean target

[Peter Maydell]

Avoid having an explicit list of directories in the 'clean'
target by using 'find' to remove all .o and .d files instead.

Signed-off-by: Peter Maydell 
---
I figured that (unlike Makefile.target) we should probably take
the xargs route here since otherwise the rm command line is huge...

There's also an argument that there's not much point having a clean
target in Makefile.target when this one blows away most of it anyway.

 Makefile |7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/Makefile b/Makefile
index 621cb86..9f7eaa8 100644
--- a/Makefile
+++ b/Makefile
@@ -212,13 +212,10 @@ clean:
 # avoid old build problems by removing potentially incorrect old files
rm -f config.mak op-i386.h opc-i386.h gen-op-i386.h op-arm.h opc-arm.h 
gen-op-arm.h
rm -f qemu-options.def
-   rm -f *.o *.d *.a *.lo $(TOOLS) $(HELPERS-y) qemu-ga TAGS cscope.* 
*.pod *~ */*~
+   find . -name '*.[od]' | xargs rm -f
+   rm -f *.a *.lo $(TOOLS) $(HELPERS-y) qemu-ga TAGS cscope.* *.pod *~ */*~
rm -Rf .libs
-   rm -f slirp/*.o slirp/*.d audio/*.o audio/*.d block/*.o block/*.d 
net/*.o net/*.d fsdev/*.o fsdev/*.d ui/*.o ui/*.d qapi/*.o qapi/*.d qga/*.o 
qga/*.d
-   rm -f qom/*.o qom/*.d
-   rm -f usb/*.o usb/*.d hw/*.o hw/*.d
rm -f qemu-img-cmds.h
-   rm -f trace/*.o trace/*.d
rm -f trace-dtrace.dtrace trace-dtrace.dtrace-timestamp
@# May not be present in GENERATED_HEADERS
rm -f trace-dtrace.h trace-dtrace.h-timestamp
-- 
1.7.9.5

Re: [Qemu-devel] KVM call agenda for Tuesday, July 31

[Stefan Hajnoczi]

On Mon, Jul 30, 2012 at 8:34 AM, Juan Quintela  wrote:
> Please send in any agenda items you are interested in covering.

QEMU 1.2 Test Day
 * Let's find -rc bugs and ensure the release is stable
 * We've done this in the past and have a wiki template but can
discuss suggestions on the call

Stefan

Re: [Qemu-devel] [PATCH 0/2] net: Make -netdev socket,listen= work

[Stefan Hajnoczi]

On Tue, Jul 31, 2012 at 12:41 PM, Laszlo Ersek  wrote:
> (in reply to
>  --
> In-Reply-To set, but that may not be enough for the web archive)
>
> Looks good to me. A minor nit: 2/2 keeps the close(s->fd) call (instead
> of calling closesocket(s->fd), like in eoc handling) in
> net_socket_cleanup().

The reason I didn't change close(2) to closesocket() is because
-netdev socket,fd= can pass arbitrary file descriptors.  I suspect
only real socket objects will work but we basically don't know what
the passed file descriptor is.  This is the reason why I didn't feel
100% happy converting it to closesocket().

> Reviewed-by: Laszlo Ersek 

Thanks!

Stefan

Re: [Qemu-devel] [PATCH v4 06/07] s390: sclp ascii console support

[Christian Borntraeger]

On 30/07/12 16:02, Alexander Graf wrote:

>> +qemu_irq sclp_read_vt220;
> 
> I'm sure this one wants a name that indicates it's an irq line ;)

ok.

> 
>> +} SCLPConsole;
>> +
>> +/* character layer call-back functions */
>> +
>> +/* Return number of bytes that fit into iov buffer */
>> +static int chr_can_read(void *opaque)
>> +{
>> +int can_read;
>> +SCLPConsole *scon = opaque;
>> +
>> +qemu_mutex_lock(&scon->event.lock);
> 
> Explicit locks now?

IIRC Heinz introduced the locks since we have multiple threads working on the 
same
kind of buffers (the cpu threads and the main thread). We could not verify that 
the
main thread has the big qemu lock in all cases. Furthermore, this makes the 
code already
thread safe if we get rid of the big qemu lock somewhen. But I agree, we have 
to double
check why there are two different kind of locks.

[...]

>> +/* if new data do not fit into current buffer */
>> +if (scon->iov_data_len + size > SIZE_BUFFER_VT220) {
>> +/* character layer sent more than allowed */
>> +qemu_mutex_unlock(&scon->event.lock);
>> +return;
> 
> So we drop the bytes it did send? Isn't there usually some can_read function 
> from the char layer that we can indicate to that we have enough space?
> 
> If so, then this should be an assert(), right?

Probably yes. Will double check.

[..]

>> +SCLPConsole *cons = DO_UPCAST(SCLPConsole, event, event);
>> +
>> +/* first byte is hex 0 saying an ascii string follows */
>> +*buf++ = '\0';
>> +avail--;
>> +/* if all data fit into provided SCLP buffer */
>> +if (avail >= cons->iov_sclp_rest) {
>> +/* copy character byte-stream to SCLP buffer */
>> +memcpy(buf, cons->iov_sclp, cons->iov_sclp_rest);
>> +*size = cons->iov_sclp_rest + 1;
>> +cons->iov_sclp = cons->iov;
>> +cons->iov_bs = cons->iov;
>> +cons->iov_data_len = 0;
>> +cons->iov_sclp_rest = 0;
>> +event->event_pending = false;
>> +/* data provided and no more data pending */
>> +} else {
>> +/* if provided buffer is too small, just copy part */
>> +memcpy(buf, cons->iov_sclp, avail);
>> +*size = avail + 1;
>> +cons->iov_sclp_rest -= avail;
>> +cons->iov_sclp += avail;
>> +/* more data pending */
>> +}
>> +}
> 
> I'm wondering whether we actually need this indirection from
> 
>   chr layer -> buffer -> sclp buffer.
> 
> Why can't we just do
> 
>   chr layer -> sclp buffer?
> 

The sclp interface is a bit different here. On an input event, the hw generated 
an service
interrupt with the event pending bit set. Then the guest kernel does a read 
event data sclp
call with input buffer. The input data has to copied into that and then 
returned via an
additional interrupt. 

Without the buffering our can_read function could only return 0 as size since 
there is yet no
buffer. Makes sense? 

Christian

Re: [Qemu-devel] [PATCH 02/11] Add migrate_set_parameter and query-migrate-parameters

[Luiz Capitulino]

On Tue, 31 Jul 2012 10:46:02 +0300
Orit Wasserman  wrote:

> On 07/30/2012 08:41 PM, Luiz Capitulino wrote:
> > On Sun, 29 Jul 2012 12:42:54 +0300
> > Orit Wasserman  wrote:
> > 
> >> The management can enable/disable a capability for the next migration by 
> >> using
> >> migrate_set_parameter command.
> >> The management can query the current migration capabilities using
> >> query-migrate-parameters
> > 
> > In general looks good to me, I have a question and a few nitpick comments
> > below.
> > 
> >>
> >> Signed-off-by: Orit Wasserman 
> >> Signed-off-by: Juan Quintela 
> >> ---
> >>  hmp-commands.hx  |   16 
> >>  hmp.c|   71 
> >> ++
> >>  hmp.h|2 +
> >>  migration.c  |   42 +++-
> >>  migration.h  |2 +
> >>  monitor.c|7 +
> >>  qapi-schema.json |   32 
> >>  qmp-commands.hx  |   60 -
> >>  8 files changed, 229 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/hmp-commands.hx b/hmp-commands.hx
> >> index 8786148..3e15338 100644
> >> --- a/hmp-commands.hx
> >> +++ b/hmp-commands.hx
> >> @@ -861,6 +861,20 @@ Set maximum tolerated downtime (in seconds) for 
> >> migration.
> >>  ETEXI
> >>  
> >>  {
> >> +.name   = "migrate_set_parameter",
> >> +.args_type  = "capability:s,state:b",
> >> +.params = "capability state",
> >> +.help   = "Enable/Disable the usage of a capability for 
> >> migration",
> >> +.mhandler.cmd = hmp_migrate_set_parameter,
> >> +},
> >> +
> >> +STEXI
> >> +@item migrate_set_parameter @var{capability} @var{state}
> >> +@findex migrate_set_parameter
> >> +Enable/Disable the usage of a capability @var{capability} for migration.
> >> +ETEXI
> >> +
> >> +{
> >>  .name   = "client_migrate_info",
> >>  .args_type  = 
> >> "protocol:s,hostname:s,port:i?,tls-port:i?,cert-subject:s?",
> >>  .params = "protocol hostname port tls-port cert-subject",
> >> @@ -1419,6 +1433,8 @@ show user network stack connection states
> >>  show migration status
> >>  @item info migration_capabilities
> >>  show migration capabilities
> >> +@item info migrate_parameters
> >> +show current migration parameters
> >>  @item info balloon
> >>  show balloon information
> >>  @item info qtree
> >> diff --git a/hmp.c b/hmp.c
> >> index 5c7d0be..f2f63fd 100644
> >> --- a/hmp.c
> >> +++ b/hmp.c
> >> @@ -131,8 +131,22 @@ void hmp_info_mice(Monitor *mon)
> >>  void hmp_info_migrate(Monitor *mon)
> >>  {
> >>  MigrationInfo *info;
> >> +MigrationCapabilityStatusList *cap;
> >> +MigrationParameters *params;
> >>  
> >>  info = qmp_query_migrate(NULL);
> >> +params = qmp_query_migrate_parameters(NULL);
> >> +
> >> +/* do not display parameters during setup */
> >> +if (info->has_status && params->capabilities) {
> >> +monitor_printf(mon, "capabilities: ");
> >> +for (cap = params->capabilities; cap; cap = cap->next) {
> >> +monitor_printf(mon, "%s: %s ",
> >> +   
> >> MigrationCapability_lookup[cap->value->capability],
> >> +   cap->value->state ? "on" : "off");
> >> +}
> >> +monitor_printf(mon, "\n");
> >> +}
> >>  
> >>  if (info->has_status) {
> >>  monitor_printf(mon, "Migration status: %s\n", info->status);
> >> @@ -159,6 +173,7 @@ void hmp_info_migrate(Monitor *mon)
> >>  }
> >>  
> >>  qapi_free_MigrationInfo(info);
> >> +qapi_free_MigrationParameters(params);
> >>  }
> >>  
> >>  void hmp_info_migration_capabilities(Monitor *mon)
> >> @@ -180,6 +195,27 @@ void hmp_info_migration_capabilities(Monitor *mon)
> >>  qapi_free_MigrationCapabilityStatusList(caps_list);
> >>  }
> >>  
> >> +void hmp_info_migrate_parameters(Monitor *mon)
> >> +{
> >> +
> >> +MigrationCapabilityStatusList *cap;
> >> +MigrationParameters *params;
> >> +
> >> +params = qmp_query_migrate_parameters(NULL);
> >> +
> >> +if (params->capabilities) {
> >> +monitor_printf(mon, "capabilities: ");
> >> +for (cap = params->capabilities; cap; cap = cap->next) {
> >> +monitor_printf(mon, "%s: %s ",
> >> +   
> >> MigrationCapability_lookup[cap->value->capability],
> >> +   cap->value->state ? "on" : "off");
> >> +}
> >> +monitor_printf(mon, "\n");
> >> +}
> >> +
> >> +qapi_free_MigrationParameters(params);
> >> +}
> >> +
> >>  void hmp_info_cpus(Monitor *mon)
> >>  {
> >>  CpuInfoList *cpu_list, *cpu;
> >> @@ -754,6 +790,41 @@ void hmp_migrate_set_speed(Monitor *mon, const QDict 
> >> *qdict)
> >>  qmp_migrate_set_speed(value, NULL);
> >>  }
> >>  
> >> +void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
> >> +{
> >> +const char *cap = qdict_get_str(qdict, "c

Re: [Qemu-devel] [PATCH v3 0/2] GlusterFS support in QEMU - v3

[Bharata B Rao]

On Tue, Jul 31, 2012 at 01:56:29PM +0100, Stefan Hajnoczi wrote:
> On Tue, Jul 31, 2012 at 1:38 PM, Bharata B Rao
>  wrote:
> > On Tue, Jul 31, 2012 at 09:12:53AM +0100, Stefan Hajnoczi wrote:
> >> On Wed, Jul 25, 2012 at 6:58 AM, Bharata B Rao
> >>  wrote:
> >> > Apart from cleanups, the major change in this version is to expose all
> >> > the gluster configuration options to QEMU user. With this, the gluster
> >> > specification looks like this:
> >> >
> >> > -drive file=gluster:server:[port]:[transport]:volname:image
> >> >
> >> > - Here 'gluster' is the protocol.
> >> > - 'server' specifies the server where the volume file specification 
> >> > for
> >> >   the given volume resides.
> >>
> >> Works fine for hostnames and IPv4 addresses.  It seems like the ':'
> >> separator may prevent users from giving IPv6 addresses unless there is
> >> a way to escape ':'.
> >
> > I feel its better to go the URI way than to escape ':'. I will have this
> > specification in v4:
> >
> > gluster://server:[port]/volname/path/to/image?transport=socket
> >
> > As per http://tools.ietf.org/html/rfc3986#section-3.2.2, ipv6 addresses
> > are specified within square brackets in a URI which makes it easy to parse
> > the port after :
> 
> Okay.  Did you check that volnames cannot contain '/'?

Yes I have verifed that.

Regards,
Bharata.

Re: [Qemu-devel] [PATCH 10/11] Add XBZRLE statistics

[Luiz Capitulino]

On Tue, 31 Jul 2012 11:31:09 +0300
Orit Wasserman  wrote:

> On 07/30/2012 10:37 PM, Luiz Capitulino wrote:
> > On Sun, 29 Jul 2012 12:43:02 +0300
> > Orit Wasserman  wrote:
> > 
> >> Signed-off-by: Benoit Hudzia 
> >> Signed-off-by: Petter Svard 
> >> Signed-off-by: Aidan Shribman 
> >> Signed-off-by: Orit Wasserman 
> >> Signed-off-by: Juan Quintela 
> >> ---
> >>  arch_init.c  |   28 
> >>  hmp.c|   21 +
> >>  migration.c  |   28 
> >>  migration.h  |4 
> >>  qapi-schema.json |   32 +++-
> >>  qmp-commands.hx  |   35 +++
> >>  6 files changed, 147 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/arch_init.c b/arch_init.c
> >> index 7f12317..9833d54 100644
> >> --- a/arch_init.c
> >> +++ b/arch_init.c
> >> @@ -203,7 +203,11 @@ int64_t xbzrle_cache_resize(int64_t new_size)
> >>  typedef struct AccountingInfo {
> >>  uint64_t dup_pages;
> >>  uint64_t norm_pages;
> >> +uint64_t xbzrle_bytes;
> >> +uint64_t xbzrle_pages;
> >> +uint64_t xbzrle_cache_miss;
> >>  uint64_t iterations;
> >> +uint64_t xbzrle_overflows;
> >>  } AccountingInfo;
> >>  
> >>  static AccountingInfo acct_info;
> >> @@ -233,6 +237,26 @@ uint64_t norm_mig_pages_transferred(void)
> >>  return acct_info.norm_pages;
> >>  }
> >>  
> >> +uint64_t xbzrle_mig_bytes_transferred(void)
> >> +{
> >> +return acct_info.xbzrle_bytes;
> >> +}
> >> +
> >> +uint64_t xbzrle_mig_pages_transferred(void)
> >> +{
> >> +return acct_info.xbzrle_pages;
> >> +}
> >> +
> >> +uint64_t xbzrle_mig_pages_cache_miss(void)
> >> +{
> >> +return acct_info.xbzrle_cache_miss;
> >> +}
> >> +
> >> +uint64_t xbzrle_mig_pages_overflow(void)
> >> +{
> >> +return acct_info.xbzrle_overflows;
> >> +}
> >> +
> >>  static void save_block_hdr(QEMUFile *f, RAMBlock *block, ram_addr_t 
> >> offset,
> >>  int cont, int flag)
> >>  {
> >> @@ -257,6 +281,7 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t 
> >> *current_data,
> >>  if (!cache_is_cached(XBZRLE.cache, current_addr)) {
> >>  cache_insert(XBZRLE.cache, current_addr,
> >>   g_memdup(current_data, TARGET_PAGE_SIZE));
> >> +acct_info.xbzrle_cache_miss++;
> >>  return -1;
> >>  }
> >>  
> >> @@ -274,6 +299,7 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t 
> >> *current_data,
> >>  return 0;
> >>  } else if (encoded_len == -1) {
> >>  DPRINTF("Overflow\n");
> >> +acct_info.xbzrle_overflows++;
> >>  /* update data in the cache */
> >>  memcpy(prev_cached_page, current_data, TARGET_PAGE_SIZE);
> >>  return -1;
> >> @@ -288,6 +314,8 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t 
> >> *current_data,
> >>  qemu_put_be16(f, encoded_len);
> >>  qemu_put_buffer(f, XBZRLE.encoded_buf, encoded_len);
> >>  bytes_sent = encoded_len + 1 + 2;
> >> +acct_info.xbzrle_pages++;
> >> +acct_info.xbzrle_bytes += bytes_sent;
> >>  
> >>  return bytes_sent;
> >>  }
> >> diff --git a/hmp.c b/hmp.c
> >> index 9770d7b..383d5b1 100644
> >> --- a/hmp.c
> >> +++ b/hmp.c
> >> @@ -172,6 +172,27 @@ void hmp_info_migrate(Monitor *mon)
> >> info->disk->total >> 10);
> >>  }
> >>  
> >> +if (info->has_xbzrle_cache) {
> >> +monitor_printf(mon, "cache size: %" PRIu64 " bytes\n",
> >> +   info->xbzrle_cache->cache_size);
> >> +if (info->xbzrle_cache->has_xbzrle_bytes) {
> >> +monitor_printf(mon, "xbzrle transferred: %" PRIu64 " 
> >> kbytes\n",
> >> +   info->xbzrle_cache->xbzrle_bytes >> 10);
> >> +}
> >> +if (info->xbzrle_cache->has_xbzrle_pages) {
> >> +monitor_printf(mon, "xbzrle pages: %" PRIu64 " pages\n",
> >> +   info->xbzrle_cache->xbzrle_pages);
> >> +}
> >> +if (info->xbzrle_cache->has_xbzrle_cache_miss) {
> >> +monitor_printf(mon, "xbzrle cache miss: %" PRIu64 "\n",
> >> +   info->xbzrle_cache->xbzrle_cache_miss);
> >> +}
> >> +if (info->xbzrle_cache->has_xbzrle_overflow) {
> >> +monitor_printf(mon, "xbzrle overflow : %" PRIu64 "\n",
> >> +   info->xbzrle_cache->xbzrle_overflow);
> >> +}
> >> +}
> >> +
> >>  qapi_free_MigrationInfo(info);
> >>  qapi_free_MigrationParameters(params);
> >>  }
> >> diff --git a/migration.c b/migration.c
> >> index 4dc99ba..fb802bc 100644
> >> --- a/migration.c
> >> +++ b/migration.c
> >> @@ -136,6 +136,23 @@ MigrationParameters 
> >> *qmp_query_migrate_parameters(Error **errp)
> >>  return params;
> >>  }
> >>  
> >> +static void get_xbzrle_cache_stats(MigrationInfo *info)
> >> +{
> >> +if (migrate_use_xbzrle()) {
> >> +info->has_xbzrle_cache = true;
> >> +info->xbz

Re: [Qemu-devel] [PATCH 09/11] Add migration accounting for normal and duplicate pages

[Luiz Capitulino]

On Tue, 31 Jul 2012 11:36:03 +0300
Orit Wasserman  wrote:

> On 07/30/2012 10:30 PM, Luiz Capitulino wrote:
> > On Sun, 29 Jul 2012 12:43:01 +0300
> > Orit Wasserman  wrote:
> > 
> >> Signed-off-by: Benoit Hudzia 
> >> Signed-off-by: Petter Svard 
> >> Signed-off-by: Aidan Shribman 
> >> Signed-off-by: Orit Wasserman 
> >> Signed-off-by: Juan Quintela 
> >> ---
> >>  arch_init.c  |   38 ++
> >>  migration.c  |4 
> >>  migration.h  |5 +
> >>  qapi-schema.json |8 ++--
> >>  qmp-commands.hx  |   14 +++---
> >>  5 files changed, 64 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/arch_init.c b/arch_init.c
> >> index d709ccb..7f12317 100644
> >> --- a/arch_init.c
> >> +++ b/arch_init.c
> >> @@ -199,6 +199,40 @@ int64_t xbzrle_cache_resize(int64_t new_size)
> >>  return pow2floor(new_size);
> >>  }
> >>  
> >> +/* accounting for migration statistics */
> >> +typedef struct AccountingInfo {
> >> +uint64_t dup_pages;
> >> +uint64_t norm_pages;
> >> +uint64_t iterations;
> >> +} AccountingInfo;
> >> +
> >> +static AccountingInfo acct_info;
> >> +
> >> +static void acct_clear(void)
> >> +{
> >> +memset(&acct_info, 0, sizeof(acct_info));
> >> +}
> >> +
> >> +uint64_t dup_mig_bytes_transferred(void)
> >> +{
> >> +return acct_info.dup_pages * TARGET_PAGE_SIZE;
> >> +}
> >> +
> >> +uint64_t dup_mig_pages_transferred(void)
> >> +{
> >> +return acct_info.dup_pages;
> >> +}
> >> +
> >> +uint64_t norm_mig_bytes_transferred(void)
> >> +{
> >> +return acct_info.norm_pages * TARGET_PAGE_SIZE;
> >> +}
> >> +
> >> +uint64_t norm_mig_pages_transferred(void)
> >> +{
> >> +return acct_info.norm_pages;
> >> +}
> >> +
> >>  static void save_block_hdr(QEMUFile *f, RAMBlock *block, ram_addr_t 
> >> offset,
> >>  int cont, int flag)
> >>  {
> >> @@ -293,6 +327,7 @@ static int ram_save_block(QEMUFile *f)
> >>  p = memory_region_get_ram_ptr(mr) + offset;
> >>  
> >>  if (is_dup_page(p)) {
> >> +acct_info.dup_pages++;
> >>  save_block_hdr(f, block, offset, cont, 
> >> RAM_SAVE_FLAG_COMPRESS);
> >>  qemu_put_byte(f, *p);
> >>  bytes_sent = 1;
> >> @@ -308,6 +343,7 @@ static int ram_save_block(QEMUFile *f)
> >>  save_block_hdr(f, block, offset, cont, 
> >> RAM_SAVE_FLAG_PAGE);
> >>  qemu_put_buffer(f, p, TARGET_PAGE_SIZE);
> >>  bytes_sent = TARGET_PAGE_SIZE;
> >> +acct_info.norm_pages++;
> >>  }
> >>  
> >>  /* if page is unmodified, continue to the next */
> >> @@ -429,6 +465,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
> >>  }
> >>  XBZRLE.encoded_buf = g_malloc0(TARGET_PAGE_SIZE);
> >>  XBZRLE.current_buf = g_malloc(TARGET_PAGE_SIZE);
> >> +acct_clear();
> >>  }
> >>  
> >>  /* Make sure all dirty bits are set */
> >> @@ -477,6 +514,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
> >>  break;
> >>  }
> >>  bytes_transferred += bytes_sent;
> >> +acct_info.iterations++;
> >>  /* we want to check in the 1st loop, just in case it was the 1st 
> >> time
> >> and we had to sync the dirty bitmap.
> >> qemu_get_clock_ns() is a bit expensive, so we only check each 
> >> some
> >> diff --git a/migration.c b/migration.c
> >> index bc2231d..4dc99ba 100644
> >> --- a/migration.c
> >> +++ b/migration.c
> >> @@ -156,6 +156,8 @@ MigrationInfo *qmp_query_migrate(Error **errp)
> >>  info->ram->total = ram_bytes_total();
> >>  info->ram->total_time = qemu_get_clock_ms(rt_clock)
> >>  - s->total_time;
> >> +info->ram->duplicate = dup_mig_pages_transferred();
> >> +info->ram->normal = norm_mig_pages_transferred();
> >>  
> >>  if (blk_mig_active()) {
> >>  info->has_disk = true;
> >> @@ -175,6 +177,8 @@ MigrationInfo *qmp_query_migrate(Error **errp)
> >>  info->ram->remaining = 0;
> >>  info->ram->total = ram_bytes_total();
> >>  info->ram->total_time = s->total_time;
> >> +info->ram->duplicate = dup_mig_pages_transferred();
> >> +info->ram->normal = norm_mig_pages_transferred();
> >>  break;
> >>  case MIG_STATE_ERROR:
> >>  info->has_status = true;
> >> diff --git a/migration.h b/migration.h
> >> index 337e225..e4a7cd7 100644
> >> --- a/migration.h
> >> +++ b/migration.h
> >> @@ -87,6 +87,11 @@ uint64_t ram_bytes_total(void);
> >>  
> >>  extern SaveVMHandlers savevm_ram_handlers;
> >>  
> >> +uint64_t dup_mig_bytes_transferred(void);
> >> +uint64_t dup_mig_pages_transferred(void);
> >> +uint64_t norm_mig_bytes_transferred(void);
> >> +uint64_t norm_mig_pages_transferred(void);
> >> +
> >>  /**
> >>   * @migrate_add_blocker - prevent migration from proceeding
> >>   *
> >> diff --git a/qapi-schema.json b/q

Re: [Qemu-devel] Adding a parameter to a helper

[Laurent Desnogues]

On Mon, Jul 30, 2012 at 6:40 PM, Jose Cano Reyes  wrote:
> I am trying to add a new integer parameter to an existing helper and call
> this helper in "targeti386/translate.c". I have several problems:
>
> 1) I cannot add an integer parameter to the helper, the compiler says that
> it must be "TCGv_i32", despite I declare this new parameter as "int" in
> "target-i386/helper.h". Why?

Helpers only accept TCGv parameters.

> 2) If I use the the function "tcg_const_i32" in order to convert my integer
> to TCGv_i32 I always obtain the same output value, that is:
>
> tcg_const_i32(10) = 1074260520
> tcg_const_i32(22) = 1074260520
> tcg_const_i32(30) = 1074260520
> ...

TCGv is an index, not the value it represents.  Think of it
as an id.

tcg_const will allocate a TCGv and then emit a TCG mov
instruction to assign it a value.

> 3) Moreover, wen I pass this value in the helper call "gen_helper_flds_ST0",
> that is:
>
>  gen_helper_flds_ST0(cpu_tmp2_i32, tcg_const_i32(MY_INT_VALUE));
>
> How can I use MY_INT_VALUE later in the function "tcg_gen_helperN" .
> This function is called by DEF_HELPER_FLAGS2, which corresponds to
> DEF_HELPER_2 (definition of my helper).

Look at helper_aam, that should help :-)


Laurent

Re: [Qemu-devel] [QEMU PATCH 3/3] x86: pc: versioned CPU model names & compatibility aliases

[Igor Mammedov]

On Thu, Jul 26, 2012 at 10:48:45AM -0400, Eduardo Habkost wrote:
... 
> 
> [1] There are multiple changes I want to make the cpudef config format:
> 
> - Make it based on boolean per-feature flags, not low-level
>   feature_ bits
I'm trying to convert features to properties before looking at global props.
current state is at https://github.com/imammedo/qemu/tree/x86-cpu-properties.WIP
Does it represent something you have in mind?

>
... 
> -- 
> Eduardo
> 

Re: [Qemu-devel] KVM call agenda for Tuesday, July 31

[Eduardo Habkost]

On Mon, Jul 30, 2012 at 09:34:05AM +0200, Juan Quintela wrote:
> 
> Hi
> 
> Please send in any agenda items you are interested in covering.

- 1.2 plans for CPU model versioning/compatibility
  (global properties vs QOM vs qdev)

-- 
Eduardo

Re: [Qemu-devel] [PATCH] Makefile: Avoid explicit list of directories in clean target

[Stefan Hajnoczi]

On Tue, Jul 31, 2012 at 02:01:35PM +0100, Peter Maydell wrote:
> Avoid having an explicit list of directories in the 'clean'
> target by using 'find' to remove all .o and .d files instead.
> 
> Signed-off-by: Peter Maydell 
> ---
> I figured that (unlike Makefile.target) we should probably take
> the xargs route here since otherwise the rm command line is huge...
> 
> There's also an argument that there's not much point having a clean
> target in Makefile.target when this one blows away most of it anyway.
> 
>  Makefile |7 ++-
>  1 file changed, 2 insertions(+), 5 deletions(-)

Looks fine but waiting for others to review in case there is a subtlety
that affects some build environments.

Stefan

Re: [Qemu-devel] [PATCH v4] Fixes related to processing of qemu's -numa option

[Eduardo Habkost]

Ping? Can anybody help Vinod, here? How can we get the attention of some
maintainer, to get this pulled in?


On Wed, Jul 18, 2012 at 06:26:52PM +, Vinod, Chegu wrote:
> Thanks Eduardo !
> 
> Hi Anthony, If you are ok with this patch...could you pl pull these changes 
> into upstream (or) 
> suggest who I should talk to get these changes in ?
> 
> Thanks!
> Vinod
> 
> -Original Message-
> From: Eduardo Habkost [mailto:ehabk...@redhat.com] 
> Sent: Wednesday, July 18, 2012 10:15 AM
> To: Vinod, Chegu
> Cc: qemu-devel@nongnu.org; aligu...@us.ibm.com; k...@vger.kernel.org
> Subject: Re: [Qemu-devel] [PATCH v4] Fixes related to processing of qemu's 
> -numa option
> 
> On Mon, Jul 16, 2012 at 09:31:30PM -0700, Chegu Vinod wrote:
> > Changes since v3:
> >- using bitmap_set() instead of set_bit() in numa_add() routine.
> >- removed call to bitmak_zero() since bitmap_new() also zeros' the 
> > bitmap.
> >- Rebased to the latest qemu.
> 
> Tested-by: Eduardo Habkost 
> Reviewed-by: Eduardo Habkost 
> 
> 
> > 
> > Changes since v2:
> >- Using "unsigned long *" for the node_cpumask[].
> >- Use bitmap_new() instead of g_malloc0() for allocation.
> >- Don't rely on "max_cpus" since it may not be initialized
> >  before the numa related qemu options are parsed & processed.
> > 
> > Note: Continuing to use a new constant for allocation of
> >   the mask (This constant is currently set to 255 since
> >   with an 8bit APIC ID VCPUs can range from 0-254 in a
> >   guest. The APIC ID 255 (0xFF) is reserved for broadcast).
> > 
> > Changes since v1:
> > 
> >- Use bitmap functions that are already in qemu (instead
> >  of cpu_set_t macro's from sched.h)
> >- Added a check for endvalue >= max_cpus.
> >- Fix to address the round-robbing assignment when
> >  cpu's are not explicitly specified.
> > ---
> > 
> > v1:
> > --
> > 
> > The -numa option to qemu is used to create [fake] numa nodes and 
> > expose them to the guest OS instance.
> > 
> > There are a couple of issues with the -numa option:
> > 
> > a) Max VCPU's that can be specified for a guest while using
> >the qemu's -numa option is 64. Due to a typecasting issue
> >when the number of VCPUs is > 32 the VCPUs don't show up
> >under the specified [fake] numa nodes.
> > 
> > b) KVM currently has support for 160VCPUs per guest. The
> >qemu's -numa option has only support for upto 64VCPUs
> >per guest.
> > This patch addresses these two issues.
> > 
> > Below are examples of (a) and (b)
> > 
> > a) >32 VCPUs are specified with the -numa option:
> > 
> > /usr/local/bin/qemu-system-x86_64 \
> > -enable-kvm \
> > 71:01:01 \
> > -net tap,ifname=tap0,script=no,downscript=no \ -vnc :4
> > 
> > ...
> > Upstream qemu :
> > --
> > 
> > QEMU 1.1.50 monitor - type 'help' for more information
> > (qemu) info numa
> > 6 nodes
> > node 0 cpus: 0 1 2 3 4 5 6 7 8 9 32 33 34 35 36 37 38 39 40 41 node 0 
> > size: 131072 MB node 1 cpus: 10 11 12 13 14 15 16 17 18 19 42 43 44 45 
> > 46 47 48 49 50 51 node 1 size: 131072 MB node 2 cpus: 20 21 22 23 24 
> > 25 26 27 28 29 52 53 54 55 56 57 58 59 node 2 size: 131072 MB node 3 
> > cpus: 30 node 3 size: 131072 MB node 4 cpus:
> > node 4 size: 131072 MB
> > node 5 cpus: 31
> > node 5 size: 131072 MB
> > 
> > With the patch applied :
> > ---
> > 
> > QEMU 1.1.50 monitor - type 'help' for more information
> > (qemu) info numa
> > 6 nodes
> > node 0 cpus: 0 1 2 3 4 5 6 7 8 9
> > node 0 size: 131072 MB
> > node 1 cpus: 10 11 12 13 14 15 16 17 18 19 node 1 size: 131072 MB node 
> > 2 cpus: 20 21 22 23 24 25 26 27 28 29 node 2 size: 131072 MB node 3 
> > cpus: 30 31 32 33 34 35 36 37 38 39 node 3 size: 131072 MB node 4 
> > cpus: 40 41 42 43 44 45 46 47 48 49 node 4 size: 131072 MB node 5 
> > cpus: 50 51 52 53 54 55 56 57 58 59 node 5 size: 131072 MB
> > 
> > b) >64 VCPUs specified with -numa option:
> > 
> > /usr/local/bin/qemu-system-x86_64 \
> > -enable-kvm \
> > -cpu 
> > Westmere,+rdtscp,+pdpe1gb,+dca,+pdcm,+xtpr,+tm2,+est,+smx,+vmx,+ds_cpl
> > ,+monitor,+dtes64,+pclmuldq,+pbe,+tm,+ht,+ss,+acpi,+d-vnc :4
> > 
> > ...
> > 
> > Upstream qemu :
> > --
> > 
> > only 63 CPUs in NUMA mode supported.
> > only 64 CPUs in NUMA mode supported.
> > QEMU 1.1.50 monitor - type 'help' for more information
> > (qemu) info numa
> > 8 nodes
> > node 0 cpus: 6 7 8 9 38 39 40 41 70 71 72 73 node 0 size: 65536 MB 
> > node 1 cpus: 10 11 12 13 14 15 16 17 18 19 42 43 44 45 46 47 48 49 50 
> > 51 74 75 76 77 78 79 node 1 size: 65536 MB node 2 cpus: 20 21 22 23 24 
> > 25 26 27 28 29 52 53 54 55 56 57 58 59 60 61 node 2 size: 65536 MB 
> > node 3 cpus: 30 62 node 3 size: 65536 MB node 4 cpus:
> > node 4 size: 65536 MB
> > node 5 cpus:
> > node 5 size: 65536 MB
> > node 6 cpus: 31 63
> > node 6 size: 65536 MB
> > node 7 cpus: 0 1 2 3 4 5 32 33 34 35 36 37 64 65 66 67 68 69 node 7 
> > size: 6

Re: [Qemu-devel] 9p broken?

[Aneesh Kumar K.V]

Avi Kivity  writes:

> On 07/31/2012 09:51 AM, Aneesh Kumar K.V wrote:
>> Avi Kivity  writes:
>> 
>>> Having an annoying bug on i386 kvm I decided to debug it buy running an
>>> i386 guest on my x86_64 host, use 9p to access a guest image, and run it
>>> using nested kvm.
>>>
>>> However, 9p appears to be broken: first, the configure test fails (patch
>>> sent).  Second, while mount works, ls on the mount point causes qemu to
>>> crash with
>> 
>> I missed that you have already sent a patch for configure fix. That
>> looks better that what i sent. I will ack that patch 
>> 
>>>
>>> (gdb) bt
>>> #0  error_set (errp=0x7fffe95fb128, fmt=0x558d4568 "{ 'class':
>>> 'VirtFSFeatureBlocksMigration', 'data': { 'path': %s, 'tag': %s } }") at
>>> /home/tlv/akivity/qemu/error.c:32
>>> #1  0x5567cb06 in v9fs_attach (opaque=0x7fffe95e3020) at
>>> /home/tlv/akivity/qemu/hw/9pfs/virtio-9p.c:988
>>> #2  0x5561d19f in coroutine_trampoline (i0=1449767888, i1=21845)
>>> at /home/tlv/akivity/qemu/coroutine-ucontext.c:138
>>> #3  0x75a93ef0 in ?? () from /lib64/libc.so.6
>>> #4  0x7fffce00 in ?? ()
>>> #5  0x in ?? (
>>>
>>> **errp already points to a VirtFSFeatureBlocksMigration error;
>>> v9fs_attach() has been called a second time (the first time,
>>> understandably, on mount; the second on ls).
>>>
>> 
>> Why are we calling attach a second time ?. I am also not able to reproduce 
>> this
>> 
>> root@qemu-img-64:~# mount -t 9p -otrans=virtio,version=9p2000.L v_tmp /mnt
>> root@qemu-img-64:~# ls /mnt/a.c
>> /mnt/a.c
>> 
>
> I'm just doing ls /mnt (even tab completion: ls /mn crashes qemu).
>

Did this help ?

http://mid.gmane.org/1343719453-26768-1-git-send-email-aneesh.ku...@linux.vnet.ibm.com

-aneesh

Re: [Qemu-devel] [PATCH 0/2] net: Make -netdev socket,listen= work

[Stefan Hajnoczi]

On Fri, Jul 20, 2012 at 2:25 PM, Stefan Hajnoczi
 wrote:
> The socket backend does not support the listen= option with -netdev.  The
> problem is how the socket NetClientState lifecycle is implemented: the socket
> backend waits for an incoming client connection before creating a
> NetClientState.  The guest -device wants a peer= on startup, so QEMU fails 
> with
> an error about the non-existent peer.
>
> This series makes -netdev socket,listen= work by creating the NetClientState
> right away.  This allows -device peer= to find the socket backend.
>
> This code was written by Zhi Yong Wu .  I have only
> cleaned up and tested it.  The following work:
>  * -net socket,listen=:1234 -net nic,model=virtio
>  * -netdev socket,listen=:1234,id=netdev0 -device 
> virtio-net-pci,netdev=netdev0
>
> Zhi Yong Wu (2):
>   net: fix the coding style
>   net: add the support for -netdev socket, listen
>
>  net/socket.c |   82 
> +++---
>  1 file changed, 50 insertions(+), 32 deletions(-)

Merged into the net tree:
https://github.com/stefanha/qemu/tree/net

Stefan

Re: [Qemu-devel] [libvirt-users] Using virsh to load scripts for the guest machine

[Eric Blake]

On 07/30/2012 12:00 PM, Mauricio Tavares wrote:
> On Mon, Jul 30, 2012 at 1:50 PM, Eric Blake  wrote:
>> On 07/15/2012 07:52 PM, Mauricio Tavares wrote:
>>> Right on the top of
>>> http://www.centos.org/docs/5/html/5.2/Virtualization/chap-Virtualization-Managing_guests_with_virsh.html,
>>> it seems to imply you can load/send scripts to the vm guest using virsh.
>>> Is that possible? How and what are the limitations? Can you query the vm
>>> guest?
>>
>> What type of scripts are you talking about?  You may be thinking more
>> about the capabilities of what libguestfs provides, for modifying disk
>> images.  In general, virsh itself controls how to start a guest, but not
>> the additional layers of communication (such as virtio, qemu-ga, or the
>> libguestfs appliance app) required for a host to command a guest to do
>> something from within the guest.
>>
>   Basic one would be in case a machine has been paused for a long
> time. You know as in "hey, you lazy vm! You have been sleeping for two
> weeks! Now your clock is way off and poor ntp can't sync it back. So,
> here's current date!"

That is something that fits better through qemu-ga, but no one has
implemented it yet (cc'ing qemu-devel in case I'm misrepresenting
things).  Of course, if we did have a qemu-ga command for pushing the
current time into the guest, then libvirt could usefully expose an API
to wrap that qemu-ga command.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [QEMU PATCH 3/3] x86: pc: versioned CPU model names & compatibility aliases

[Eduardo Habkost]

On Tue, Jul 31, 2012 at 03:22:59PM +0200, Igor Mammedov wrote:
> On Thu, Jul 26, 2012 at 10:48:45AM -0400, Eduardo Habkost wrote:
> ... 
> > 
> > [1] There are multiple changes I want to make the cpudef config format:
> > 
> > - Make it based on boolean per-feature flags, not low-level
> >   feature_ bits
> I'm trying to convert features to properties before looking at global props.
> current state is at 
> https://github.com/imammedo/qemu/tree/x86-cpu-properties.WIP
> Does it represent something you have in mind?

Yes, it's like what I have in mind.

But note that cpudefs are not objects, yet. We have lots of people
focusing on the CPU objects themselves, but the cpudefs read from the
config are still plain old structs, with no properties, and there's no
clear plan on how to model them in the code.

Even if we convert the cpudefs to become classes, we still don't have a
simple way to make the cpudef read from the config file set CPU
properties only for specific classes.

Some of the approaches I see:
- Make classes have properties, too;
- Make cpudefs be full-featured objects, that are used as argument when
  creating CPU objects;
- Do it manually: have a feature bitmap (or other data structure) for
  each CPU class, translate the cpudef config options to that bitmap
  manually, and translate it to CPU features manually when creating the
  CPU object;
- Translate all the cpudef sections to global properties, that would
  in turn set CPU properties once the CPU objects are created.

The last option seems to work (once we make the CPU objects really able
to use global properties). But I have the feeling that setting lots of
global properties by default is an abuse of the global property system.

-- 
Eduardo

Re: [Qemu-devel] 9p broken?

[Avi Kivity]

On 07/31/2012 04:30 PM, Aneesh Kumar K.V wrote:

> 
> Did this help ?
> 
> http://mid.gmane.org/1343719453-26768-1-git-send-email-aneesh.ku...@linux.vnet.ibm.com
> 

It did: thanks.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] Compile errors on latest checkout

[Paolo Bonzini]

Il 31/07/2012 08:04, Gerhard Wiesinger ha scritto:
> Hello,
> 
> I'm getting the following compile errors:
> 
> /root/download/qemu/git/qemu/hw/megasas.c: In function
> ‘megasas_class_init’:
> /root/download/qemu/git/qemu/hw/megasas.c:2155:14: error: assignment
> from incompatible pointer type [-Werror]
> pc->exit = megasas_scsi_uninit;

See http://permalink.gmane.org/gmane.comp.emulators.qemu/162607 for this.

> With #define DEBUG_SCSI
> hw/scsi-disk.c: In function ‘scsi_write_complete’:
> hw/scsi-disk.c:450:9: error: format ‘%d’ expects argument of type ‘int’,
> but argument 3 has type ‘size_t’ [-Werror=format]
> hw/scsi-disk.c: In function ‘scsi_disk_emulate_read_data’:
> hw/scsi-disk.c:1274:9: error: format ‘%zd’ expects argument of type
> ‘signed size_t’, but argument 2 has type ‘int’ [-Werror=format]
> hw/scsi-disk.c: In function ‘scsi_disk_emulate_write_data’:
> hw/scsi-disk.c:1452:9: error: format ‘%zd’ expects argument of type
> ‘signed size_t’, but argument 2 has type ‘int’ [-Werror=format]
> hw/scsi-disk.c: In function ‘scsi_new_request’:
> hw/scsi-disk.c:2091:5: error: format ‘%x’ expects a matching ‘unsigned
> int’ argument [-Werror=format]
> hw/scsi-disk.c:2094:25: error: ‘r’ undeclared (first use in this function)
> hw/scsi-disk.c:2094:25: note: each undeclared identifier is reported
> only once for each function it appears in

See attached patch.

Paolo

> Please fix it.
> 
> gcc (GCC) 4.6.3 20120306 (Red Hat 4.6.3-2)
> 
> Thnx.
> 
> Ciao,
> Gerhard
> 
> 
> 


>From 8d64c6b73c8385107a9cc7aa25069ebb1aedd1c3 Mon Sep 17 00:00:00 2001
From: Paolo Bonzini 
Date: Tue, 31 Jul 2012 16:10:23 +0200
Subject: [PATCH] scsi-disk: fix compilation with DEBUG_SCSI

Reported-by: Gerhard Wiesinger 
Signed-off-by: Paolo Bonzini 
---
 hw/scsi-disk.c |   23 ---
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index e2ec177..a9c7279 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -447,7 +447,7 @@ static void scsi_write_complete(void * opaque, int ret)
 return;
 } else {
 scsi_init_iovec(r, SCSI_DMA_BUF_SIZE);
-DPRINTF("Write complete tag=0x%x more=%d\n", r->req.tag, r->qiov.size);
+DPRINTF("Write complete tag=0x%x more=%zd\n", r->req.tag, r->qiov.size);
 scsi_req_data(&r->req, r->qiov.size);
 }
 
@@ -1277,7 +1277,7 @@ static void scsi_disk_emulate_read_data(SCSIRequest *req)
 int buflen = r->iov.iov_len;
 
 if (buflen) {
-DPRINTF("Read buf_len=%zd\n", buflen);
+DPRINTF("Read buf_len=%d\n", buflen);
 r->iov.iov_len = 0;
 r->started = true;
 scsi_req_data(&r->req, buflen);
@@ -1455,7 +1455,7 @@ static void scsi_disk_emulate_write_data(SCSIRequest *req)
 
 if (r->iov.iov_len) {
 int buflen = r->iov.iov_len;
-DPRINTF("Write buf_len=%zd\n", buflen);
+DPRINTF("Write buf_len=%d\n", buflen);
 r->iov.iov_len = 0;
 scsi_req_data(&r->req, buflen);
 return;
@@ -2093,23 +2093,24 @@ static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun,
 const SCSIReqOps *ops;
 uint8_t command;
 
+command = buf[0];
+ops = scsi_disk_reqops_dispatch[command];
+if (!ops) {
+ops = &scsi_disk_emulate_reqops;
+}
+req = scsi_req_alloc(ops, &s->qdev, tag, lun, hba_private);
+
 #ifdef DEBUG_SCSI
-DPRINTF("Command: lun=%d tag=0x%x data=0x%02x", lun, buf[0]);
+DPRINTF("Command: lun=%d tag=0x%x data=0x%02x", lun, tag, buf[0]);
 {
 int i;
-for (i = 1; i < r->req.cmd.len; i++) {
+for (i = 1; i < req->cmd.len; i++) {
 printf(" 0x%02x", buf[i]);
 }
 printf("\n");
 }
 #endif
 
-command = buf[0];
-ops = scsi_disk_reqops_dispatch[command];
-if (!ops) {
-ops = &scsi_disk_emulate_reqops;
-}
-req = scsi_req_alloc(ops, &s->qdev, tag, lun, hba_private);
 return req;
 }
 
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH 10/11] Add XBZRLE statistics

[Orit Wasserman]

On 07/31/2012 04:16 PM, Luiz Capitulino wrote:
> On Tue, 31 Jul 2012 11:31:09 +0300
> Orit Wasserman  wrote:
> 
>> On 07/30/2012 10:37 PM, Luiz Capitulino wrote:
>>> On Sun, 29 Jul 2012 12:43:02 +0300
>>> Orit Wasserman  wrote:
>>>
 Signed-off-by: Benoit Hudzia 
 Signed-off-by: Petter Svard 
 Signed-off-by: Aidan Shribman 
 Signed-off-by: Orit Wasserman 
 Signed-off-by: Juan Quintela 
 ---
  arch_init.c  |   28 
  hmp.c|   21 +
  migration.c  |   28 
  migration.h  |4 
  qapi-schema.json |   32 +++-
  qmp-commands.hx  |   35 +++
  6 files changed, 147 insertions(+), 1 deletions(-)

 diff --git a/arch_init.c b/arch_init.c
 index 7f12317..9833d54 100644
 --- a/arch_init.c
 +++ b/arch_init.c
 @@ -203,7 +203,11 @@ int64_t xbzrle_cache_resize(int64_t new_size)
  typedef struct AccountingInfo {
  uint64_t dup_pages;
  uint64_t norm_pages;
 +uint64_t xbzrle_bytes;
 +uint64_t xbzrle_pages;
 +uint64_t xbzrle_cache_miss;
  uint64_t iterations;
 +uint64_t xbzrle_overflows;
  } AccountingInfo;
  
  static AccountingInfo acct_info;
 @@ -233,6 +237,26 @@ uint64_t norm_mig_pages_transferred(void)
  return acct_info.norm_pages;
  }
  
 +uint64_t xbzrle_mig_bytes_transferred(void)
 +{
 +return acct_info.xbzrle_bytes;
 +}
 +
 +uint64_t xbzrle_mig_pages_transferred(void)
 +{
 +return acct_info.xbzrle_pages;
 +}
 +
 +uint64_t xbzrle_mig_pages_cache_miss(void)
 +{
 +return acct_info.xbzrle_cache_miss;
 +}
 +
 +uint64_t xbzrle_mig_pages_overflow(void)
 +{
 +return acct_info.xbzrle_overflows;
 +}
 +
  static void save_block_hdr(QEMUFile *f, RAMBlock *block, ram_addr_t 
 offset,
  int cont, int flag)
  {
 @@ -257,6 +281,7 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t 
 *current_data,
  if (!cache_is_cached(XBZRLE.cache, current_addr)) {
  cache_insert(XBZRLE.cache, current_addr,
   g_memdup(current_data, TARGET_PAGE_SIZE));
 +acct_info.xbzrle_cache_miss++;
  return -1;
  }
  
 @@ -274,6 +299,7 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t 
 *current_data,
  return 0;
  } else if (encoded_len == -1) {
  DPRINTF("Overflow\n");
 +acct_info.xbzrle_overflows++;
  /* update data in the cache */
  memcpy(prev_cached_page, current_data, TARGET_PAGE_SIZE);
  return -1;
 @@ -288,6 +314,8 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t 
 *current_data,
  qemu_put_be16(f, encoded_len);
  qemu_put_buffer(f, XBZRLE.encoded_buf, encoded_len);
  bytes_sent = encoded_len + 1 + 2;
 +acct_info.xbzrle_pages++;
 +acct_info.xbzrle_bytes += bytes_sent;
  
  return bytes_sent;
  }
 diff --git a/hmp.c b/hmp.c
 index 9770d7b..383d5b1 100644
 --- a/hmp.c
 +++ b/hmp.c
 @@ -172,6 +172,27 @@ void hmp_info_migrate(Monitor *mon)
 info->disk->total >> 10);
  }
  
 +if (info->has_xbzrle_cache) {
 +monitor_printf(mon, "cache size: %" PRIu64 " bytes\n",
 +   info->xbzrle_cache->cache_size);
 +if (info->xbzrle_cache->has_xbzrle_bytes) {
 +monitor_printf(mon, "xbzrle transferred: %" PRIu64 " 
 kbytes\n",
 +   info->xbzrle_cache->xbzrle_bytes >> 10);
 +}
 +if (info->xbzrle_cache->has_xbzrle_pages) {
 +monitor_printf(mon, "xbzrle pages: %" PRIu64 " pages\n",
 +   info->xbzrle_cache->xbzrle_pages);
 +}
 +if (info->xbzrle_cache->has_xbzrle_cache_miss) {
 +monitor_printf(mon, "xbzrle cache miss: %" PRIu64 "\n",
 +   info->xbzrle_cache->xbzrle_cache_miss);
 +}
 +if (info->xbzrle_cache->has_xbzrle_overflow) {
 +monitor_printf(mon, "xbzrle overflow : %" PRIu64 "\n",
 +   info->xbzrle_cache->xbzrle_overflow);
 +}
 +}
 +
  qapi_free_MigrationInfo(info);
  qapi_free_MigrationParameters(params);
  }
 diff --git a/migration.c b/migration.c
 index 4dc99ba..fb802bc 100644
 --- a/migration.c
 +++ b/migration.c
 @@ -136,6 +136,23 @@ MigrationParameters 
 *qmp_query_migrate_parameters(Error **errp)
  return params;
  }
  
 +static void get_xbzrle_cache_stats(MigrationInfo *info)
 +{
 +if (migrate_use_xbzrle()) {
 +i

Re: [Qemu-devel] [PATCH] megasas: Update function megasys_scsi_uninit

[Paolo Bonzini]

Il 31/07/2012 07:54, Stefan Weil ha scritto:
> Commit f90c2bcdbc69e41e575f868b984c3e2de8f51bac changed
> PCIUnregisterFunc, therefore the function prototype
> needs an update.
> 
> megasas.o is currently not linked, so this bug was not
> detected by the buildbots.
> 
> Signed-off-by: Stefan Weil 
> ---
>  hw/megasas.c |3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/hw/megasas.c b/hw/megasas.c
> index 833f313..5235c50 100644
> --- a/hw/megasas.c
> +++ b/hw/megasas.c
> @@ -2041,7 +2041,7 @@ static const VMStateDescription vmstate_megasas = {
>  }
>  };
>  
> -static int megasas_scsi_uninit(PCIDevice *d)
> +static void megasas_scsi_uninit(PCIDevice *d)
>  {
>  MegasasState *s = DO_UPCAST(MegasasState, dev, d);
>  
> @@ -2051,7 +2051,6 @@ static int megasas_scsi_uninit(PCIDevice *d)
>  memory_region_destroy(&s->mmio_io);
>  memory_region_destroy(&s->port_io);
>  memory_region_destroy(&s->queue_io);
> -return 0;
>  }
>  
>  static const struct SCSIBusInfo megasas_scsi_info = {
> 

Applied to scsi-next branch, thanks.

Paolo

Re: [Qemu-devel] [PATCH] Makefile: Avoid explicit list of directories in clean target

[Markus Armbruster]

Peter Maydell  writes:

> Avoid having an explicit list of directories in the 'clean'
> target by using 'find' to remove all .o and .d files instead.
>
> Signed-off-by: Peter Maydell 
> ---
> I figured that (unlike Makefile.target) we should probably take
> the xargs route here since otherwise the rm command line is huge...
>
> There's also an argument that there's not much point having a clean
> target in Makefile.target when this one blows away most of it anyway.
>
>  Makefile |7 ++-
>  1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 621cb86..9f7eaa8 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -212,13 +212,10 @@ clean:
>  # avoid old build problems by removing potentially incorrect old files
>   rm -f config.mak op-i386.h opc-i386.h gen-op-i386.h op-arm.h opc-arm.h 
> gen-op-arm.h
>   rm -f qemu-options.def
> - rm -f *.o *.d *.a *.lo $(TOOLS) $(HELPERS-y) qemu-ga TAGS cscope.* 
> *.pod *~ */*~
> + find . -name '*.[od]' | xargs rm -f
> + rm -f *.a *.lo $(TOOLS) $(HELPERS-y) qemu-ga TAGS cscope.* *.pod *~ */*~

Shit happens if you somehow manage to create a "mean" file name in the
build tree.  Sure you don't want to -print0 | xargs -0?

>   rm -Rf .libs
> - rm -f slirp/*.o slirp/*.d audio/*.o audio/*.d block/*.o block/*.d 
> net/*.o net/*.d fsdev/*.o fsdev/*.d ui/*.o ui/*.d qapi/*.o qapi/*.d qga/*.o 
> qga/*.d
> - rm -f qom/*.o qom/*.d
> - rm -f usb/*.o usb/*.d hw/*.o hw/*.d
>   rm -f qemu-img-cmds.h
> - rm -f trace/*.o trace/*.d
>   rm -f trace-dtrace.dtrace trace-dtrace.dtrace-timestamp
>   @# May not be present in GENERATED_HEADERS
>   rm -f trace-dtrace.h trace-dtrace.h-timestamp

Re: [Qemu-devel] [PATCH 0/2] net: Make -netdev socket,listen= work

[Paolo Bonzini]

Il 31/07/2012 15:04, Stefan Hajnoczi ha scritto:
>> > Looks good to me. A minor nit: 2/2 keeps the close(s->fd) call (instead
>> > of calling closesocket(s->fd), like in eoc handling) in
>> > net_socket_cleanup().
> The reason I didn't change close(2) to closesocket() is because
> -netdev socket,fd= can pass arbitrary file descriptors.  I suspect
> only real socket objects will work but we basically don't know what
> the passed file descriptor is.  This is the reason why I didn't feel
> 100% happy converting it to closesocket().
> 

-netdev socket,fd= is probably not usable from Windows, so no problem
using close().

Paolo

Re: [Qemu-devel] [PATCH] Makefile: Avoid explicit list of directories in clean target

[Peter Maydell]

On 31 July 2012 15:19, Markus Armbruster  wrote:
> Peter Maydell  writes:
>> + find . -name '*.[od]' | xargs rm -f
>> + rm -f *.a *.lo $(TOOLS) $(HELPERS-y) qemu-ga TAGS cscope.* *.pod *~ 
>> */*~
>
> Shit happens if you somehow manage to create a "mean" file name in the
> build tree.  Sure you don't want to -print0 | xargs -0?

-print0 isn't POSIX, so I wasn't sure it would be present on all
our platforms. (It's probably fairly safe, though...)

-- PMM

Re: [Qemu-devel] [PATCH V4 01/12] hw/sd.c: convert wp_groups in SDState to bitfield

[Peter Maydell]

On 27 July 2012 20:29, Igor Mitsyanko  wrote:
> Representing each group write protection flag with only one bit instead of int
> variable significantly reduces memory consumption.

...and it looks much nicer too.

Reviewed-by: Peter Maydell 

-- PMM

Re: [Qemu-devel] [PATCH] Makefile: Avoid explicit list of directories in clean target

[Eric Blake]

On 07/31/2012 08:19 AM, Markus Armbruster wrote:
> Peter Maydell  writes:
> 
>> Avoid having an explicit list of directories in the 'clean'
>> target by using 'find' to remove all .o and .d files instead.
>>

>>  rm -f qemu-options.def
>> -rm -f *.o *.d *.a *.lo $(TOOLS) $(HELPERS-y) qemu-ga TAGS cscope.* 
>> *.pod *~ */*~
>> +find . -name '*.[od]' | xargs rm -f
>> +rm -f *.a *.lo $(TOOLS) $(HELPERS-y) qemu-ga TAGS cscope.* *.pod *~ */*~
> 
> Shit happens if you somehow manage to create a "mean" file name in the
> build tree.  Sure you don't want to -print0 | xargs -0?

Except that 'find -print0' and 'xargs -0' are both GNU extensions, not
available everywhere.  We may be requiring gmake and gcc, but are we
also requiring GNU find?

The POSIX way to write this, without relying on extensions, is:

find . -name '*.[od]' -exec rm -f {} +

(although then you get into the arguments of whether 'find -exec {} +'
is portable yet, even though it has now been required by POSIX for more
than 4 years.)

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH V4 02/12] hw/sd.c: make sd_wp_addr() accept 64 bit address argument

[Peter Maydell]

On 27 July 2012 20:29, Igor Mitsyanko  wrote:
> Currently sd_wp_addr() accepts 32 bit address arguments therefore implicitly
> restricting SD card address range. Change address argument type to uint64_t.
>
> Signed-off-by: Igor Mitsyanko 

Reviewed-by: Peter Maydell 

-- PMM

Re: [Qemu-devel] [PATCH V4 03/12] hw/sd.c: introduce wrapper for conversion address to wp group

[Peter Maydell]

On 27 July 2012 20:29, Igor Mitsyanko  wrote:
> Add wrapper function sd_addr_to_wpnum() to replace long address-->wg_group
> conversion line.
>
> Signed-off-by: Igor Mitsyanko 

Reviewed-by: Peter Maydell 

-- PMM

[Qemu-devel] KVM call minutes July 31th

[Juan Quintela]

Hi

Today minutes:

- Avi fixed and got 9p working
- QEMU 1.2 Test Day (stefan)
  * Let's find -rc bugs and ensure the release is stable
  * We've done this in the past and have a wiki template but can
discuss suggestions on the call
  * A date?

- 1.2 plans for CPU model versioning/compatibility (eduardo)
  (global properties vs QOM vs qdev)
  how to do it ?  configuration file?  moving back to the code?
  different external interface from internal one

  Eduardo/Andreas/Anthony, 

Re: [Qemu-devel] [PATCH V4 04/12] hw/sd.c: favour SD card type (SDSC or SDHC) when performing erase

[Peter Maydell]

On 27 July 2012 20:29, Igor Mitsyanko  wrote:
> Standard capacity cards SDSC use byte unit address while SDHC and SDXC Cards 
> use
> block unit address (512 bytes) when setting ERASE_START and ERASE_END with 
> CMD32
> and CMD33, we have to account for this.
>
> Signed-off-by: Igor Mitsyanko 
> ---
>  hw/sd.c |6 --
>  1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/hw/sd.c b/hw/sd.c
> index d0674d5..f7aa580 100644
> --- a/hw/sd.c
> +++ b/hw/sd.c
> @@ -482,8 +482,10 @@ static void sd_erase(SDState *sd)
>  return;
>  }
>
> -start = sd_addr_to_wpnum(sd->erase_start);
> -end = sd_addr_to_wpnum(sd->erase_end);
> +start = sd_addr_to_wpnum(sd->ocr & (1 << 30) ?
> +(uint64_t)sd->erase_start * 512 : sd->erase_start);
> +end = sd_addr_to_wpnum(sd->ocr & (1 << 30) ?
> +(uint64_t)sd->erase_end * 512 : sd->erase_end);

I think this would be a little clearer phrased as:

uint64_t erase_start = sd->erase_start;
uint64_t erase_end = sd->erase_end;

[...]

if (extract32(sd->ocr, 30, 1)) {
/* High capacity memory card: erase units are 512 byte blocks */
erase_start *= 512;
erase_end *= 512;
}

start = sd_addr_to_wpnum(erase_start);
end = sd_addr_to_wpnum(erase_end);

(I don't insist on the extract32() if you don't like it, but I
would like to avoid the repeated inline ?: ops.)

-- PMM

Re: [Qemu-devel] [PATCH] Makefile: Avoid explicit list of directories in clean target

[Markus Armbruster]

Peter Maydell  writes:

> On 31 July 2012 15:19, Markus Armbruster  wrote:
>> Peter Maydell  writes:
>>> + find . -name '*.[od]' | xargs rm -f
>>> + rm -f *.a *.lo $(TOOLS) $(HELPERS-y) qemu-ga TAGS cscope.* *.pod
>>> *~ */*~
>>
>> Shit happens if you somehow manage to create a "mean" file name in the
>> build tree.  Sure you don't want to -print0 | xargs -0?
>
> -print0 isn't POSIX, so I wasn't sure it would be present on all
> our platforms. (It's probably fairly safe, though...)

Another option is "-exec rm {} +".

Re: [Qemu-devel] [PATCH] megasas: Update function megasys_scsi_uninit

[Andreas Färber]

Am 31.07.2012 07:54, schrieb Stefan Weil:
> Commit f90c2bcdbc69e41e575f868b984c3e2de8f51bac changed
> PCIUnregisterFunc, therefore the function prototype
> needs an update.
> 
> megasas.o is currently not linked, so this bug was not
> detected by the buildbots.

Why would megasas be in master but not compiled/linked?

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH] Makefile: Avoid explicit list of directories in clean target

[Daniel P. Berrange]

On Tue, Jul 31, 2012 at 04:35:42PM +0200, Markus Armbruster wrote:
> Peter Maydell  writes:
> 
> > On 31 July 2012 15:19, Markus Armbruster  wrote:
> >> Peter Maydell  writes:
> >>> + find . -name '*.[od]' | xargs rm -f
> >>> + rm -f *.a *.lo $(TOOLS) $(HELPERS-y) qemu-ga TAGS cscope.* *.pod
> >>> *~ */*~
> >>
> >> Shit happens if you somehow manage to create a "mean" file name in the
> >> build tree.  Sure you don't want to -print0 | xargs -0?
> >
> > -print0 isn't POSIX, so I wasn't sure it would be present on all
> > our platforms. (It's probably fairly safe, though...)
> 
> Another option is "-exec rm {} +".

Isn't using 'find' somewhat overkill here really. QEMU only creates
.o and .d files in 2 levels of directory, so sure we can just avoid
find entirely

  rm -f *.[od] */*.[od]

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

Re: [Qemu-devel] [PATCH V4 07/12] hw/sd.c: make sd_wp_addr() return bool

[Peter Maydell]

On 27 July 2012 20:29, Igor Mitsyanko  wrote:
> For the sake of code clarity
>
> Signed-off-by: Igor Mitsyanko 

Reviewed-by: Peter Maydell 

-- PMM

Re: [Qemu-devel] [PATCH] megasas: Update function megasys_scsi_uninit

[Paolo Bonzini]

Il 31/07/2012 16:39, Andreas Färber ha scritto:
>> > Commit f90c2bcdbc69e41e575f868b984c3e2de8f51bac changed
>> > PCIUnregisterFunc, therefore the function prototype
>> > needs an update.
>> > 
>> > megasas.o is currently not linked, so this bug was not
>> > detected by the buildbots.
> Why would megasas be in master but not compiled/linked?

Because Anthony objected to how it picks the initiator WWN.

Paolo

Re: [Qemu-devel] [RFC 00/27]: add new error format

[Luiz Capitulino]

On Fri, 27 Jul 2012 18:31:41 -0300
Luiz Capitulino  wrote:

> [Please, read below why this is an RFC]
> 
> This series implements the 'Plan for error handling in QMP' as described
> by Anthony in this email:

For anyone willing to try this series, it applies on top of dfe1ce5d80
on master. My local mirror broke a few days ago and I only noticed
yesterday.

Re: [Qemu-devel] [PATCH] megasas: Update function megasys_scsi_uninit

[Andreas Färber]

Am 31.07.2012 16:43, schrieb Paolo Bonzini:
> Il 31/07/2012 16:39, Andreas Färber ha scritto:
 Commit f90c2bcdbc69e41e575f868b984c3e2de8f51bac changed
 PCIUnregisterFunc, therefore the function prototype
 needs an update.

 megasas.o is currently not linked, so this bug was not
 detected by the buildbots.
>> Why would megasas be in master but not compiled/linked?
> 
> Because Anthony objected to how it picks the initiator WWN.

Ah, anything keeping us from fixing that? :)

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg