Re: [Qemu-devel] [PATCH arm-devs v1 1/1] char/cadence_uart: Fix reset for unattached instances

[Sören Brinkmann]

On Fri, Jun 07, 2013 at 02:02:43PM +1000, peter.crosthwa...@xilinx.com wrote:
> From: Peter Crosthwaite 
> 
> commit 1db8b5efe0c2b5000e50691eea61264a615f43de introduced an issue
> where QEMU would segfault if you have an unattached Cadence UART.
> 
> Fix by guarding the flush-on-reset logic on there being a qemu_chr
> attachment.
> 
> Reported-by: Soren Brinkmann 
> Signed-off-by: Peter Crosthwaite 
Tested-by: Soren Brinkmann 

Thanks,
Sören

Re: [Qemu-devel] [PATCH RFT 2/5] virtio: Convert VirtioDevice to QOM realize/unrealize

[Peter Crosthwaite]

Hi Andreas,

On Sat, Jun 8, 2013 at 4:18 AM, Andreas Färber  wrote:
> VirtioDevice's DeviceClass::exit code cleaning up bus_name is no longer
> overwritten by virtio-{blk,serial,net,scsi,balloon,rng} and vhost-scsi.
>
> Note: VirtIOSCSI and VHostSCSI now perform some initializations after
> VirtIOSCSICommon's realize calls virtio_bus_plug_device(), namely
> creating the SCSIBus and initializing /dev/vhost-scsi respectively.
>
> While at it, drop duplicate VIRTIO_DEVICE() casts and avoid qdev.
>
> Signed-off-by: Andreas Färber 
> ---
>  hw/9pfs/virtio-9p-device.c | 67 --
>  hw/9pfs/virtio-9p.h| 13 ++
>  hw/block/virtio-blk.c  | 52 ++-
>  hw/char/virtio-serial-bus.c| 49 ++
>  hw/net/virtio-net.c| 48 -
>  hw/scsi/vhost-scsi.c   | 59 +++---
>  hw/scsi/virtio-scsi.c  | 85 
> --
>  hw/virtio/virtio-balloon.c | 50 +-
>  hw/virtio/virtio-rng.c | 53 ++--
>  hw/virtio/virtio.c | 20 -
>  include/hw/virtio/vhost-scsi.h | 13 ++
>  include/hw/virtio/virtio-balloon.h | 13 ++
>  include/hw/virtio/virtio-blk.h | 13 ++
>  include/hw/virtio/virtio-net.h | 13 ++
>  include/hw/virtio/virtio-rng.h | 13 ++
>  include/hw/virtio/virtio-scsi.h| 29 +++--
>  include/hw/virtio/virtio-serial.h  | 13 ++
>  include/hw/virtio/virtio.h |  6 ++-
>  18 files changed, 406 insertions(+), 203 deletions(-)
>
> diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
> index dc6f4e4..409d315 100644
> --- a/hw/9pfs/virtio-9p-device.c
> +++ b/hw/9pfs/virtio-9p-device.c
> @@ -41,15 +41,17 @@ static void virtio_9p_get_config(VirtIODevice *vdev, 
> uint8_t *config)
>  g_free(cfg);
>  }
>
> -static int virtio_9p_device_init(VirtIODevice *vdev)
> +static void virtio_9p_device_realize(DeviceState *dev, Error **errp)
>  {
> -V9fsState *s = VIRTIO_9P(vdev);
> +VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> +V9fsState *s = VIRTIO_9P(dev);
> +V9fsClass *v9c = VIRTIO_9P_GET_CLASS(dev);
>  int i, len;
>  struct stat stat;
>  FsDriverEntry *fse;
>  V9fsPath path;
>
> -virtio_init(VIRTIO_DEVICE(s), "virtio-9p", VIRTIO_ID_9P,
> +virtio_init(vdev, "virtio-9p", VIRTIO_ID_9P,
>  sizeof(struct virtio_9p_config) + MAX_TAG_LEN);
>
>  /* initialize pdu allocator */
> @@ -65,17 +67,17 @@ static int virtio_9p_device_init(VirtIODevice *vdev)
>
>  if (!fse) {
>  /* We don't have a fsdev identified by fsdev_id */
> -fprintf(stderr, "Virtio-9p device couldn't find fsdev with the "
> -"id = %s\n",
> -s->fsconf.fsdev_id ? s->fsconf.fsdev_id : "NULL");
> -return -1;
> +error_setg(errp, "Virtio-9p device couldn't find fsdev with the "
> +   "id = %s",
> +   s->fsconf.fsdev_id ? s->fsconf.fsdev_id : "NULL");
> +return;
>  }
>
>  if (!s->fsconf.tag) {
>  /* we haven't specified a mount_tag */
> -fprintf(stderr, "fsdev with id %s needs mount_tag arguments\n",
> -s->fsconf.fsdev_id);
> -return -1;
> +error_setg(errp, "fsdev with id %s needs mount_tag arguments",
> +   s->fsconf.fsdev_id);
> +return;
>  }
>
>  s->ctx.export_flags = fse->export_flags;
> @@ -83,9 +85,9 @@ static int virtio_9p_device_init(VirtIODevice *vdev)
>  s->ctx.exops.get_st_gen = NULL;
>  len = strlen(s->fsconf.tag);
>  if (len > MAX_TAG_LEN - 1) {
> -fprintf(stderr, "mount tag '%s' (%d bytes) is longer than "
> -"maximum (%d bytes)", s->fsconf.tag, len, MAX_TAG_LEN - 1);
> -return -1;
> +error_setg(errp, "mount tag '%s' (%d bytes) is longer than "
> +   "maximum (%d bytes)", s->fsconf.tag, len, MAX_TAG_LEN - 
> 1);
> +return;
>  }
>
>  s->tag = strdup(s->fsconf.tag);
> @@ -97,13 +99,13 @@ static int virtio_9p_device_init(VirtIODevice *vdev)
>  qemu_co_rwlock_init(&s->rename_lock);
>
>  if (s->ops->init(&s->ctx) < 0) {
> -fprintf(stderr, "Virtio-9p Failed to initialize fs-driver with id:%s"
> -" and export path:%s\n", s->fsconf.fsdev_id, s->ctx.fs_root);
> -return -1;
> +error_setg(errp, "Virtio-9p Failed to initialize fs-driver with 
> id:%s"
> +   " and export path:%s", s->fsconf.fsdev_id, 
> s->ctx.fs_root);
> +return;
>  }
>  if (v9fs_init_worker_threads() < 0) {
> -fprintf(stderr, "worker thread initialization failed\n");
> -return -1;
> +error_setg(errp, "worker thread initialization failed");
> +return;
>  }
>
>  /*
> @@ -113,20 +115,20 @@ static int virtio_9p_device_init(Vir

Re: [Qemu-devel] [PATCH V2] build: remove compile warning

[Wenchao Xia]

  I insist to remove compile warning since I want gcc check my code with
strict rule.


Wenchao Xia  writes:


This patch simply remove "variable may be used uninitialized" warning.

Signed-off-by: Wenchao Xia 
---
V2: Address Stefan and Peter's comments, use 0 in send_msg() instead of
initialize mhHeader.

  libcacard/vscclient.c |3 +--
  util/iov.c|2 +-
  2 files changed, 2 insertions(+), 3 deletions(-)






diff --git a/util/iov.c b/util/iov.c
index cc6e837..b91cfb9 100644
--- a/util/iov.c
+++ b/util/iov.c
@@ -146,7 +146,7 @@ ssize_t iov_send_recv(int sockfd, struct iovec *iov, 
unsigned iov_cnt,
  {
  ssize_t total = 0;
  ssize_t ret;
-size_t orig_len, tail;
+size_t orig_len = 0, tail;
  unsigned niov;

  while (bytes > 0) {


Here are the uses of orig_len:

 if (tail) {
 /* second, fixup the last element, and remember the original
  * length */
 assert(niov < iov_cnt);
 assert(iov[niov].iov_len > tail);
 orig_len = iov[niov].iov_len;
 iov[niov++].iov_len = tail;
 }

 ret = do_send_recv(sockfd, iov, niov, do_send);

 /* Undo the changes above before checking for errors */
 if (tail) {
 iov[niov-1].iov_len = orig_len;
 }


gcc is too stupid to understand the control flow.  The initialization
shuts it up.

Personally, I dislike "shut up" initializations, because when you
mistakenly adds a new uninitialized use, you get the arbitrary "shut up"
value without warning.

Possible alternative:

 if (tail) {
 /* second, fixup the last element, and remember the original
  * length */
 assert(niov < iov_cnt);
 assert(iov[niov].iov_len > tail);
 orig_len = iov[niov].iov_len;
 iov[niov++].iov_len = tail;
 ret = do_send_recv(sockfd, iov, niov, do_send);
 /* Undo the changes above before checking for errors */
 iov[niov-1].iov_len = orig_len;
 } else {
 ret = do_send_recv(sockfd, iov, niov, do_send);
 }


  OK to work, but duplicated a line. I think it is not bad to give
default value as zero, even there will be new use later.

--
Best Regards

Wenchao Xia

[Qemu-devel] [PATCH] kvmclock: clock should count only if vm is running

[Marcelo Tosatti]

kvmclock should not count while vm is paused, because:

1) if the vm is paused for long periods, timekeeping 
math can overflow while converting the (large) clocksource 
delta to nanoseconds.

2) Users rely on CLOCK_MONOTONIC to count run time, that is, 
time which OS has been in a runnable state (see CLOCK_BOOTTIME).

Change kvmclock driver so as to save clock value when vm transitions
from runnable to stopped state, and to restore clock value from stopped
to runnable transition.

Signed-off-by: Marcelo Tosatti 

diff --git a/hw/i386/kvm/clock.c b/hw/i386/kvm/clock.c
index 87d4d0f..7d2d005 100644
--- a/hw/i386/kvm/clock.c
+++ b/hw/i386/kvm/clock.c
@@ -28,38 +28,6 @@ typedef struct KVMClockState {
 bool clock_valid;
 } KVMClockState;
 
-static void kvmclock_pre_save(void *opaque)
-{
-KVMClockState *s = opaque;
-struct kvm_clock_data data;
-int ret;
-
-if (s->clock_valid) {
-return;
-}
-ret = kvm_vm_ioctl(kvm_state, KVM_GET_CLOCK, &data);
-if (ret < 0) {
-fprintf(stderr, "KVM_GET_CLOCK failed: %s\n", strerror(ret));
-data.clock = 0;
-}
-s->clock = data.clock;
-/*
- * If the VM is stopped, declare the clock state valid to avoid re-reading
- * it on next vmsave (which would return a different value). Will be reset
- * when the VM is continued.
- */
-s->clock_valid = !runstate_is_running();
-}
-
-static int kvmclock_post_load(void *opaque, int version_id)
-{
-KVMClockState *s = opaque;
-struct kvm_clock_data data;
-
-data.clock = s->clock;
-data.flags = 0;
-return kvm_vm_ioctl(kvm_state, KVM_SET_CLOCK, &data);
-}
 
 static void kvmclock_vm_state_change(void *opaque, int running,
  RunState state)
@@ -70,8 +38,18 @@ static void kvmclock_vm_state_change(void *opaque, int 
running,
 int ret;
 
 if (running) {
+struct kvm_clock_data data;
+
 s->clock_valid = false;
 
+data.clock = s->clock;
+data.flags = 0;
+ret = kvm_vm_ioctl(kvm_state, KVM_SET_CLOCK, &data);
+if (ret < 0) {
+fprintf(stderr, "KVM_SET_CLOCK failed: %s\n", strerror(ret));
+abort();
+}
+
 if (!cap_clock_ctrl) {
 return;
 }
@@ -84,6 +62,26 @@ static void kvmclock_vm_state_change(void *opaque, int 
running,
 return;
 }
 }
+} else {
+struct kvm_clock_data data;
+int ret;
+
+if (s->clock_valid) {
+return;
+}
+ret = kvm_vm_ioctl(kvm_state, KVM_GET_CLOCK, &data);
+if (ret < 0) {
+fprintf(stderr, "KVM_GET_CLOCK failed: %s\n", strerror(ret));
+abort();
+}
+s->clock = data.clock;
+
+/*
+ * If the VM is stopped, declare the clock state valid to
+ * avoid re-reading it on next vmsave (which would return
+ * a different value). Will be reset when the VM is continued.
+ */
+s->clock_valid = !runstate_is_running();
 }
 }
 
@@ -100,8 +98,6 @@ static const VMStateDescription kvmclock_vmsd = {
 .version_id = 1,
 .minimum_version_id = 1,
 .minimum_version_id_old = 1,
-.pre_save = kvmclock_pre_save,
-.post_load = kvmclock_post_load,
 .fields = (VMStateField[]) {
 VMSTATE_UINT64(clock, KVMClockState),
 VMSTATE_END_OF_LIST()

Re: [Qemu-devel] [PATCH 0/2] gdbstub runstate check follow-ups

[Andreas Färber]

Am 03.06.2013 17:06, schrieb Paolo Bonzini:
> My patch committed at 87f25c12bfeaaa0c41fb857713bbc7e8a9b757dc
> was broken.  These patches fix the problem in a better way.
> 
> Paolo
> 
> Paolo Bonzini (2):
>   gdbstub: fix for commit 87f25c12bfeaaa0c41fb857713bbc7e8a9b757dc
>   gdbstub: let the debugger resume from guest panicked state

Tested-by: Andreas Färber 

Andreas

>  gdbstub.c | 5 -
>  vl.c  | 1 +
>  2 files changed, 5 insertions(+), 1 deletion(-)

-- 
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] [Qemu-trivial] [PATCH v1 1/5] intc/xilinx_intc: Use qemu_set_irq

[Michael Tokarev]

07.06.2013 06:39, Peter Crosthwaite wrote:
>> Use qemu_set_irq rather than if-elsing qemu_irq_(lower|raise). No
>> functional change, just reduces verbosity.

Thanks, applied to the trivial patches queue.

/mjt

Re: [Qemu-devel] [Qemu-trivial] [PATCH] configure: Disable host-bsd USB on FreeBSD

[Michael Tokarev]

06.06.2013 17:18, Ed Maste wrote:
> It hasn't built since FreeBSD 8.x, and is disabled by a patch in the
> FreeBSD ports tree.  FreeBSD is migrating to QEMU's libusb support.

Thanks, applied to the trivial patches queue.

/mjt

Re: [Qemu-devel] [Qemu-trivial] [PATCH] configure: remove ${config_host_ld} variable

[Michael Tokarev]

06.06.2013 16:53, Ed Maste wrote:
> It was only used in one place (and already expanded in one other).

Thanks, applied to the trivial patches queue.

I think we should just as well get rid of ${config_host_mak},
as it makes no sense too... ;)   And while at it, maybe use
common constructs like

{ echo a; echo b; echo c; ... } > config-host.mak

But that's for another time :)

/mjt

Re: [Qemu-devel] [PATCH v4] qemu-char: don't issue CHR_EVENT_OPEN in a BH

[Luiz Capitulino]

On Fri, 07 Jun 2013 16:01:34 -0500
Anthony Liguori  wrote:

> Luiz Capitulino  writes:
> 
> > On Fri, 7 Jun 2013 09:56:00 -0500
> > mdroth  wrote:
> >
> >> On Fri, Jun 07, 2013 at 09:50:39AM -0400, Luiz Capitulino wrote:
> >> > On Tue,  4 Jun 2013 16:35:09 -0500
> >> > Michael Roth  wrote:
> >> > 
> >> > > When CHR_EVENT_OPENED was initially added, it was CHR_EVENT_RESET,
> >> > > and it was issued as a bottom-half:
> >> > > 
> >> > > 86e94dea5b740dad65446c857f6959eae43e0ba6
> >> > > 
> >> > > Which we basically used to print out a greeting/prompt for the
> >> > > monitor.
> >> > > 
> >> > > AFAICT the only reason this was ever done in a BH was because in
> >> > > some cases we'd modify the chr_write handler for a new chardev
> >> > > backend *after* the site where we issued the reset (see:
> >> > > 86e94d:qemu_chr_open_stdio())
> >> > > 
> >> > > At some point this event was renamed to CHR_EVENT_OPENED, and we've
> >> > > maintained the use of this BH ever since.
> >> > > 
> >> > > However, due to 9f939df955a4152aad69a19a77e0898631bb2c18, we schedule
> >> > > the BH via g_idle_add(), which is causing events to sometimes be
> >> > > delivered after we've already begun processing data from backends,
> >> > > leading to:
> >> > > 
> >> > >  known bugs:
> >> > > 
> >> > >   QMP:
> >> > > session negotation resets with OPENED event, in some cases this
> >> > > is causing new sessions to get sporadically reset
> >> > > 
> >> > >  potential bugs:
> >> > > 
> >> > >   hw/usb/redirect.c:
> >> > > can_read handler checks for dev->parser != NULL, which may be
> >> > > true if CLOSED BH has not been executed yet. In the past, OPENED
> >> > > quiesced outstanding CLOSED events prior to us reading client
> >> > > data. If it's delayed, our check may allow reads to occur even
> >> > > though we haven't processed the OPENED event yet, and when we
> >> > > do finally get the OPENED event, our state may get reset.
> >> > > 
> >> > >   qtest.c:
> >> > > can begin session before OPENED event is processed, leading to
> >> > > a spurious reset of the system and irq_levels
> >> > > 
> >> > >   gdbstub.c:
> >> > > may start a gdb session prior to the machine being paused
> >> > > 
> >> > > To fix these, let's just drop the BH.
> >> > > 
> >> > > Since the initial reasoning for using it still applies to an extent,
> >> > > work around that by deferring the delivery of CHR_EVENT_OPENED until
> >> > > after the chardevs have been fully initialized, toward the end of
> >> > > qmp_chardev_add() (or some cases, qemu_chr_new_from_opts()). This
> >> > > defers delivery long enough that we can be assured a CharDriverState
> >> > > is fully initialized before CHR_EVENT_OPENED is sent.
> >> > > 
> >> > > Also, rather than requiring each chardev to do an explicit open, do it
> >> > > automatically, and allow the small few who don't desire such behavior 
> >> > > to
> >> > > suppress the OPENED-on-init behavior by setting a 'explicit_be_open'
> >> > > flag.
> >> > > 
> >> > > We additionally add missing OPENED events for stdio backends on w32,
> >> > > which were previously not being issued, causing us to not recieve the
> >> > > banner and initial prompts for qmp/hmp.
> >> > > 
> >> > > Reported-by: Stefan Priebe 
> >> > > Cc: qemu-sta...@nongnu.org
> >> > > Signed-off-by: Michael Roth 
> >> > 
> >> > I don't know if the QMP queue is the ideal queue for this patch, but
> >> > I'd happily apply it if I get at least one Reviewed-by from the CC'ed
> >> > people.
> >> 
> >> Anthony actually added his Reviewed-by for v3, but I forgot to roll it
> >> in the commit. If you do take it in through your tree can you add that?
> >
> > Sorry for the bureaucracy, but I don't like to add other people's tags
> > when the patch changes. Even when I'm sure they would be OK with the
> > new version.
> >
> > Anthony, can you please add your Reviewed-by again?
> 
> I'll apply this patch directly since it's really a chardev patch more
> than a QMP patch.

ACK

Re: [Qemu-devel] [PATCH] correct RTC_CHANGE_EVENT description (v2)

[Luiz Capitulino]

On Fri, 7 Jun 2013 16:52:43 -0300
Marcelo Tosatti  wrote:

> 
> Fix RTC_CHANGE event description to match implementation.
> 
> Signed-off-by: Marcelo Tosatti 

Applied to the qmp branch, thanks.

> 
> diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
> index 92fe5fb..24e804e9 100644
> --- a/QMP/qmp-events.txt
> +++ b/QMP/qmp-events.txt
> @@ -203,7 +203,8 @@ Emitted when the guest changes the RTC time.
>  
>  Data:
>  
> -- "offset": delta against the host UTC in seconds (json-number)
> +- "offset": Offset between base RTC clock (as specified by -rtc base), and
> +new RTC clock value (json-number)
>  
>  Example:
>  
> 

Re: [Qemu-devel] [PATCH v4] qemu-char: don't issue CHR_EVENT_OPEN in a BH

[Anthony Liguori]

Luiz Capitulino  writes:

> On Fri, 7 Jun 2013 09:56:00 -0500
> mdroth  wrote:
>
>> On Fri, Jun 07, 2013 at 09:50:39AM -0400, Luiz Capitulino wrote:
>> > On Tue,  4 Jun 2013 16:35:09 -0500
>> > Michael Roth  wrote:
>> > 
>> > > When CHR_EVENT_OPENED was initially added, it was CHR_EVENT_RESET,
>> > > and it was issued as a bottom-half:
>> > > 
>> > > 86e94dea5b740dad65446c857f6959eae43e0ba6
>> > > 
>> > > Which we basically used to print out a greeting/prompt for the
>> > > monitor.
>> > > 
>> > > AFAICT the only reason this was ever done in a BH was because in
>> > > some cases we'd modify the chr_write handler for a new chardev
>> > > backend *after* the site where we issued the reset (see:
>> > > 86e94d:qemu_chr_open_stdio())
>> > > 
>> > > At some point this event was renamed to CHR_EVENT_OPENED, and we've
>> > > maintained the use of this BH ever since.
>> > > 
>> > > However, due to 9f939df955a4152aad69a19a77e0898631bb2c18, we schedule
>> > > the BH via g_idle_add(), which is causing events to sometimes be
>> > > delivered after we've already begun processing data from backends,
>> > > leading to:
>> > > 
>> > >  known bugs:
>> > > 
>> > >   QMP:
>> > > session negotation resets with OPENED event, in some cases this
>> > > is causing new sessions to get sporadically reset
>> > > 
>> > >  potential bugs:
>> > > 
>> > >   hw/usb/redirect.c:
>> > > can_read handler checks for dev->parser != NULL, which may be
>> > > true if CLOSED BH has not been executed yet. In the past, OPENED
>> > > quiesced outstanding CLOSED events prior to us reading client
>> > > data. If it's delayed, our check may allow reads to occur even
>> > > though we haven't processed the OPENED event yet, and when we
>> > > do finally get the OPENED event, our state may get reset.
>> > > 
>> > >   qtest.c:
>> > > can begin session before OPENED event is processed, leading to
>> > > a spurious reset of the system and irq_levels
>> > > 
>> > >   gdbstub.c:
>> > > may start a gdb session prior to the machine being paused
>> > > 
>> > > To fix these, let's just drop the BH.
>> > > 
>> > > Since the initial reasoning for using it still applies to an extent,
>> > > work around that by deferring the delivery of CHR_EVENT_OPENED until
>> > > after the chardevs have been fully initialized, toward the end of
>> > > qmp_chardev_add() (or some cases, qemu_chr_new_from_opts()). This
>> > > defers delivery long enough that we can be assured a CharDriverState
>> > > is fully initialized before CHR_EVENT_OPENED is sent.
>> > > 
>> > > Also, rather than requiring each chardev to do an explicit open, do it
>> > > automatically, and allow the small few who don't desire such behavior to
>> > > suppress the OPENED-on-init behavior by setting a 'explicit_be_open'
>> > > flag.
>> > > 
>> > > We additionally add missing OPENED events for stdio backends on w32,
>> > > which were previously not being issued, causing us to not recieve the
>> > > banner and initial prompts for qmp/hmp.
>> > > 
>> > > Reported-by: Stefan Priebe 
>> > > Cc: qemu-sta...@nongnu.org
>> > > Signed-off-by: Michael Roth 
>> > 
>> > I don't know if the QMP queue is the ideal queue for this patch, but
>> > I'd happily apply it if I get at least one Reviewed-by from the CC'ed
>> > people.
>> 
>> Anthony actually added his Reviewed-by for v3, but I forgot to roll it
>> in the commit. If you do take it in through your tree can you add that?
>
> Sorry for the bureaucracy, but I don't like to add other people's tags
> when the patch changes. Even when I'm sure they would be OK with the
> new version.
>
> Anthony, can you please add your Reviewed-by again?

I'll apply this patch directly since it's really a chardev patch more
than a QMP patch.

Regards,

Anthony Liguori

>
>> 
>> Thanks!
>> 
>> > 
>> > 
>> > > ---
>> > > v3->v4:
>> > >  * renamed 'suppress_be_open_on_init' to 'explicit_be_open' to match
>> > >existing 'explicit_fe_open' flag (Hans)
>> > >  * added missing 'explicit_be_open' flags for spice vmc/port and
>> > >msmouse backends
>> > > 
>> > > v2->v3:
>> > >  * removed artifact in from v1 in backends/baum.c, test build with
>> > >BRLAPI=y (Anthony)
>> > >  * rebased on latest origin/master
>> > > 
>> > > v1->v2:
>> > >  * default to sending OPENED on backend init, add flag to suppress
>> > >it (Anthony)
>> > >  * fix missing OPENED for stdio backends on w32
>> > >  * fix missing OPENED when qemu_chr_new_from_opts() doesn't use
>> > >qmp_chardev_add()
>> > >  * clean up/update commit message
>> > > 
>> > >  backends/baum.c   |2 --
>> > >  backends/msmouse.c|1 +
>> > >  include/sysemu/char.h |2 +-
>> > >  qemu-char.c   |   38 +-
>> > >  spice-qemu-char.c |1 +
>> > >  ui/console.c  |1 -
>> > >  ui/gtk.c  |1 -
>> > >  7 files changed, 20 insertions(+), 26 deletions(-)
>> > > 
>> > > diff --git a/backends/baum.c b/backends/baum

Re: [Qemu-devel] [PULL 0/3] QMP queue

[Luiz Capitulino]

On Fri,  7 Jun 2013 15:54:12 -0400
Luiz Capitulino  wrote:

> The changes (since 7387de16d0e4d2988df350926537cd12a8e34206) are available
> in the following repository:
> 
> git://repo.or.cz/qemu/qmp-unstable.git queue/qmp
> 
> Luiz Capitulino (2):
>   MAINTAINERS: new maintainers for qapi-schema.json
>   MAINTAINERS: split Monitor (QMP/HMP) entry
> 
> Marcelo Tosatti (1):
>   correct RTC_CHANGE_EVENT description

Anthony, I've replaced Marcelo's patch. Hope this won't disturb
your work flow.

[Qemu-devel] [PATCH] correct RTC_CHANGE_EVENT description (v2)

[Marcelo Tosatti]

Fix RTC_CHANGE event description to match implementation.

Signed-off-by: Marcelo Tosatti 

diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
index 92fe5fb..24e804e9 100644
--- a/QMP/qmp-events.txt
+++ b/QMP/qmp-events.txt
@@ -203,7 +203,8 @@ Emitted when the guest changes the RTC time.
 
 Data:
 
-- "offset": delta against the host UTC in seconds (json-number)
+- "offset": Offset between base RTC clock (as specified by -rtc base), and
+new RTC clock value (json-number)
 
 Example:
 

Re: [Qemu-devel] [PATCH] script: git script to compile every commit in a range of commits

[Jeff Cody]

On Fri, Jun 07, 2013 at 11:51:36AM -0500, Anthony Liguori wrote:
> Laszlo Ersek  writes:
> 
> > On 06/07/13 16:44, Jeff Cody wrote:
> >
> >> Thanks.  I can either do the above changes for a v2, or as follow on
> >> patches.
> >
> > Whichever is easier for you, certainly! I'm fine with the script
> > going-in as is.
> 
> A suggestion I'll make is to split the script into two parts.

Hi Anthony,

I'm sorry, but I'm a bit confused by your suggestion.  I think I know
what you are asking (see below), but I'm not positive.

> git-bisect run is a terribly useful command and I use a bisect script
> that looks like this:
> 
> #!/bin/sh
> 
> set -e
> 
> pushd ~/build/qemu
> make -j1 || exit 1
> popd
> 
> # Add right seed here
> if test "$1"; then
> "$@"
> fi
> 
> I'm sure we all have bisect scripts like this.
> 
> What you're proposing is very similar to bisect but instead of doing a
> binary search, it does a linear search starting from the oldest commit.
> Basically:

I agree that git bisect is useful, but solves a slightly different
problem than what I am looking to solve.

For instance, in my working branches I have a whole stack of commits
that I will rebase and squash into a set of sane patches before
submitting.  To make sure I don't do something silly in that process,
and create a patch X that does not build without patch X+1, I want to
explicitly compile each patch, without skipping over any patches.


> #!/bin/sh
> 
> refspec="$1"
> shift
> 
> git rev-list $refspec | while read commit; do
> git checkout $commit
> "$@" || exit $?
> done
> 
> And indeed, I have a local script called git-foreach to do exactly
> this.  I suspect a nicer version would make a very good addition to the
> git project.
> 
> So to bisect for a make check failure, I do:
> 
>   git bisect run bisect.sh make check
> 
> Or to bisect for a qemu-test failure:
> 
>   git bisect run bisect.sh qemu-test-regress.sh
> 
> With git-foreach, you can do:
> 
>   git-foreach bisect.sh
> 
> To do a simple build test.  Or you can do:
> 
>   git-foreach git show checkpatch-head.sh
>
> etc.

Ah!  So if I understand correctly, what you are asking is to split
the script up into two different scripts:

1.) A 'foreach' framework script to run an arbitrary command over a
range of commits, against each commit  (i.e. in the place where I run
'make clean, git checkout, configure, and make [lines 188-191], simply
do the git checkout and execute a passed script / command).

2.) A second script to perform the complication check, intended to be
called by script 1).  We could then add additional scripts to be
called by the 'foreach' framework patch as desired.

Heck, if we wanted to, we could then even create a menu-drive
meta-script to interactively run any number of tests (checkpatch,
compilation, etc..) using that framework.

> 
> Regards,
> 
> Anthony Liguori
>

Thanks,

Jeff

[Qemu-devel] [PATCH v5] qemu-char: don't issue CHR_EVENT_OPEN in a BH

[Michael Roth]

When CHR_EVENT_OPENED was initially added, it was CHR_EVENT_RESET,
and it was issued as a bottom-half:

86e94dea5b740dad65446c857f6959eae43e0ba6

Which we basically used to print out a greeting/prompt for the
monitor.

AFAICT the only reason this was ever done in a BH was because in
some cases we'd modify the chr_write handler for a new chardev
backend *after* the site where we issued the reset (see:
86e94d:qemu_chr_open_stdio())

At some point this event was renamed to CHR_EVENT_OPENED, and we've
maintained the use of this BH ever since.

However, due to 9f939df955a4152aad69a19a77e0898631bb2c18, we schedule
the BH via g_idle_add(), which is causing events to sometimes be
delivered after we've already begun processing data from backends,
leading to:

 known bugs:

  QMP:
session negotation resets with OPENED event, in some cases this
is causing new sessions to get sporadically reset

 potential bugs:

  hw/usb/redirect.c:
can_read handler checks for dev->parser != NULL, which may be
true if CLOSED BH has not been executed yet. In the past, OPENED
quiesced outstanding CLOSED events prior to us reading client
data. If it's delayed, our check may allow reads to occur even
though we haven't processed the OPENED event yet, and when we
do finally get the OPENED event, our state may get reset.

  qtest.c:
can begin session before OPENED event is processed, leading to
a spurious reset of the system and irq_levels

  gdbstub.c:
may start a gdb session prior to the machine being paused

To fix these, let's just drop the BH.

Since the initial reasoning for using it still applies to an extent,
work around that by deferring the delivery of CHR_EVENT_OPENED until
after the chardevs have been fully initialized, toward the end of
qmp_chardev_add() (or some cases, qemu_chr_new_from_opts()). This
defers delivery long enough that we can be assured a CharDriverState
is fully initialized before CHR_EVENT_OPENED is sent.

Also, rather than requiring each chardev to do an explicit open, do it
automatically, and allow the small few who don't desire such behavior to
suppress the OPENED-on-init behavior by setting a 'explicit_be_open'
flag.

We additionally add missing OPENED events for stdio backends on w32,
which were previously not being issued, causing us to not recieve the
banner and initial prompts for qmp/hmp.

Reported-by: Stefan Priebe 
Cc: qemu-sta...@nongnu.org
Signed-off-by: Michael Roth 
---
v4->v5:
 * fix segfault when using sdl/gtk console backends for
   monitor (Anthony)
 * rebased on latest origin/master

v3->v4:
 * renamed 'suppress_be_open_on_init' to 'explicit_be_open' to match
   existing 'explicit_fe_open' flag (Hans)
 * added missing 'explicit_be_open' flags for spice vmc/port and
   msmouse backends

v2->v3:
 * removed artifact in from v1 in backends/baum.c, test build with
   BRLAPI=y (Anthony)
 * rebased on latest origin/master

v1->v2:
 * default to sending OPENED on backend init, add flag to suppress
   it (Anthony)
 * fix missing OPENED for stdio backends on w32
 * fix missing OPENED when qemu_chr_new_from_opts() doesn't use
   qmp_chardev_add()
 * clean up/update commit message

 backends/baum.c   |2 --
 backends/msmouse.c|1 +
 include/sysemu/char.h |2 +-
 qemu-char.c   |   38 +-
 spice-qemu-char.c |1 +
 ui/console.c  |4 
 ui/gtk.c  |2 ++
 7 files changed, 26 insertions(+), 24 deletions(-)

diff --git a/backends/baum.c b/backends/baum.c
index 4cba79f..62aa784 100644
--- a/backends/baum.c
+++ b/backends/baum.c
@@ -611,8 +611,6 @@ CharDriverState *chr_baum_init(void)
 
 qemu_set_fd_handler(baum->brlapi_fd, baum_chr_read, NULL, baum);
 
-qemu_chr_be_generic_open(chr);
-
 return chr;
 
 fail:
diff --git a/backends/msmouse.c b/backends/msmouse.c
index 0ac05a0..c0dbfcd 100644
--- a/backends/msmouse.c
+++ b/backends/msmouse.c
@@ -70,6 +70,7 @@ CharDriverState *qemu_chr_open_msmouse(void)
 chr = g_malloc0(sizeof(CharDriverState));
 chr->chr_write = msmouse_chr_write;
 chr->chr_close = msmouse_chr_close;
+chr->explicit_be_open = true;
 
 qemu_add_mouse_event_handler(msmouse_event, chr, 0, "QEMU Microsoft 
Mouse");
 
diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index 5e42c90..066c216 100644
--- a/include/sysemu/char.h
+++ b/include/sysemu/char.h
@@ -70,12 +70,12 @@ struct CharDriverState {
 void (*chr_set_echo)(struct CharDriverState *chr, bool echo);
 void (*chr_set_fe_open)(struct CharDriverState *chr, int fe_open);
 void *opaque;
-int idle_tag;
 char *label;
 char *filename;
 int be_open;
 int fe_open;
 int explicit_fe_open;
+int explicit_be_open;
 int avail_connections;
 QemuOpts *opts;
 QTAILQ_ENTRY(CharDriverState) next;
diff --git a/qemu-char.c b/qemu-char.c
index d04b429..0b6ae95 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -110,19 +110,9 @@ void qemu_chr_be_e

Re: [Qemu-devel] [PATCH v4] qemu-char: don't issue CHR_EVENT_OPEN in a BH

[mdroth]

On Fri, Jun 07, 2013 at 11:36:16AM -0400, Luiz Capitulino wrote:
> On Fri, 7 Jun 2013 09:56:00 -0500
> mdroth  wrote:
> 
> > On Fri, Jun 07, 2013 at 09:50:39AM -0400, Luiz Capitulino wrote:
> > > On Tue,  4 Jun 2013 16:35:09 -0500
> > > Michael Roth  wrote:
> > > 
> > > > When CHR_EVENT_OPENED was initially added, it was CHR_EVENT_RESET,
> > > > and it was issued as a bottom-half:
> > > > 
> > > > 86e94dea5b740dad65446c857f6959eae43e0ba6
> > > > 
> > > > Which we basically used to print out a greeting/prompt for the
> > > > monitor.
> > > > 
> > > > AFAICT the only reason this was ever done in a BH was because in
> > > > some cases we'd modify the chr_write handler for a new chardev
> > > > backend *after* the site where we issued the reset (see:
> > > > 86e94d:qemu_chr_open_stdio())
> > > > 
> > > > At some point this event was renamed to CHR_EVENT_OPENED, and we've
> > > > maintained the use of this BH ever since.
> > > > 
> > > > However, due to 9f939df955a4152aad69a19a77e0898631bb2c18, we schedule
> > > > the BH via g_idle_add(), which is causing events to sometimes be
> > > > delivered after we've already begun processing data from backends,
> > > > leading to:
> > > > 
> > > >  known bugs:
> > > > 
> > > >   QMP:
> > > > session negotation resets with OPENED event, in some cases this
> > > > is causing new sessions to get sporadically reset
> > > > 
> > > >  potential bugs:
> > > > 
> > > >   hw/usb/redirect.c:
> > > > can_read handler checks for dev->parser != NULL, which may be
> > > > true if CLOSED BH has not been executed yet. In the past, OPENED
> > > > quiesced outstanding CLOSED events prior to us reading client
> > > > data. If it's delayed, our check may allow reads to occur even
> > > > though we haven't processed the OPENED event yet, and when we
> > > > do finally get the OPENED event, our state may get reset.
> > > > 
> > > >   qtest.c:
> > > > can begin session before OPENED event is processed, leading to
> > > > a spurious reset of the system and irq_levels
> > > > 
> > > >   gdbstub.c:
> > > > may start a gdb session prior to the machine being paused
> > > > 
> > > > To fix these, let's just drop the BH.
> > > > 
> > > > Since the initial reasoning for using it still applies to an extent,
> > > > work around that by deferring the delivery of CHR_EVENT_OPENED until
> > > > after the chardevs have been fully initialized, toward the end of
> > > > qmp_chardev_add() (or some cases, qemu_chr_new_from_opts()). This
> > > > defers delivery long enough that we can be assured a CharDriverState
> > > > is fully initialized before CHR_EVENT_OPENED is sent.
> > > > 
> > > > Also, rather than requiring each chardev to do an explicit open, do it
> > > > automatically, and allow the small few who don't desire such behavior to
> > > > suppress the OPENED-on-init behavior by setting a 'explicit_be_open'
> > > > flag.
> > > > 
> > > > We additionally add missing OPENED events for stdio backends on w32,
> > > > which were previously not being issued, causing us to not recieve the
> > > > banner and initial prompts for qmp/hmp.
> > > > 
> > > > Reported-by: Stefan Priebe 
> > > > Cc: qemu-sta...@nongnu.org
> > > > Signed-off-by: Michael Roth 
> > > 
> > > I don't know if the QMP queue is the ideal queue for this patch, but
> > > I'd happily apply it if I get at least one Reviewed-by from the CC'ed
> > > people.
> > 
> > Anthony actually added his Reviewed-by for v3, but I forgot to roll it
> > in the commit. If you do take it in through your tree can you add that?
> 
> Sorry for the bureaucracy, but I don't like to add other people's tags
> when the patch changes. Even when I'm sure they would be OK with the
> new version.

Good call, v5 incoming with gtk/sdl that fixes Anthony caught

> 
> Anthony, can you please add your Reviewed-by again?
> 
> > 
> > Thanks!
> > 
> > > 
> > > 
> > > > ---
> > > > v3->v4:
> > > >  * renamed 'suppress_be_open_on_init' to 'explicit_be_open' to match
> > > >existing 'explicit_fe_open' flag (Hans)
> > > >  * added missing 'explicit_be_open' flags for spice vmc/port and
> > > >msmouse backends
> > > > 
> > > > v2->v3:
> > > >  * removed artifact in from v1 in backends/baum.c, test build with
> > > >BRLAPI=y (Anthony)
> > > >  * rebased on latest origin/master
> > > > 
> > > > v1->v2:
> > > >  * default to sending OPENED on backend init, add flag to suppress
> > > >it (Anthony)
> > > >  * fix missing OPENED for stdio backends on w32
> > > >  * fix missing OPENED when qemu_chr_new_from_opts() doesn't use
> > > >qmp_chardev_add()
> > > >  * clean up/update commit message
> > > > 
> > > >  backends/baum.c   |2 --
> > > >  backends/msmouse.c|1 +
> > > >  include/sysemu/char.h |2 +-
> > > >  qemu-char.c   |   38 +-
> > > >  spice-qemu-char.c |1 +
> > > >  ui/console.c  |1 -
> > > >  ui/gtk.c  |1 -
> > > >  7 files cha

[Qemu-devel] [PATCH 2/9] rng-random: use error_setg_file_open()

[Luiz Capitulino]

Signed-off-by: Luiz Capitulino 
---
 backends/rng-random.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/backends/rng-random.c b/backends/rng-random.c
index 830360c..68dfc8a 100644
--- a/backends/rng-random.c
+++ b/backends/rng-random.c
@@ -78,9 +78,8 @@ static void rng_random_opened(RngBackend *b, Error **errp)
   "filename", "a valid filename");
 } else {
 s->fd = qemu_open(s->filename, O_RDONLY | O_NONBLOCK);
-
 if (s->fd == -1) {
-error_set(errp, QERR_OPEN_FILE_FAILED, s->filename);
+error_setg_file_open(errp, errno, s->filename);
 }
 }
 }
-- 
1.8.1.4

[Qemu-devel] [PULL 2/3] MAINTAINERS: new maintainers for qapi-schema.json

[Luiz Capitulino]

I'm facing two problems lately wrt QMP patch review: increasingly
lack of bandwidth and lack of background in so many different areas
that are getting new QMP commands almost every week.

In order to help me mitigate this problem, I'm adding Eric and Markus
(besides me) as maintainers of the qapi-schema.json file.

Markus has been an old timer reviewer. Eric is being the most active
and prolific reviewer of QMP patches for some time now.

I believe Markus and Eric will keep doing their work as before, but
starting now I'll require the ACK of at least one of them before
appling a patch/series that touches the qapi-schema.json file.

Signed-off-by: Luiz Capitulino 
Reviewed-by: Eric Blake 
Acked-by: Markus Armbruster 
---
 MAINTAINERS | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index be02724..432185a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -706,6 +706,13 @@ F: nbd.*
 F: qemu-nbd.c
 T: git git://github.com/bonzini/qemu.git nbd-next
 
+QAPI Schema
+M: Eric Blake 
+M: Luiz Capitulino 
+M: Markus Armbruster 
+S: Supported
+F: qapi-schema.json
+
 SLIRP
 M: Jan Kiszka 
 S: Maintained
-- 
1.8.1.4

[Qemu-devel] [PATCH 9/9] qerror: drop QERR_OPEN_FILE_FAILED macro

[Luiz Capitulino]

Not used since the last commit.

Signed-off-by: Luiz Capitulino 
---
 include/qapi/qmp/qerror.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index 6c0a18d..c30c2f6 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -177,9 +177,6 @@ void assert_no_error(Error *err);
 #define QERR_NOT_SUPPORTED \
 ERROR_CLASS_GENERIC_ERROR, "Not supported"
 
-#define QERR_OPEN_FILE_FAILED \
-ERROR_CLASS_GENERIC_ERROR, "Could not open '%s'"
-
 #define QERR_PERMISSION_DENIED \
 ERROR_CLASS_GENERIC_ERROR, "Insufficient permission to perform this 
operation"
 
-- 
1.8.1.4

[Qemu-devel] [PATCH 8/9] savevm: qmp_xen_save_devices_state(): use error_setg_file_open()

[Luiz Capitulino]

Signed-off-by: Luiz Capitulino 
---
 savevm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/savevm.c b/savevm.c
index 2ce439f..ff5ece6 100644
--- a/savevm.c
+++ b/savevm.c
@@ -2410,7 +2410,7 @@ void qmp_xen_save_devices_state(const char *filename, 
Error **errp)
 
 f = qemu_fopen(filename, "wb");
 if (!f) {
-error_set(errp, QERR_OPEN_FILE_FAILED, filename);
+error_setg_file_open(errp, errno, filename);
 goto the_end;
 }
 ret = qemu_save_device_state(f);
-- 
1.8.1.4

[Qemu-devel] [PATCH 7/9] dump: qmp_dump_guest_memory(): use error_setg_file_open()

[Luiz Capitulino]

Signed-off-by: Luiz Capitulino 
---
 dump.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dump.c b/dump.c
index c0d3da5..28fd296 100644
--- a/dump.c
+++ b/dump.c
@@ -847,7 +847,7 @@ void qmp_dump_guest_memory(bool paging, const char *file, 
bool has_begin,
 if  (strstart(file, "file:", &p)) {
 fd = qemu_open(p, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, S_IRUSR);
 if (fd < 0) {
-error_set(errp, QERR_OPEN_FILE_FAILED, p);
+error_setg_file_open(errp, errno, p);
 return;
 }
 }
-- 
1.8.1.4

[Qemu-devel] [PATCH 6/9] cpus: use error_setg_file_open()

[Luiz Capitulino]

Signed-off-by: Luiz Capitulino 
---
 cpus.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/cpus.c b/cpus.c
index c232265..c8bc8ad 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1278,7 +1278,7 @@ void qmp_memsave(int64_t addr, int64_t size, const char 
*filename,
 
 f = fopen(filename, "wb");
 if (!f) {
-error_set(errp, QERR_OPEN_FILE_FAILED, filename);
+error_setg_file_open(errp, errno, filename);
 return;
 }
 
@@ -1308,7 +1308,7 @@ void qmp_pmemsave(int64_t addr, int64_t size, const char 
*filename,
 
 f = fopen(filename, "wb");
 if (!f) {
-error_set(errp, QERR_OPEN_FILE_FAILED, filename);
+error_setg_file_open(errp, errno, filename);
 return;
 }
 
-- 
1.8.1.4

[Qemu-devel] [PULL 1/3] correct RTC_CHANGE_EVENT description

[Luiz Capitulino]

From: Marcelo Tosatti 

Fix RTC_CHANGE event description to match implementation.

Signed-off-by: Marcelo Tosatti 
Signed-off-by: Luiz Capitulino 
---
 QMP/qmp-events.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
index 92fe5fb..00b4087 100644
--- a/QMP/qmp-events.txt
+++ b/QMP/qmp-events.txt
@@ -203,7 +203,8 @@ Emitted when the guest changes the RTC time.
 
 Data:
 
-- "offset": delta against the host UTC in seconds (json-number)
+- "offset": Offset between old RTC clock value and new one
+in seconds (json-number)
 
 Example:
 
-- 
1.8.1.4

[Qemu-devel] [PATCH 5/9] blockdev: use error_setg_file_open()

[Luiz Capitulino]

Signed-off-by: Luiz Capitulino 
---
 blockdev.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 9937311..c09adba 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -899,7 +899,7 @@ static void external_snapshot_prepare(BlkTransactionStates 
*common,
 ret = bdrv_open(states->new_bs, new_image_file, NULL,
 flags | BDRV_O_NO_BACKING, drv);
 if (ret != 0) {
-error_set(errp, QERR_OPEN_FILE_FAILED, new_image_file);
+error_setg_file_open(errp, errno, new_image_file);
 }
 }
 
@@ -1063,7 +1063,7 @@ static void qmp_bdrv_open_encrypted(BlockDriverState *bs, 
const char *filename,
 const char *password, Error **errp)
 {
 if (bdrv_open(bs, filename, NULL, bdrv_flags, drv) < 0) {
-error_set(errp, QERR_OPEN_FILE_FAILED, filename);
+error_setg_file_open(errp, errno, filename);
 return;
 }
 
@@ -1483,7 +1483,7 @@ void qmp_drive_mirror(const char *device, const char 
*target,
 
 if (ret < 0) {
 bdrv_delete(target_bs);
-error_set(errp, QERR_OPEN_FILE_FAILED, target);
+error_setg_file_open(errp, errno, target);
 return;
 }
 
-- 
1.8.1.4

[Qemu-devel] [PULL 0/3] QMP queue

[Luiz Capitulino]

The changes (since 7387de16d0e4d2988df350926537cd12a8e34206) are available
in the following repository:

git://repo.or.cz/qemu/qmp-unstable.git queue/qmp

Luiz Capitulino (2):
  MAINTAINERS: new maintainers for qapi-schema.json
  MAINTAINERS: split Monitor (QMP/HMP) entry

Marcelo Tosatti (1):
  correct RTC_CHANGE_EVENT description

 MAINTAINERS| 26 --
 QMP/qmp-events.txt |  3 ++-
 2 files changed, 26 insertions(+), 3 deletions(-)

-- 
1.8.1.4

[Qemu-devel] [PULL 3/3] MAINTAINERS: split Monitor (QMP/HMP) entry

[Luiz Capitulino]

This entry doesn't reflect reality for a few years now. This commit
splits it into Human Monitor (HMP), QAPI and QMP. Markus is dropped
as a maintainer.

This is what we have been for the last few years. Also, it's going
to help me to offload some of this work to someone else in the near
future.

Signed-off-by: Luiz Capitulino 
Acked-by: Markus Armbruster 
---
 MAINTAINERS | 19 +--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 432185a..13c0cc5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -685,11 +685,12 @@ M: Anthony Liguori 
 S: Supported
 F: vl.c
 
-Monitor (QMP/HMP)
+Human Monitor (HMP)
 M: Luiz Capitulino 
-M: Markus Armbruster 
 S: Supported
 F: monitor.c
+F: hmp.c
+F: hmp-commands.hx
 
 Network device layer
 M: Anthony Liguori 
@@ -706,6 +707,12 @@ F: nbd.*
 F: qemu-nbd.c
 T: git git://github.com/bonzini/qemu.git nbd-next
 
+QAPI
+M: Luiz Capitulino 
+M: Michael Roth 
+S: Supported
+F: qapi/
+
 QAPI Schema
 M: Eric Blake 
 M: Luiz Capitulino 
@@ -713,6 +720,14 @@ M: Markus Armbruster 
 S: Supported
 F: qapi-schema.json
 
+QMP
+M: Luiz Capitulino 
+S: Supported
+F: qmp.c
+F: monitor.c
+F: qmp-commands.hx
+F: QMP/
+
 SLIRP
 M: Jan Kiszka 
 S: Maintained
-- 
1.8.1.4

[Qemu-devel] [PATCH 3/9] block: bdrv_reopen_prepare(): use error_setg_file_open()

[Luiz Capitulino]

Signed-off-by: Luiz Capitulino 
---
 block.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/block.c b/block.c
index 79ad33d..c78f152 100644
--- a/block.c
+++ b/block.c
@@ -1291,8 +1291,7 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, 
BlockReopenQueue *queue,
 if (local_err != NULL) {
 error_propagate(errp, local_err);
 } else {
-error_set(errp, QERR_OPEN_FILE_FAILED,
-  reopen_state->bs->filename);
+error_setg_file_open(errp, errno, reopen_state->bs->filename);
 }
 goto error;
 }
-- 
1.8.1.4

[Qemu-devel] [PATCH 0/9] QMP/HMP: add error reason to open failures

[Luiz Capitulino]

Today I was surprised to find out that we still have old users of 
QERR_OPEN_FILE_FAILED that print errors like:

(qemu) dump-guest-memory -p /lkmads/foo
Could not open '/lkmads/foo'
(qemu)

This series converts all those users to a new helper called 
error_setg_file_open(), which adds error reason to open failures:

(qemu) dump-guest-memory -p /sfmdkjf/foo
Could not open '/sfmdkjf/foo': No such file or directory
(qemu) 

Luiz Capitulino (9):
  error: add error_setg_file_open() helper
  rng-random: use error_setg_file_open()
  block: bdrv_reopen_prepare(): use error_setg_file_open()
  block: mirror_complete(): use error_setg_file_open()
  blockdev: use error_setg_file_open()
  cpus: use error_setg_file_open()
  dump: qmp_dump_guest_memory(): use error_setg_file_open()
  savevm: qmp_xen_save_devices_state(): use error_setg_file_open()
  qerror: drop QERR_OPEN_FILE_FAILED macro

 backends/rng-random.c | 3 +--
 block.c   | 3 +--
 block/mirror.c| 2 +-
 blockdev.c| 6 +++---
 cpus.c| 4 ++--
 dump.c| 2 +-
 include/qapi/error.h  | 5 +
 include/qapi/qmp/qerror.h | 3 ---
 savevm.c  | 2 +-
 util/error.c  | 5 +
 10 files changed, 20 insertions(+), 15 deletions(-)

-- 
1.8.1.4

[Qemu-devel] [PATCH 1/9] error: add error_setg_file_open() helper

[Luiz Capitulino]

Signed-off-by: Luiz Capitulino 
---
 include/qapi/error.h | 5 +
 util/error.c | 5 +
 2 files changed, 10 insertions(+)

diff --git a/include/qapi/error.h b/include/qapi/error.h
index 5cd2f0c..ffd1cea 100644
--- a/include/qapi/error.h
+++ b/include/qapi/error.h
@@ -45,6 +45,11 @@ void error_set_errno(Error **err, int os_error, ErrorClass 
err_class, const char
 error_set_errno(err, os_error, ERROR_CLASS_GENERIC_ERROR, fmt, ## 
__VA_ARGS__)
 
 /**
+ * Helper for open() errors
+ */
+void error_setg_file_open(Error **errp, int os_errno, const char *filename);
+
+/**
  * Returns true if an indirect pointer to an error is pointing to a valid
  * error object.
  */
diff --git a/util/error.c b/util/error.c
index 519f6b6..53b0435 100644
--- a/util/error.c
+++ b/util/error.c
@@ -71,6 +71,11 @@ void error_set_errno(Error **errp, int os_errno, ErrorClass 
err_class,
 *errp = err;
 }
 
+void error_setg_file_open(Error **errp, int os_errno, const char *filename)
+{
+error_setg_errno(errp, os_errno, "Could not open '%s'", filename);
+}
+
 Error *error_copy(const Error *err)
 {
 Error *err_new;
-- 
1.8.1.4

[Qemu-devel] [PATCH 4/9] block: mirror_complete(): use error_setg_file_open()

[Luiz Capitulino]

Signed-off-by: Luiz Capitulino 
---
 block/mirror.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/mirror.c b/block/mirror.c
index 8b07dec..89d531d 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -512,7 +512,7 @@ static void mirror_complete(BlockJob *job, Error **errp)
 char backing_filename[PATH_MAX];
 bdrv_get_full_backing_filename(s->target, backing_filename,
sizeof(backing_filename));
-error_set(errp, QERR_OPEN_FILE_FAILED, backing_filename);
+error_setg_file_open(errp, errno, backing_filename);
 return;
 }
 if (!s->synced) {
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH] script: git script to compile every commit in a range of commits

[Jeff Cody]

On Sat, Jun 08, 2013 at 02:36:33AM +1000, Peter Crosthwaite wrote:
> Hi Jeff,
> 
> On Sat, Jun 1, 2013 at 2:39 AM, Jeff Cody  wrote:
> > This is a git script that will iterate through every commit in a
> > specified range, and perform a configure and make.  The intention of
> > this script is not to act as a check of code correctness, but to see if
> > any commit breaks compilation of the tree.
> >
> 
> Should we possibly throw a checkpatch in there?
> 

Hi Peter,

That could be interesting, but I'd rather keep the patch more
single-purpose, focused on checking compile errors rather than also
patch formatting / code style checks.  I'm afraid the output would get
too complicated, and the purpose of the script itself unclear.

> > The idea is that prior to submitting a patch or patch series, the
> > submitter should verify that no patch in the series breaks the build,
> > thereby breaking git-bisect.  This script may also be useful for
> > reviewers/maintainers dealing with multi-patch series.
> >
> > The range passed in to the script is in the form of a standard git range;
> > the default is HEAD^!  (i.e. just the latest commit).
> >
> > Options may be passed in for configure, as well as make.  The log output
> > filename and directory may also be optionally specified.  All options
> > are stored via git-config.
> >
> > If everything compiled without error, then the exit code from the
> > program will be 0.  Upon error, the stats for the offending
> > commit will be shown.
> >
> > Signed-off-by: Jeff Cody 
> > ---
> >  scripts/git-compile-check | 202 
> > ++
> >  1 file changed, 202 insertions(+)
> >  create mode 100755 scripts/git-compile-check
> >
> > diff --git a/scripts/git-compile-check b/scripts/git-compile-check
> > new file mode 100755
> > index 000..4b90f91
> > --- /dev/null
> > +++ b/scripts/git-compile-check
> > @@ -0,0 +1,202 @@
> > +#!/bin/bash
> > +#
> > +# See $desc, below, for program description
> > +#
> > +# Copyright (c) 2013 Red Hat, Inc.
> > +#
> > +# Author(s):
> > +#   Jeff Cody 
> > +#
> > +# This program is free software; you can redistribute it and/or modify it 
> > under
> > +# the terms of the GNU General Public License as published by the Free 
> > Software
> > +# Foundation; under version 2 of the license
> > +#
> > +# This program is distributed in the hope that it will be useful, but 
> > WITHOUT
> > +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or 
> > FITNESS
> > +# FOR A PARTICULAR PURPOSE.  See the GNU General Public License for more
> > +# details.
> > +#
> > +# You should have received a copy of the GNU General Public License along 
> > with
> > +# this program; if not, see .
> > +#
> > +
> > +set -C -u -e
> > +set -o pipefail
> > +
> > +desc="
> > +$0 iterates through a git commit range, and performs the
> > +following on each commit:
> > +  - git checkout
> > +  - make clean
> > +  - configure
> > +  - make
> > +
> > +It will also optionally perform a git-reset and git-clean between
> > +checkouts, if requested via the '-f' option.
> > +
> > +The script will exit and report on first error on any of the above steps,
> > +(except no error checking is performed on 'make clean')
> > +
> > +NOTE: While executing, the script will checkout out each commit
> > +  in the range in the current git tree.  On exit, the HEAD
> > +  at the time the script was called is checked out"
> > +
> > +
> > +# default range is the last commit
> > +def_range="HEAD^!"
> > +def_config_opt="--target-list=x86_64-softmmu"
> > +# you may want to have make perform multiple jobs, e.g. -j4
> > +# this is ommitted as the default in case the project makefile
> > +# is not safe for parallel make processes
> > +def_make_opt=""
> > +def_log="output-$$.log"
> > +def_logdir=""
> > +force_clean='n'
> > +
> > +logfile=$def_log
> > +range=`git config compile-check.range || true`
> > +config_opt=`git config compile-check.configopt || true`
> > +make_opt=`git config compile-check.makeopt || true`
> > +logdir=`git config compile-check.logdir || true`
> > +
> > +if [[ -z "$range" ]]
> > +then
> > +range=$def_range
> > +git config compile-check.range $range || true
> > +fi
> > +if [[ -z "$config_opt" ]]
> > +then
> > +config_opt=$def_config_opt
> > +git config compile-check.configopt $config_opt || true
> > +fi
> > +if [[ -z "$make_opt" ]]
> > +then
> > +make_opt=$def_make_opt
> > +git config compile-check.makeopt $make_opt || true
> > +fi
> > +if [[ -z "$logdir" ]]
> > +then
> > +logdir=$def_logdir
> > +git config compile-check.logdir $logdir || true
> > +fi
> > +
> > +usage() {
> > +echo ""
> > +echo "$0 [OPTIONS]"
> > +echo "$desc"
> > +echo ""
> > +echo "OPTIONS:"
> > +echo "  -r git range
> > +optional; default is '$range'
> > +"
> > +echo "  -c configure options
> > +optional; defa

[Qemu-devel] [PATCH] gtk: use better icon

[Anthony Liguori]

The current icon looks pretty terrible rendered in Gnome.  This
switches to a transparent SVG which looks much nicer.

Signed-off-by: Anthony Liguori 
---
 Makefile  |   2 +-
 pc-bios/qemu_logo_no_text.svg | 976 ++
 ui/gtk.c  |   2 +-
 3 files changed, 978 insertions(+), 2 deletions(-)
 create mode 100644 pc-bios/qemu_logo_no_text.svg

diff --git a/Makefile b/Makefile
index 9a77ae0..306e7bd 100644
--- a/Makefile
+++ b/Makefile
@@ -289,7 +289,7 @@ pxe-e1000.rom pxe-eepro100.rom pxe-ne2k_pci.rom \
 pxe-pcnet.rom pxe-rtl8139.rom pxe-virtio.rom \
 efi-e1000.rom efi-eepro100.rom efi-ne2k_pci.rom \
 efi-pcnet.rom efi-rtl8139.rom efi-virtio.rom \
-qemu-icon.bmp \
+qemu-icon.bmp qemu_logo_no_text.svg \
 bamboo.dtb petalogix-s3adsp1800.dtb petalogix-ml605.dtb \
 multiboot.bin linuxboot.bin kvmvapic.bin \
 s390-zipl.rom \
diff --git a/pc-bios/qemu_logo_no_text.svg b/pc-bios/qemu_logo_no_text.svg
new file mode 100644
index 000..24ca23a
--- /dev/null
+++ b/pc-bios/qemu_logo_no_text.svg
@@ -0,0 +1,976 @@
+
+
+
+http://purl.org/dc/elements/1.1/";
+   xmlns:cc="http://creativecommons.org/ns#";
+   xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#";
+   xmlns:svg="http://www.w3.org/2000/svg";
+   xmlns="http://www.w3.org/2000/svg";
+   xmlns:xlink="http://www.w3.org/1999/xlink";
+   xmlns:sodipodi="http://sodipodi.sourceforge.net/DTD/sodipodi-0.dtd";
+   xmlns:inkscape="http://www.inkscape.org/namespaces/inkscape";
+   width="111.71874"
+   height="111.12498"
+   id="svg2"
+   version="1.1"
+   inkscape:version="0.48.2 r9819"
+   sodipodi:docname="qemu_logo_no_text.svg">
+  
+
+  
+  
+  
+  
+  
+  
+
+
+  
+  
+
+
+  
+  
+
+
+  
+  
+
+
+  
+  
+
+
+  
+  
+
+
+  
+  
+
+
+  
+  
+
+
+  
+  
+
+
+  
+  
+
+
+  
+  
+
+
+  
+  
+
+
+  
+  
+
+
+  
+  
+
+
+  
+  
+
+
+  
+  
+
+
+  
+  
+
+
+  
+  
+
+
+
+  
+  
+
+
+  
+  
+
+
+
+
+  
+  
+
+
+
+  
+  
+
+
+
+  
+  
+
+
+
+  
+  
+
+
+
+
+  
+  
+
+
+
+  
+  
+
+
+
+  
+  
+
+
+
+  
+  
+
+
+
+  
+  
+
+
+
+  
+  
+
+
+  
+  
+
+
+  
+  
+
+
+  
+  
+
+
+  
+  
+
+
+  
+  
+
+
+  
+  
+
+
+  
+  
+
+
+  
+  
+
+
+  
+  
+
+
+  
+  
+
+
+  
+  
+
+
+  
+  
+
+
+
+
+
+  
+  
+
+
+
+  
+  
+
+
+  
+  
+
+
+  
+  
+
+
+  
+  
+
+
+  
+  
+
+
+
+
+
+  
+  
+
+
+
+  
+  
+
+
+
+  
+  
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+  
+  
+
+  
+image/svg+xml
+http://purl.org/dc/dcmitype/StillImage"; />
+
+  
+
+  
+  
+
+
+
+
+
+
+  
+
diff --git a/ui/gtk.c b/ui/gtk.c
index 3bc2842..efd7836 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -1469,7 +1469,7 @@ void gtk_display_init(DisplayState *ds)
 
 gtk_notebook_append_page(GTK_NOTEBOOK(s->notebook), s->drawing_area, 
gtk_label_new("VGA"));
 
-filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, "qemu-icon.bmp");
+filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, "qemu_logo_no_text.svg");
 if (filename) {
 GError *error = NULL;
 GdkPixbuf *pixbuf = gdk_pixbuf_new_from_file(filename, &error);
-- 
1.8.0

[Qemu-devel] [PATCH RFT 5/5] virtio-serial-port: Convert to QOM realize/unrealize

[Andreas Färber]

Note: virtconsole's/virtserialport's realizefn now registers its
handlers before VirtIOSerialPort's realizefn.

Signed-off-by: Andreas Färber 
---
 hw/char/virtio-console.c  | 71 ---
 hw/char/virtio-serial-bus.c   | 45 ++---
 include/hw/virtio/virtio-serial.h | 11 --
 3 files changed, 68 insertions(+), 59 deletions(-)

diff --git a/hw/char/virtio-console.c b/hw/char/virtio-console.c
index 73e18f2..ab8b902 100644
--- a/hw/char/virtio-console.c
+++ b/hw/char/virtio-console.c
@@ -18,6 +18,10 @@
 #define TYPE_VIRTIO_CONSOLE "virtconsole"
 #define VIRTIO_CONSOLE(obj) \
 OBJECT_CHECK(VirtConsole, (obj), TYPE_VIRTIO_CONSOLE)
+#define VIRTIO_CONSOLE_GET_CLASS(obj) \
+OBJECT_GET_CLASS(VirtConsoleClass, (obj), TYPE_VIRTIO_CONSOLE)
+#define VIRTIO_CONSOLE_CLASS(cls) \
+OBJECT_CLASS_CHECK(VirtConsoleClass, (cls), TYPE_VIRTIO_CONSOLE)
 
 typedef struct VirtConsole {
 VirtIOSerialPort parent_obj;
@@ -26,6 +30,13 @@ typedef struct VirtConsole {
 guint watch;
 } VirtConsole;
 
+typedef struct VirtConsoleClass {
+VirtIOSerialPortClass parent_class;
+
+DeviceRealize parent_realize;
+DeviceUnrealize parent_unrealize;
+} VirtConsoleClass;
+
 /*
  * Callback function that's called from chardevs when backend becomes
  * writable.
@@ -126,14 +137,17 @@ static void chr_event(void *opaque, int event)
 }
 }
 
-static int virtconsole_initfn(VirtIOSerialPort *port)
+static void virtconsole_realize(DeviceState *dev, Error **errp)
 {
-VirtConsole *vcon = VIRTIO_CONSOLE(port);
-VirtIOSerialPortClass *k = VIRTIO_SERIAL_PORT_GET_CLASS(port);
+VirtIOSerialPort *port = VIRTIO_SERIAL_PORT(dev);
+VirtConsole *vcon = VIRTIO_CONSOLE(dev);
+VirtIOSerialPortClass *k = VIRTIO_SERIAL_PORT_GET_CLASS(dev);
+VirtConsoleClass *vcc = VIRTIO_CONSOLE_GET_CLASS(dev);
 
 if (port->id == 0 && !k->is_console) {
-error_report("Port number 0 on virtio-serial devices reserved for 
virtconsole devices for backward compatibility.");
-return -1;
+error_setg(errp, "Port number 0 on virtio-serial devices reserved "
+   "for virtconsole devices for backward compatibility.");
+return;
 }
 
 if (vcon->chr) {
@@ -142,18 +156,24 @@ static int virtconsole_initfn(VirtIOSerialPort *port)
   vcon);
 }
 
-return 0;
+vcc->parent_realize(dev, errp);
 }
 
-static int virtconsole_exitfn(VirtIOSerialPort *port)
+static void virtconsole_unrealize(DeviceState *dev, Error **errp)
 {
-VirtConsole *vcon = VIRTIO_CONSOLE(port);
+VirtConsole *vcon = VIRTIO_CONSOLE(dev);
+VirtConsoleClass *vcc = VIRTIO_CONSOLE_GET_CLASS(dev);
+Error *err = NULL;
+
+vcc->parent_unrealize(dev, &err);
+if (err != NULL) {
+error_propagate(errp, err);
+return;
+}
 
 if (vcon->watch) {
 g_source_remove(vcon->watch);
 }
-
-return 0;
 }
 
 static Property virtconsole_properties[] = {
@@ -161,14 +181,19 @@ static Property virtconsole_properties[] = {
 DEFINE_PROP_END_OF_LIST(),
 };
 
-static void virtconsole_class_init(ObjectClass *klass, void *data)
+static void virtconsole_class_init(ObjectClass *oc, void *data)
 {
-DeviceClass *dc = DEVICE_CLASS(klass);
-VirtIOSerialPortClass *k = VIRTIO_SERIAL_PORT_CLASS(klass);
+DeviceClass *dc = DEVICE_CLASS(oc);
+VirtIOSerialPortClass *k = VIRTIO_SERIAL_PORT_CLASS(oc);
+VirtConsoleClass *vcc = VIRTIO_CONSOLE_CLASS(oc);
+
+vcc->parent_realize = dc->realize;
+dc->realize = virtconsole_realize;
+
+vcc->parent_unrealize = dc->unrealize;
+dc->unrealize = virtconsole_unrealize;
 
 k->is_console = true;
-k->init = virtconsole_initfn;
-k->exit = virtconsole_exitfn;
 k->have_data = flush_buf;
 k->set_guest_connected = set_guest_connected;
 dc->props = virtconsole_properties;
@@ -179,6 +204,7 @@ static const TypeInfo virtconsole_info = {
 .parent= TYPE_VIRTIO_SERIAL_PORT,
 .instance_size = sizeof(VirtConsole),
 .class_init= virtconsole_class_init,
+.class_size= sizeof(VirtConsoleClass),
 };
 
 static Property virtserialport_properties[] = {
@@ -186,13 +212,18 @@ static Property virtserialport_properties[] = {
 DEFINE_PROP_END_OF_LIST(),
 };
 
-static void virtserialport_class_init(ObjectClass *klass, void *data)
+static void virtserialport_class_init(ObjectClass *oc, void *data)
 {
-DeviceClass *dc = DEVICE_CLASS(klass);
-VirtIOSerialPortClass *k = VIRTIO_SERIAL_PORT_CLASS(klass);
+DeviceClass *dc = DEVICE_CLASS(oc);
+VirtIOSerialPortClass *k = VIRTIO_SERIAL_PORT_CLASS(oc);
+VirtConsoleClass *vcc = VIRTIO_CONSOLE_CLASS(oc);
+
+vcc->parent_realize = dc->realize;
+dc->realize = virtconsole_realize;
+
+vcc->parent_unrealize = dc->unrealize;
+dc->unrealize = virtconsole_unrealize;
 
-k->init = virtconsole_initfn;
-k->exit = virtconsole_exitfn;
 k->have_

[Qemu-devel] [PATCH RFT 3/5] virtio-console: QOM'ify VirtConsole

[Andreas Färber]

Introduce type constant, cast macro and rename parent field.

Signed-off-by: Andreas Färber 
---
 hw/char/virtio-console.c | 33 -
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/hw/char/virtio-console.c b/hw/char/virtio-console.c
index 6759e51..7b1a382 100644
--- a/hw/char/virtio-console.c
+++ b/hw/char/virtio-console.c
@@ -15,8 +15,13 @@
 #include "trace.h"
 #include "hw/virtio/virtio-serial.h"
 
+#define TYPE_VIRTIO_CONSOLE "virtconsole"
+#define VIRTIO_CONSOLE(obj) \
+OBJECT_CHECK(VirtConsole, (obj), TYPE_VIRTIO_CONSOLE)
+
 typedef struct VirtConsole {
-VirtIOSerialPort port;
+VirtIOSerialPort parent_obj;
+
 CharDriverState *chr;
 guint watch;
 } VirtConsole;
@@ -31,7 +36,7 @@ static gboolean chr_write_unblocked(GIOChannel *chan, 
GIOCondition cond,
 VirtConsole *vcon = opaque;
 
 vcon->watch = 0;
-virtio_serial_throttle_port(&vcon->port, false);
+virtio_serial_throttle_port(VIRTIO_SERIAL_PORT(vcon), false);
 return FALSE;
 }
 
@@ -39,7 +44,7 @@ static gboolean chr_write_unblocked(GIOChannel *chan, 
GIOCondition cond,
 static ssize_t flush_buf(VirtIOSerialPort *port,
  const uint8_t *buf, ssize_t len)
 {
-VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
+VirtConsole *vcon = VIRTIO_CONSOLE(port);
 ssize_t ret;
 
 if (!vcon->chr) {
@@ -75,7 +80,7 @@ static ssize_t flush_buf(VirtIOSerialPort *port,
 /* Callback function that's called when the guest opens/closes the port */
 static void set_guest_connected(VirtIOSerialPort *port, int guest_connected)
 {
-VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
+VirtConsole *vcon = VIRTIO_CONSOLE(port);
 
 if (!vcon->chr) {
 return;
@@ -88,40 +93,42 @@ static int chr_can_read(void *opaque)
 {
 VirtConsole *vcon = opaque;
 
-return virtio_serial_guest_ready(&vcon->port);
+return virtio_serial_guest_ready(VIRTIO_SERIAL_PORT(vcon));
 }
 
 /* Send data from a char device over to the guest */
 static void chr_read(void *opaque, const uint8_t *buf, int size)
 {
 VirtConsole *vcon = opaque;
+VirtIOSerialPort *port = VIRTIO_SERIAL_PORT(vcon);
 
-trace_virtio_console_chr_read(vcon->port.id, size);
-virtio_serial_write(&vcon->port, buf, size);
+trace_virtio_console_chr_read(port->id, size);
+virtio_serial_write(port, buf, size);
 }
 
 static void chr_event(void *opaque, int event)
 {
 VirtConsole *vcon = opaque;
+VirtIOSerialPort *port = VIRTIO_SERIAL_PORT(vcon);
 
-trace_virtio_console_chr_event(vcon->port.id, event);
+trace_virtio_console_chr_event(port->id, event);
 switch (event) {
 case CHR_EVENT_OPENED:
-virtio_serial_open(&vcon->port);
+virtio_serial_open(port);
 break;
 case CHR_EVENT_CLOSED:
 if (vcon->watch) {
 g_source_remove(vcon->watch);
 vcon->watch = 0;
 }
-virtio_serial_close(&vcon->port);
+virtio_serial_close(port);
 break;
 }
 }
 
 static int virtconsole_initfn(VirtIOSerialPort *port)
 {
-VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
+VirtConsole *vcon = VIRTIO_CONSOLE(port);
 VirtIOSerialPortClass *k = VIRTIO_SERIAL_PORT_GET_CLASS(port);
 
 if (port->id == 0 && !k->is_console) {
@@ -140,7 +147,7 @@ static int virtconsole_initfn(VirtIOSerialPort *port)
 
 static int virtconsole_exitfn(VirtIOSerialPort *port)
 {
-VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
+VirtConsole *vcon = VIRTIO_CONSOLE(port);
 
 if (vcon->watch) {
 g_source_remove(vcon->watch);
@@ -168,7 +175,7 @@ static void virtconsole_class_init(ObjectClass *klass, void 
*data)
 }
 
 static const TypeInfo virtconsole_info = {
-.name  = "virtconsole",
+.name  = TYPE_VIRTIO_CONSOLE,
 .parent= TYPE_VIRTIO_SERIAL_PORT,
 .instance_size = sizeof(VirtConsole),
 .class_init= virtconsole_class_init,
-- 
1.8.1.4

[Qemu-devel] [PATCH RFT 0/5] QOM realize for virtio

[Andreas Färber]

Hello,

This series converts virtio devices to QOM realize/unrealize.
It is intended as base for fixing virtio-net initialization order issues,
as reported by Jesse. Only partially tested though.

Note that while VirtioDevice was setting a DeviceClass::exit callback
for cleaning up the bus name, this was overwritten by most derived classes.
That is fixed as part of this conversion.

Similarly, virtio_scsi_common_{init,exit} can be moved to VirtIOSCSICommon now.
This has the side-effect that the two SCSI subclasses now perform some
initializations after the common SCSI implementation has invoked
virtio_bus_plug_device().

As a follow-up, VirtIOSerialPort is also converted to QOM realize/unrealize.
As a side-effect, virtio-console realization is changed from in-order to 
pre-order.

Incidentally I stumbled over a minor cleanup issue with virtserialport.

Available from:
https://github.com/afaerber/qemu-cpu/commits/realize-virtio.v1
git://github.com/afaerber/qemu-cpu.git realize-virtio.v1

Regards,
Andreas

Cc: Anthony Liguori 
Cc: Paolo Bonzini 
Cc: Michael S. Tsirkin 
Cc: Jesse Larrew 
Cc: Frederic Konrad 

Andreas Färber (5):
  virtio-blk-dataplane: Improve error reporting
  virtio: Convert VirtioDevice to QOM realize/unrealize
  virtio-console: QOM'ify VirtConsole
  virtio-console: Use exitfn for virtserialport, too
  virtio-serial-port: Convert to QOM realize/unrealize

 hw/9pfs/virtio-9p-device.c | 67 ++
 hw/9pfs/virtio-9p.h| 13 +
 hw/block/dataplane/virtio-blk.c| 25 +-
 hw/block/dataplane/virtio-blk.h|  5 +-
 hw/block/virtio-blk.c  | 56 +
 hw/char/virtio-console.c   | 99 ++
 hw/char/virtio-serial-bus.c| 94 ++--
 hw/net/virtio-net.c| 48 ++
 hw/scsi/vhost-scsi.c   | 59 +--
 hw/scsi/virtio-scsi.c  | 85 
 hw/virtio/virtio-balloon.c | 50 +++
 hw/virtio/virtio-rng.c | 53 +++-
 hw/virtio/virtio.c | 20 +++-
 include/hw/virtio/vhost-scsi.h | 13 +
 include/hw/virtio/virtio-balloon.h | 13 +
 include/hw/virtio/virtio-blk.h | 13 +
 include/hw/virtio/virtio-net.h | 13 +
 include/hw/virtio/virtio-rng.h | 13 +
 include/hw/virtio/virtio-scsi.h| 29 +--
 include/hw/virtio/virtio-serial.h  | 24 -
 include/hw/virtio/virtio.h |  6 ++-
 21 files changed, 513 insertions(+), 285 deletions(-)

-- 
1.8.1.4

[Qemu-devel] [PATCH RFT 2/5] virtio: Convert VirtioDevice to QOM realize/unrealize

[Andreas Färber]

VirtioDevice's DeviceClass::exit code cleaning up bus_name is no longer
overwritten by virtio-{blk,serial,net,scsi,balloon,rng} and vhost-scsi.

Note: VirtIOSCSI and VHostSCSI now perform some initializations after
VirtIOSCSICommon's realize calls virtio_bus_plug_device(), namely
creating the SCSIBus and initializing /dev/vhost-scsi respectively.

While at it, drop duplicate VIRTIO_DEVICE() casts and avoid qdev.

Signed-off-by: Andreas Färber 
---
 hw/9pfs/virtio-9p-device.c | 67 --
 hw/9pfs/virtio-9p.h| 13 ++
 hw/block/virtio-blk.c  | 52 ++-
 hw/char/virtio-serial-bus.c| 49 ++
 hw/net/virtio-net.c| 48 -
 hw/scsi/vhost-scsi.c   | 59 +++---
 hw/scsi/virtio-scsi.c  | 85 --
 hw/virtio/virtio-balloon.c | 50 +-
 hw/virtio/virtio-rng.c | 53 ++--
 hw/virtio/virtio.c | 20 -
 include/hw/virtio/vhost-scsi.h | 13 ++
 include/hw/virtio/virtio-balloon.h | 13 ++
 include/hw/virtio/virtio-blk.h | 13 ++
 include/hw/virtio/virtio-net.h | 13 ++
 include/hw/virtio/virtio-rng.h | 13 ++
 include/hw/virtio/virtio-scsi.h| 29 +++--
 include/hw/virtio/virtio-serial.h  | 13 ++
 include/hw/virtio/virtio.h |  6 ++-
 18 files changed, 406 insertions(+), 203 deletions(-)

diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index dc6f4e4..409d315 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -41,15 +41,17 @@ static void virtio_9p_get_config(VirtIODevice *vdev, 
uint8_t *config)
 g_free(cfg);
 }
 
-static int virtio_9p_device_init(VirtIODevice *vdev)
+static void virtio_9p_device_realize(DeviceState *dev, Error **errp)
 {
-V9fsState *s = VIRTIO_9P(vdev);
+VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+V9fsState *s = VIRTIO_9P(dev);
+V9fsClass *v9c = VIRTIO_9P_GET_CLASS(dev);
 int i, len;
 struct stat stat;
 FsDriverEntry *fse;
 V9fsPath path;
 
-virtio_init(VIRTIO_DEVICE(s), "virtio-9p", VIRTIO_ID_9P,
+virtio_init(vdev, "virtio-9p", VIRTIO_ID_9P,
 sizeof(struct virtio_9p_config) + MAX_TAG_LEN);
 
 /* initialize pdu allocator */
@@ -65,17 +67,17 @@ static int virtio_9p_device_init(VirtIODevice *vdev)
 
 if (!fse) {
 /* We don't have a fsdev identified by fsdev_id */
-fprintf(stderr, "Virtio-9p device couldn't find fsdev with the "
-"id = %s\n",
-s->fsconf.fsdev_id ? s->fsconf.fsdev_id : "NULL");
-return -1;
+error_setg(errp, "Virtio-9p device couldn't find fsdev with the "
+   "id = %s",
+   s->fsconf.fsdev_id ? s->fsconf.fsdev_id : "NULL");
+return;
 }
 
 if (!s->fsconf.tag) {
 /* we haven't specified a mount_tag */
-fprintf(stderr, "fsdev with id %s needs mount_tag arguments\n",
-s->fsconf.fsdev_id);
-return -1;
+error_setg(errp, "fsdev with id %s needs mount_tag arguments",
+   s->fsconf.fsdev_id);
+return;
 }
 
 s->ctx.export_flags = fse->export_flags;
@@ -83,9 +85,9 @@ static int virtio_9p_device_init(VirtIODevice *vdev)
 s->ctx.exops.get_st_gen = NULL;
 len = strlen(s->fsconf.tag);
 if (len > MAX_TAG_LEN - 1) {
-fprintf(stderr, "mount tag '%s' (%d bytes) is longer than "
-"maximum (%d bytes)", s->fsconf.tag, len, MAX_TAG_LEN - 1);
-return -1;
+error_setg(errp, "mount tag '%s' (%d bytes) is longer than "
+   "maximum (%d bytes)", s->fsconf.tag, len, MAX_TAG_LEN - 1);
+return;
 }
 
 s->tag = strdup(s->fsconf.tag);
@@ -97,13 +99,13 @@ static int virtio_9p_device_init(VirtIODevice *vdev)
 qemu_co_rwlock_init(&s->rename_lock);
 
 if (s->ops->init(&s->ctx) < 0) {
-fprintf(stderr, "Virtio-9p Failed to initialize fs-driver with id:%s"
-" and export path:%s\n", s->fsconf.fsdev_id, s->ctx.fs_root);
-return -1;
+error_setg(errp, "Virtio-9p Failed to initialize fs-driver with id:%s"
+   " and export path:%s", s->fsconf.fsdev_id, s->ctx.fs_root);
+return;
 }
 if (v9fs_init_worker_threads() < 0) {
-fprintf(stderr, "worker thread initialization failed\n");
-return -1;
+error_setg(errp, "worker thread initialization failed");
+return;
 }
 
 /*
@@ -113,20 +115,20 @@ static int virtio_9p_device_init(VirtIODevice *vdev)
  */
 v9fs_path_init(&path);
 if (s->ops->name_to_path(&s->ctx, NULL, "/", &path) < 0) {
-fprintf(stderr,
-"error in converting name to path %s", strerror(errno));
-return -1;
+error_setg(errp,
+   "erro

[Qemu-devel] [PATCH RFT 1/5] virtio-blk-dataplane: Improve error reporting

[Andreas Färber]

Return an Error so that it can be propagated later.

Signed-off-by: Andreas Färber 
---
 hw/block/dataplane/virtio-blk.c | 25 +
 hw/block/dataplane/virtio-blk.h |  5 +++--
 hw/block/virtio-blk.c   |  8 +++-
 3 files changed, 23 insertions(+), 15 deletions(-)

diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 0356665..5ae8d6e 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -395,8 +395,9 @@ static void start_data_plane_bh(void *opaque)
s, QEMU_THREAD_JOINABLE);
 }
 
-bool virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *blk,
-  VirtIOBlockDataPlane **dataplane)
+void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *blk,
+  VirtIOBlockDataPlane **dataplane,
+  Error **errp)
 {
 VirtIOBlockDataPlane *s;
 int fd;
@@ -404,25 +405,26 @@ bool virtio_blk_data_plane_create(VirtIODevice *vdev, 
VirtIOBlkConf *blk,
 *dataplane = NULL;
 
 if (!blk->data_plane) {
-return true;
+return;
 }
 
 if (blk->scsi) {
-error_report("device is incompatible with x-data-plane, use scsi=off");
-return false;
+error_setg(errp,
+   "device is incompatible with x-data-plane, use scsi=off");
+return;
 }
 
 if (blk->config_wce) {
-error_report("device is incompatible with x-data-plane, "
- "use config-wce=off");
-return false;
+error_setg(errp, "device is incompatible with x-data-plane, "
+ "use config-wce=off");
+return;
 }
 
 fd = raw_get_aio_fd(blk->conf.bs);
 if (fd < 0) {
-error_report("drive is incompatible with x-data-plane, "
- "use format=raw,cache=none,aio=native");
-return false;
+error_setg(errp, "drive is incompatible with x-data-plane, "
+ "use format=raw,cache=none,aio=native");
+return;
 }
 
 s = g_new0(VirtIOBlockDataPlane, 1);
@@ -438,7 +440,6 @@ bool virtio_blk_data_plane_create(VirtIODevice *vdev, 
VirtIOBlkConf *blk,
 migrate_add_blocker(s->migration_blocker);
 
 *dataplane = s;
-return true;
 }
 
 void virtio_blk_data_plane_destroy(VirtIOBlockDataPlane *s)
diff --git a/hw/block/dataplane/virtio-blk.h b/hw/block/dataplane/virtio-blk.h
index c90e99f..1750c99 100644
--- a/hw/block/dataplane/virtio-blk.h
+++ b/hw/block/dataplane/virtio-blk.h
@@ -19,8 +19,9 @@
 
 typedef struct VirtIOBlockDataPlane VirtIOBlockDataPlane;
 
-bool virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *blk,
-  VirtIOBlockDataPlane **dataplane);
+void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *blk,
+  VirtIOBlockDataPlane **dataplane,
+  Error **errp);
 void virtio_blk_data_plane_destroy(VirtIOBlockDataPlane *s);
 void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s);
 void virtio_blk_data_plane_stop(VirtIOBlockDataPlane *s);
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index cf12469..8ea1f03 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -633,6 +633,9 @@ static int virtio_blk_device_init(VirtIODevice *vdev)
 DeviceState *qdev = DEVICE(vdev);
 VirtIOBlock *s = VIRTIO_BLK(vdev);
 VirtIOBlkConf *blk = &(s->blk);
+#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
+Error *err = NULL;
+#endif
 static int virtio_blk_id;
 
 if (!blk->conf.bs) {
@@ -660,7 +663,10 @@ static int virtio_blk_device_init(VirtIODevice *vdev)
 
 s->vq = virtio_add_queue(vdev, 128, virtio_blk_handle_output);
 #ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
-if (!virtio_blk_data_plane_create(vdev, blk, &s->dataplane)) {
+virtio_blk_data_plane_create(vdev, blk, &s->dataplane, &err);
+if (err != NULL) {
+error_report("%s", error_get_pretty(err));
+error_free(err);
 virtio_cleanup(vdev);
 return -1;
 }
-- 
1.8.1.4

[Qemu-devel] [PATCH RFT 4/5] virtio-console: Use exitfn for virtserialport, too

[Andreas Färber]

virtconsole and virtserialport are identical in every other aspect
except for the distinguishing VirtIOSerialPortClass::is_console field.

Cc: qemu-sta...@nongnu.org
Signed-off-by: Andreas Färber 
---
 hw/char/virtio-console.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/char/virtio-console.c b/hw/char/virtio-console.c
index 7b1a382..73e18f2 100644
--- a/hw/char/virtio-console.c
+++ b/hw/char/virtio-console.c
@@ -192,6 +192,7 @@ static void virtserialport_class_init(ObjectClass *klass, 
void *data)
 VirtIOSerialPortClass *k = VIRTIO_SERIAL_PORT_CLASS(klass);
 
 k->init = virtconsole_initfn;
+k->exit = virtconsole_exitfn;
 k->have_data = flush_buf;
 k->set_guest_connected = set_guest_connected;
 dc->props = virtserialport_properties;
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH] linux-user: add SIOCADDRT/SIOCDELRT support

[Laurent Vivier]

Ping


Le 27/05/2013 20:48, Laurent Vivier a écrit :

This allows to pass the device name.

You can test this with the "route" command.

WITHOUT this patch:

$ sudo route add -net default gw 10.0.3.1 eth0
SIOCADDRT: Bad address
$ netstat -nr
Kernel IP routing table
Destination Gateway Genmask Flags   MSS Window  irtt Ifa
10.0.3.00.0.0.0 255.255.255.0   U 0 0  0 eth

WITH this patch:

$ sudo route add -net default gw 10.0.3.1 eth0
$ netstat -nr
Kernel IP routing table
Destination Gateway Genmask Flags   MSS Window  irtt Ifa
0.0.0.0 10.0.3.10.0.0.0 UG0 0  0 eth
10.0.3.00.0.0.0 255.255.255.0   U 0 0  0 eth

Signed-off-by: Laurent Vivier 
---
  linux-user/ioctls.h  |  6 +++--
  linux-user/syscall.c | 64 
  2 files changed, 68 insertions(+), 2 deletions(-)

diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
index 8a47767..439c2a9 100644
--- a/linux-user/ioctls.h
+++ b/linux-user/ioctls.h
@@ -88,8 +88,6 @@
  #endif
  
IOCTL(SIOCATMARK, 0, TYPE_NULL)

-  IOCTL(SIOCADDRT, IOC_W, MK_PTR(MK_STRUCT(STRUCT_rtentry)))
-  IOCTL(SIOCDELRT, IOC_W, MK_PTR(MK_STRUCT(STRUCT_rtentry)))
IOCTL(SIOCGIFNAME, IOC_RW, MK_PTR(TYPE_INT))
IOCTL(SIOCGIFFLAGS, IOC_W | IOC_R, MK_PTR(MK_STRUCT(STRUCT_short_ifreq)))
IOCTL(SIOCSIFFLAGS, IOC_W, MK_PTR(MK_STRUCT(STRUCT_short_ifreq)))
@@ -379,3 +377,7 @@
  MK_PTR(MK_STRUCT(STRUCT_dm_ioctl)))
IOCTL_SPECIAL(DM_DEV_SET_GEOMETRY, IOC_RW, do_ioctl_dm,
  MK_PTR(MK_STRUCT(STRUCT_dm_ioctl)))
+  IOCTL_SPECIAL(SIOCADDRT, IOC_W, do_ioctl_rt,
+MK_PTR(MK_STRUCT(STRUCT_rtentry)))
+  IOCTL_SPECIAL(SIOCDELRT, IOC_W, do_ioctl_rt,
+MK_PTR(MK_STRUCT(STRUCT_rtentry)))
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 1b3c0ed..a5cd166 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -105,6 +105,7 @@ int __clone2(int (*fn)(void *), void *child_stack_base,
  #include 
  #include 
  #include 
+#include 
  #include "linux_loop.h"
  #include "cpu-uname.h"
  
@@ -3714,6 +3715,69 @@ out:

  return ret;
  }
  
+static abi_long do_ioctl_rt(const IOCTLEntry *ie, uint8_t *buf_temp,

+int fd, abi_long cmd, abi_long arg)
+{
+const argtype *arg_type = ie->arg_type;
+const StructEntry *se;
+const argtype *field_types;
+const int *dst_offsets, *src_offsets;
+int target_size;
+void *argptr;
+abi_ulong *target_rt_dev_ptr;
+unsigned long *host_rt_dev_ptr;
+abi_long ret;
+int i;
+
+assert(ie->access == IOC_W);
+assert(*arg_type == TYPE_PTR);
+arg_type++;
+assert(*arg_type == TYPE_STRUCT);
+target_size = thunk_type_size(arg_type, 0);
+argptr = lock_user(VERIFY_READ, arg, target_size, 1);
+if (!argptr) {
+return -TARGET_EFAULT;
+}
+arg_type++;
+assert(*arg_type == (int)STRUCT_rtentry);
+se = struct_entries + *arg_type++;
+assert(se->convert[0] == NULL);
+/* convert struct here to be able to catch rt_dev string */
+field_types = se->field_types;
+dst_offsets = se->field_offsets[THUNK_HOST];
+src_offsets = se->field_offsets[THUNK_TARGET];
+for (i = 0; i < se->nb_fields; i++) {
+if (dst_offsets[i] == offsetof(struct rtentry, rt_dev)) {
+assert(*field_types == TYPE_PTRVOID);
+target_rt_dev_ptr = (abi_ulong *)(argptr + src_offsets[i]);
+host_rt_dev_ptr = (unsigned long *)(buf_temp + dst_offsets[i]);
+if (*target_rt_dev_ptr != 0) {
+*host_rt_dev_ptr = (unsigned long)lock_user_string(
+  tswapal(*target_rt_dev_ptr));
+if (!*host_rt_dev_ptr) {
+unlock_user(argptr, arg, 0);
+return -TARGET_EFAULT;
+}
+} else {
+*host_rt_dev_ptr = 0;
+}
+field_types++;
+continue;
+}
+field_types = thunk_convert(buf_temp + dst_offsets[i],
+argptr + src_offsets[i],
+field_types, THUNK_HOST);
+}
+unlock_user(argptr, arg, 0);
+
+ret = get_errno(ioctl(fd, ie->host_cmd, buf_temp));
+if (*host_rt_dev_ptr != 0) {
+unlock_user((void *)*host_rt_dev_ptr,
+*target_rt_dev_ptr, 0);
+}
+return ret;
+}
+
  static IOCTLEntry ioctl_entries[] = {
  #define IOCTL(cmd, access, ...) \
  { TARGET_ ## cmd, cmd, #cmd, access, 0, {  __VA_ARGS__ } },

[Qemu-devel] [PATCH v4 2/2] sheepdog: support 'qemu-img snapshot -a'

[Liu Yuan]

Just call sd_create_branch() in the snapshot_goto to rollback the image is good
enough. With this patch, 'loadvm' process for sheepdog is modified:

Suppose we have a snapshot chain A --> B --> C, we do 'loadvm A' so as to get
a new chain,

A --> B
|
V
C1

in the old code:

1 reload inode of A (in snapshot_goto)
2 read vmstate via A's vdi_id (loadvm_state)
3 delete C and create C1, reload inode of C1 (sd_create_branch on write)

with this patch applied:

1 reload inode of A, delete C and create C1  (in snapshot_goto)
2 read vmstate via C1's parent, that is A's vdi_id (loadvm_state)

This will fix the possible bug that QEMU exit between 2 and 3 in the old code

Cc: qemu-devel@nongnu.org
Cc: MORITA Kazutaka 
Cc: Kevin Wolf 
Cc: Stefan Hajnoczi 
Signed-off-by: Liu Yuan 
---
 block/sheepdog.c |   12 +---
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index 94218ac..1b7c3f1 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -2071,14 +2071,11 @@ static int sd_snapshot_goto(BlockDriverState *bs, const 
char *snapshot_id)
 goto out;
 }
 
-if (!s->inode.vm_state_size) {
-error_report("Invalid snapshot");
-ret = -ENOENT;
+ret = sd_create_branch(s);
+if (ret) {
 goto out;
 }
 
-s->is_snapshot = true;
-
 g_free(old_s);
 
 return 0;
@@ -2196,8 +2193,9 @@ static int do_load_save_vmstate(BDRVSheepdogState *s, 
uint8_t *data,
 int fd, ret = 0, remaining = size;
 unsigned int data_len;
 uint64_t vmstate_oid;
-uint32_t vdi_index;
 uint64_t offset;
+uint32_t vdi_index;
+uint32_t vdi_id = load ? s->inode.parent_vdi_id : s->inode.vdi_id;
 
 fd = connect_to_sdog(s);
 if (fd < 0) {
@@ -2210,7 +2208,7 @@ static int do_load_save_vmstate(BDRVSheepdogState *s, 
uint8_t *data,
 
 data_len = MIN(remaining, SD_DATA_OBJ_SIZE - offset);
 
-vmstate_oid = vid_to_vmstate_oid(s->inode.vdi_id, vdi_index);
+vmstate_oid = vid_to_vmstate_oid(vdi_id, vdi_index);
 
 create = (offset == 0);
 if (load) {
-- 
1.7.9.5

[Qemu-devel] [PATCH 1/2] sheepdog: fix snapshot tag initialization

[Liu Yuan]

This is an old and obvious bug. We should pass snapshot_id to the
tag. Or simple command like 'qemu-img snapshot -a tag sheepdog:image' will fail

Cc: qemu-devel@nongnu.org
Cc: MORITA Kazutaka 
Cc: Kevin Wolf 
Cc: Stefan Hajnoczi 
Reviewed-by: Kevin Wolf 
Signed-off-by: Liu Yuan 
---
 block/sheepdog.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index 21a4edf..94218ac 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -2063,7 +2063,7 @@ static int sd_snapshot_goto(BlockDriverState *bs, const 
char *snapshot_id)
 if (snapid) {
 tag[0] = 0;
 } else {
-pstrcpy(tag, sizeof(tag), s->name);
+pstrcpy(tag, sizeof(tag), snapshot_id);
 }
 
 ret = reload_inode(s, snapid, tag);
-- 
1.7.9.5

[Qemu-devel] [PATCH v4 0/2] fix 'qemu-img snapshot -a' operation for sheepdog

[Liu Yuan]

v4:
 - fix savevm, pass current vdi_id instead of parent_vdi_id

v3:
 - fix sheepdog's loadvm handling, don't rely on the write to create branch

v2:
 - add the comment to make things more clear
 - call sd_create_branch() after s->is_snapshot = true because after calling
   sd_create_branch, it is not snapshot anymore.

Nothing big, just two simple patches to enable this commind for sheepdog.

Cc: qemu-devel@nongnu.org
Cc: MORITA Kazutaka 
Cc: Kevin Wolf 
Cc: Stefan Hajnoczi 

Liu Yuan (2):
  sheepdog: fix snapshot tag initialization
  sheepdog: support 'qemu-img snapshot -a'

 block/sheepdog.c |   10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

-- 
1.7.9.5

Re: [Qemu-devel] [PATCH v3 0/2] fix 'qemu-img snapshot -a' operation for sheepdog

[Liu Yuan]

On 06/08/2013 01:20 AM, Liu Yuan wrote:
> v3:
>  - fix sheepdog's loadvm handling, don't rely on the write to create branch

Oops, seems this version has problems. Please drop it.

Thanks,
Yuan

[Qemu-devel] [PATCH 1/8] [PATCH RFC v2] s390-qemu: cpu hotplug - Define New SCLP Codes

[Jason J. Herne]

From: "Jason J. Herne" 

Define new SCLP codes to improve code readability.

Signed-off-by: Jason J. Herne 
---
 hw/s390x/sclp.c |2 +-
 include/hw/s390x/sclp.h |8 
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
index 86d6ae0..cb53d7e 100644
--- a/hw/s390x/sclp.c
+++ b/hw/s390x/sclp.c
@@ -45,7 +45,7 @@ static void sclp_execute(SCCB *sccb, uint64_t code)
 {
 S390SCLPDevice *sdev = get_event_facility();
 
-switch (code) {
+switch (code & SCLP_NO_CMD_PARM) {
 case SCLP_CMDW_READ_SCP_INFO:
 case SCLP_CMDW_READ_SCP_INFO_FORCED:
 read_SCP_info(sccb);
diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
index 231a38a..174097d 100644
--- a/include/hw/s390x/sclp.h
+++ b/include/hw/s390x/sclp.h
@@ -26,6 +26,14 @@
 #define SCLP_CMD_WRITE_EVENT_DATA   0x00760005
 #define SCLP_CMD_WRITE_EVENT_MASK   0x00780005
 
+/* CPU hotplug SCLP codes */
+#define SCLP_NO_CMD_PARM0x00ff
+#define SCLP_HAS_CPU_INFO   0x0C00ULL
+#define SCLP_CMDW_READ_CPU_INFO 0x00010001
+#define SCLP_CMDW_CONFIGURE_CPU 0x00110001
+#define SCLP_CMDW_DECONFIGURE_CPU   0x0011
+#define SCLP_CMDW_CPU_CMD_PARM  0xff00
+
 /* SCLP response codes */
 #define SCLP_RC_NORMAL_READ_COMPLETION  0x0010
 #define SCLP_RC_NORMAL_COMPLETION   0x0020
-- 
1.7.10.4

[Qemu-devel] [PATCH 6/8] [PATCH RFC v2] s390-qemu: cpu hotplug - Storage key Global Access

[Jason J. Herne]

From: "Jason J. Herne" 

In preparation for treating cpus as devices we refactor the code such that cpu
initialization no longer relies on cpu 0 to obtain a pointer to the storage key
data.  This patch introduces global access to that data.

Signed-off-by: Jason J. Herne 
---
 hw/s390x/s390-virtio-ccw.c |5 ++---
 hw/s390x/s390-virtio.c |   21 -
 hw/s390x/s390-virtio.h |2 +-
 target-s390x/cpu.h |3 +++
 4 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index eb774d9..70bd858 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -65,7 +65,6 @@ static void ccw_init(QEMUMachineInitArgs *args)
 MemoryRegion *sysmem = get_system_memory();
 MemoryRegion *ram = g_new(MemoryRegion, 1);
 int shift = 0;
-uint8_t *storage_keys;
 int ret;
 VirtualCssBus *css_bus;
 
@@ -94,10 +93,10 @@ static void ccw_init(QEMUMachineInitArgs *args)
 memory_region_add_subregion(sysmem, 0, ram);
 
 /* allocate storage keys */
-storage_keys = g_malloc0(my_ram_size / TARGET_PAGE_SIZE);
+s390_set_storage_keys_p(g_malloc0(my_ram_size / TARGET_PAGE_SIZE));
 
 /* init CPUs */
-s390_init_cpus(args->cpu_model, storage_keys);
+s390_init_cpus(args->cpu_model);
 
 if (kvm_enabled()) {
 kvm_s390_enable_css_support(s390_cpu_addr2state(0));
diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c
index ef4f1ae..4af2d86 100644
--- a/hw/s390x/s390-virtio.c
+++ b/hw/s390x/s390-virtio.c
@@ -136,6 +136,18 @@ static void s390_virtio_register_hcalls(void)
s390_virtio_hcall_set_status);
 }
 
+static uint8_t *storage_keys;
+
+uint8_t *s390_get_storage_keys_p(void)
+{
+return storage_keys;
+}
+
+void s390_set_storage_keys_p(uint8_t *keys_p)
+{
+storage_keys = keys_p;
+}
+
 /*
  * The number of running CPUs. On s390 a shutdown is the state of all CPUs
  * being either stopped or disabled (for interrupts) waiting. We have to
@@ -189,7 +201,7 @@ void s390_init_ipl_dev(const char *kernel_filename,
 qdev_init_nofail(dev);
 }
 
-void s390_init_cpus(const char *cpu_model, uint8_t *storage_keys)
+void s390_init_cpus(const char *cpu_model)
 {
 int i;
 
@@ -209,7 +221,7 @@ void s390_init_cpus(const char *cpu_model, uint8_t 
*storage_keys)
 ipi_states[i] = cpu;
 cs->halted = 1;
 cpu->env.exception_index = EXCP_HLT;
-cpu->env.storage_keys = storage_keys;
+cpu->env.storage_keys = s390_get_storage_keys_p();
 }
 }
 
@@ -244,7 +256,6 @@ static void s390_init(QEMUMachineInitArgs *args)
 MemoryRegion *sysmem = get_system_memory();
 MemoryRegion *ram = g_new(MemoryRegion, 1);
 int shift = 0;
-uint8_t *storage_keys;
 void *virtio_region;
 hwaddr virtio_region_len;
 hwaddr virtio_region_start;
@@ -283,10 +294,10 @@ static void s390_init(QEMUMachineInitArgs *args)
   virtio_region_len);
 
 /* allocate storage keys */
-storage_keys = g_malloc0(my_ram_size / TARGET_PAGE_SIZE);
+s390_set_storage_keys_p(g_malloc0(my_ram_size / TARGET_PAGE_SIZE));
 
 /* init CPUs */
-s390_init_cpus(args->cpu_model, storage_keys);
+s390_init_cpus(args->cpu_model);
 
 /* Create VirtIO network adapters */
 s390_create_virtio_net((BusState *)s390_bus, "virtio-net-s390");
diff --git a/hw/s390x/s390-virtio.h b/hw/s390x/s390-virtio.h
index 5c405e7..c1cb042 100644
--- a/hw/s390x/s390-virtio.h
+++ b/hw/s390x/s390-virtio.h
@@ -20,7 +20,7 @@
 typedef int (*s390_virtio_fn)(const uint64_t *args);
 void s390_register_virtio_hypercall(uint64_t code, s390_virtio_fn fn);
 
-void s390_init_cpus(const char *cpu_model, uint8_t *storage_keys);
+void s390_init_cpus(const char *cpu_model);
 void s390_init_ipl_dev(const char *kernel_filename,
const char *kernel_cmdline,
const char *initrd_filename,
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index 029d0c5..a4b87bf 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -393,6 +393,9 @@ uint16_t s390_cpu_get_free_state_idx(void);
 void s390_add_running_cpu(S390CPU *cpu);
 unsigned s390_del_running_cpu(S390CPU *cpu);
 
+uint8_t *s390_get_storage_keys_p(void);
+void s390_set_storage_keys_p(uint8_t *keys_p);
+
 /* service interrupts are floating therefore we must not pass an cpustate */
 void s390_sclp_extint(uint32_t parm);
 
-- 
1.7.10.4

[Qemu-devel] [PATCH 7/8] [PATCH RFC v2] s390-qemu: cpu hotplug - Infrastructure for Cpu Devices

[Jason J. Herne]

From: "Jason J. Herne" 

Add infrastructure for treating cpus as devices. This patch allows cpus to be
specified using a combination of '-smp' and '-device cpu'.  This approach
forces a change in the way cpus are counted via smp_cpus.

Signed-off-by: Jason J. Herne 
---
 include/hw/boards.h |3 ++
 vl.c|   95 +++
 2 files changed, 84 insertions(+), 14 deletions(-)

diff --git a/include/hw/boards.h b/include/hw/boards.h
index ed427a1..b0c86bf 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -47,8 +47,11 @@ typedef struct QEMUMachine {
 GlobalProperty *compat_props;
 struct QEMUMachine *next;
 const char *hw_version;
+const char *cpu_device_str;
 } QEMUMachine;
 
+#define CPUS_ARE_DEVICES(qemu_mach)(qemu_mach->cpu_device_str != NULL)
+
 int qemu_register_machine(QEMUMachine *m);
 QEMUMachine *find_default_machine(void);
 
diff --git a/vl.c b/vl.c
index 71e1e6d..873834f 100644
--- a/vl.c
+++ b/vl.c
@@ -546,6 +546,46 @@ static int default_driver_check(QemuOpts *opts, void 
*opaque)
 return 0;
 }
 
+static void convert_smp_to_cpu_devices(QEMUMachine *machine)
+{
+int i;
+QemuOpts *opts;
+
+for (i = 0; i < smp_cpus; i++) {
+opts = qemu_opts_create_nofail(qemu_find_opts("device"));
+qemu_opt_set(opts, "driver", machine->cpu_device_str);
+}
+smp_cpus = 0;
+}
+
+static int count_cpu_devices(QemuOpts *opts, void *opaque)
+{
+const char *driver = qemu_opt_get(opts, "driver");
+QEMUMachine *machine = (QEMUMachine *)opaque;
+
+/* Skip non-cpu devices*/
+if (!driver || strcmp(driver, machine->cpu_device_str) != 0) {
+return 0;
+}
+
+smp_cpus += 1;
+return 0;
+}
+
+static int handle_cpu_device(QemuOpts *opts, void *opaque)
+{
+const char *driver = qemu_opt_get(opts, "driver");
+QEMUMachine *machine = (QEMUMachine *)opaque;
+
+/* Skip non-cpu devices*/
+if (!driver || strcmp(driver, machine->cpu_device_str) != 0) {
+return 0;
+}
+
+qdev_device_add(opts);
+return 0;
+}
+
 /***/
 /* QEMU state */
 
@@ -2318,6 +2358,13 @@ static int device_help_func(QemuOpts *opts, void *opaque)
 static int device_init_func(QemuOpts *opts, void *opaque)
 {
 DeviceState *dev;
+const char *driver = qemu_opt_get(opts, "driver");
+QEMUMachine *machine = (QEMUMachine *)opaque;
+
+/* Skip cpu devices*/
+if (!driver || strcmp(driver, machine->cpu_device_str) == 0) {
+return 0;
+}
 
 dev = qdev_device_add(opts);
 if (!dev)
@@ -3630,19 +3677,6 @@ int main(int argc, char **argv, char **envp)
 break;
 case QEMU_OPTION_smp:
 smp_parse(optarg);
-if (smp_cpus < 1) {
-fprintf(stderr, "Invalid number of CPUs\n");
-exit(1);
-}
-if (max_cpus < smp_cpus) {
-fprintf(stderr, "maxcpus must be equal to or greater than "
-"smp\n");
-exit(1);
-}
-if (max_cpus > 255) {
-fprintf(stderr, "Unsupported number of maxcpus\n");
-exit(1);
-}
 break;
case QEMU_OPTION_vnc:
 #ifdef CONFIG_VNC
@@ -3965,6 +3999,16 @@ int main(int argc, char **argv, char **envp)
 }
 
 /*
+ * Count cpu devices. Cpu count is determied by adding -device cpu
+ * statements to the number of cpus specified on the -smp statement.
+ */
+if (CPUS_ARE_DEVICES(machine)) {
+convert_smp_to_cpu_devices(machine);
+qemu_opts_foreach(qemu_find_opts("device"), count_cpu_devices,
+  machine, 0);
+}
+
+/*
  * Default to max_cpus = smp_cpus, in case the user doesn't
  * specify a max_cpus value.
  */
@@ -3979,6 +4023,20 @@ int main(int argc, char **argv, char **envp)
 exit(1);
 }
 
+if (smp_cpus < 1) {
+fprintf(stderr, "Invalid number of CPUs\n");
+exit(1);
+}
+if (max_cpus < smp_cpus) {
+fprintf(stderr, "maxcpus must be equal to or greater than the number 
of"
+" cpus defined\n");
+exit(1);
+}
+if (max_cpus > 255) {
+fprintf(stderr, "Unsupported number of maxcpus\n");
+exit(1);
+}
+
 /*
  * Get the default machine options from the machine if it is not already
  * specified either by the configuration file or by the command line.
@@ -4305,6 +4363,13 @@ int main(int argc, char **argv, char **envp)
  .cpu_model = cpu_model };
 machine->init(&args);
 
+/* Create cpu devices */
+if (CPUS_ARE_DEVICES(machine)) {
+smp_cpus = 0;  /* Reset this because each cpu will count itself */
+qemu_opts_foreach(qemu_find_opts("device"), handle_cpu_device,
+ 

[Qemu-devel] [PATCH 0/8] [PATCH RFC v2] s390-qemu: cpu hotplug

[Jason J. Herne]

Latest code for cpu Hotplug designed to be controled via the QOM infrastructure.
cpu on S390 are treated as devices via a new platform independent
infrastructure I designed to allow this "new way" to exist with the "old way"
of representing cpus.

The Qemu command line now allows "-device s390-cpu" which will instantiate a 
cpu device. This is additive to anything that might be specified on the -smp
parameter.

Devices can be hot plugged via the monitor command "device_add s390-cpu".
Hotplugged cpus are created in the configured state and can be used by the
guest after the guest onlines the cpu by: 
"echo 1 > /sys/bus/cpu/devices/cpuN/online"

Hot unplugging is currently not implemented by this code. 

[Qemu-devel] [PATCH 8/8] [PATCH RFC v2] s390-qemu: cpu hotplug - Treat S390 cpus as devices

[Jason J. Herne]

From: "Jason J. Herne" 

Modify cpu initialization and QOM routines associated with s390-cpu such that
all cpus on S390 are now created via the QOM device creation code path.

Signed-off-by: Jason J. Herne 
---
 hw/s390x/s390-virtio-ccw.c |   15 ++-
 hw/s390x/s390-virtio.c |   25 +
 hw/s390x/s390-virtio.h |2 +-
 include/qapi/qmp/qerror.h  |3 +++
 qdev-monitor.c |   17 +
 target-s390x/cpu.c |   24 ++--
 6 files changed, 58 insertions(+), 28 deletions(-)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 70bd858..141adce 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -95,12 +95,8 @@ static void ccw_init(QEMUMachineInitArgs *args)
 /* allocate storage keys */
 s390_set_storage_keys_p(g_malloc0(my_ram_size / TARGET_PAGE_SIZE));
 
-/* init CPUs */
-s390_init_cpus(args->cpu_model);
+s390_init_ipi_states();
 
-if (kvm_enabled()) {
-kvm_s390_enable_css_support(s390_cpu_addr2state(0));
-}
 /*
  * Create virtual css and set it as default so that non mcss-e
  * enabled guests only see virtio devices.
@@ -112,11 +108,20 @@ static void ccw_init(QEMUMachineInitArgs *args)
 s390_create_virtio_net(BUS(css_bus), "virtio-net-ccw");
 }
 
+static void ccw_post_cpu_init(void)
+{
+if (kvm_enabled()) {
+kvm_s390_enable_css_support(s390_cpu_addr2state(0));
+}
+}
+
 static QEMUMachine ccw_machine = {
 .name = "s390-ccw-virtio",
 .alias = "s390-ccw",
 .desc = "VirtIO-ccw based S390 machine",
+.cpu_device_str = "s390-cpu",
 .init = ccw_init,
+.post_cpu_init = ccw_post_cpu_init,
 .block_default_type = IF_VIRTIO,
 .no_cdrom = 1,
 .no_floppy = 1,
diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c
index 4af2d86..069a187 100644
--- a/hw/s390x/s390-virtio.c
+++ b/hw/s390x/s390-virtio.c
@@ -201,31 +201,17 @@ void s390_init_ipl_dev(const char *kernel_filename,
 qdev_init_nofail(dev);
 }
 
-void s390_init_cpus(const char *cpu_model)
+void s390_init_ipi_states(void)
 {
 int i;
 
-if (cpu_model == NULL) {
-cpu_model = "host";
-}
-
-ipi_states = g_malloc(sizeof(S390CPU *) * smp_cpus);
-
-for (i = 0; i < smp_cpus; i++) {
-S390CPU *cpu;
-CPUState *cs;
+ipi_states = g_malloc(sizeof(S390CPU *) * max_cpus);
 
-cpu = cpu_s390x_init(cpu_model);
-cs = CPU(cpu);
-
-ipi_states[i] = cpu;
-cs->halted = 1;
-cpu->env.exception_index = EXCP_HLT;
-cpu->env.storage_keys = s390_get_storage_keys_p();
+for (i = 0; i < max_cpus; i++) {
+ipi_states[i] = NULL;
 }
 }
 
-
 void s390_create_virtio_net(BusState *bus, const char *name)
 {
 int i;
@@ -296,8 +282,7 @@ static void s390_init(QEMUMachineInitArgs *args)
 /* allocate storage keys */
 s390_set_storage_keys_p(g_malloc0(my_ram_size / TARGET_PAGE_SIZE));
 
-/* init CPUs */
-s390_init_cpus(args->cpu_model);
+s390_init_ipi_states();
 
 /* Create VirtIO network adapters */
 s390_create_virtio_net((BusState *)s390_bus, "virtio-net-s390");
diff --git a/hw/s390x/s390-virtio.h b/hw/s390x/s390-virtio.h
index c1cb042..7b1ef9f 100644
--- a/hw/s390x/s390-virtio.h
+++ b/hw/s390x/s390-virtio.h
@@ -20,7 +20,7 @@
 typedef int (*s390_virtio_fn)(const uint64_t *args);
 void s390_register_virtio_hypercall(uint64_t code, s390_virtio_fn fn);
 
-void s390_init_cpus(const char *cpu_model);
+void s390_init_ipi_states(void);
 void s390_init_ipl_dev(const char *kernel_filename,
const char *kernel_cmdline,
const char *initrd_filename,
diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index 6c0a18d..6627dc4 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -162,6 +162,9 @@ void assert_no_error(Error *err);
 #define QERR_KVM_MISSING_CAP \
 ERROR_CLASS_K_V_M_MISSING_CAP, "Using KVM without %s, %s unavailable"
 
+#define QERR_MAX_CPUS \
+ERROR_CLASS_GENERIC_ERROR, "The maximum number of cpus has already been 
created for this guest"
+
 #define QERR_MIGRATION_ACTIVE \
 ERROR_CLASS_GENERIC_ERROR, "There's a migration process in progress"
 
diff --git a/qdev-monitor.c b/qdev-monitor.c
index e54dbc2..a4adeb8 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -23,6 +23,9 @@
 #include "monitor/qdev.h"
 #include "qmp-commands.h"
 #include "sysemu/arch_init.h"
+#include "sysemu/sysemu.h"
+#include "hw/boards.h"
+#include "sysemu/cpus.h"
 #include "qemu/config-file.h"
 
 /*
@@ -442,6 +445,14 @@ DeviceState *qdev_device_add(QemuOpts *opts)
 return NULL;
 }
 
+if (driver && current_machine &&
+strcmp(driver, current_machine->cpu_device_str) == 0) {
+if (smp_cpus == max_cpus) {
+qerror_report(QERR_MAX_CPUS);
+return NULL;
+}
+}
+
 k = DEVICE_CLASS(obj);
 
 /* find bu

[Qemu-devel] [PATCH 2/8] [PATCH RFC v2] s390-qemu: cpu hotplug - SCLP CPU Info

[Jason J. Herne]

From: "Jason J. Herne" 

Implement the CPU data in SCLP "Read SCP Info".  And implement "Read CPU Info"
SCLP command. This data will be used by the guest to get information about hot
plugged cpus.

Signed-off-by: Jason J. Herne 
---
 hw/s390x/sclp.c |   38 ++
 include/hw/s390x/sclp.h |   30 ++
 2 files changed, 68 insertions(+)

diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
index cb53d7e..20b718f 100644
--- a/hw/s390x/sclp.c
+++ b/hw/s390x/sclp.c
@@ -15,6 +15,7 @@
 #include "cpu.h"
 #include "sysemu/kvm.h"
 #include "exec/memory.h"
+#include "sysemu/sysemu.h"
 
 #include "hw/s390x/sclp.h"
 
@@ -32,6 +33,19 @@ static void read_SCP_info(SCCB *sccb)
 {
 ReadInfo *read_info = (ReadInfo *) sccb;
 int shift = 0;
+int i = 0;
+
+/* CPU information */
+read_info->entries_cpu = cpu_to_be16(smp_cpus);
+read_info->offset_cpu = cpu_to_be16(offsetof(ReadInfo, entries));
+read_info->highest_cpu = cpu_to_be16(max_cpus);
+
+for (i = 0; i < smp_cpus; i++) {
+read_info->entries[i].address = i;
+read_info->entries[i].type = 0;
+}
+
+read_info->facilities = cpu_to_be64(SCLP_HAS_CPU_INFO);
 
 while ((ram_size >> (20 + shift)) > 65535) {
 shift++;
@@ -41,6 +55,27 @@ static void read_SCP_info(SCCB *sccb)
 sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_READ_COMPLETION);
 }
 
+static void sclp_read_cpu_info(SCCB *sccb)
+{
+ReadCpuInfo *cpu_info = (ReadCpuInfo *) sccb;
+int i = 0;
+
+cpu_info->nr_configured = cpu_to_be16(smp_cpus);
+cpu_info->offset_configured = cpu_to_be16(offsetof(ReadCpuInfo, entries));
+cpu_info->nr_standby = cpu_to_be16(0);
+
+/* The standby offset is 16-byte for each CPU */
+cpu_info->offset_standby = cpu_to_be16(cpu_info->offset_configured
++ cpu_info->nr_configured*sizeof(CpuEntry));
+
+for (i = 0; i < smp_cpus; i++) {
+cpu_info->entries[i].address = i;
+cpu_info->entries[i].type = 0;
+}
+
+sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_READ_COMPLETION);
+}
+
 static void sclp_execute(SCCB *sccb, uint64_t code)
 {
 S390SCLPDevice *sdev = get_event_facility();
@@ -50,6 +85,9 @@ static void sclp_execute(SCCB *sccb, uint64_t code)
 case SCLP_CMDW_READ_SCP_INFO_FORCED:
 read_SCP_info(sccb);
 break;
+case SCLP_CMDW_READ_CPU_INFO:
+sclp_read_cpu_info(sccb);
+break;
 default:
 sdev->sclp_command_handler(sdev->ef, sccb, code);
 break;
diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
index 174097d..6783d40 100644
--- a/include/hw/s390x/sclp.h
+++ b/include/hw/s390x/sclp.h
@@ -79,12 +79,42 @@ typedef struct SCCBHeader {
 
 #define SCCB_DATA_LEN (SCCB_SIZE - sizeof(SCCBHeader))
 
+/* CPU information */
+typedef struct CpuEntry {
+uint8_t address;
+uint8_t reserved0[13];
+uint8_t type;
+uint8_t reserved1;
+} QEMU_PACKED CpuEntry;
+
 typedef struct ReadInfo {
 SCCBHeader h;
 uint16_t rnmax;
 uint8_t rnsize;
+uint8_t  _reserved1[16 - 11];   /* 11-15 */
+uint16_t entries_cpu;   /* 16-17 */
+uint16_t offset_cpu;/* 18-19 */
+uint8_t  _reserved2[48 - 20];   /* 20-47 */
+uint64_t facilities;/* 48-55 */
+uint8_t  _reserved3[100 - 56];
+uint32_t rnsize2;
+uint64_t rnmax2;
+uint8_t  _reserved4[120-112];   /* 112-119 */
+uint16_t highest_cpu;
+uint8_t  _reserved5[128 - 122]; /* 122-127 */
+struct CpuEntry entries[0];
 } QEMU_PACKED ReadInfo;
 
+typedef struct ReadCpuInfo {
+SCCBHeader h;
+uint16_t nr_configured; /* 8-9 */
+uint16_t offset_configured; /* 10-11 */
+uint16_t nr_standby;/* 12-13 */
+uint16_t offset_standby;/* 14-15 */
+uint8_t reserved0[24-16];   /* 16-23 */
+struct CpuEntry entries[0];
+} QEMU_PACKED ReadCpuInfo;
+
 typedef struct SCCB {
 SCCBHeader h;
 char data[SCCB_DATA_LEN];
-- 
1.7.10.4

[Qemu-devel] [PATCH 5/8] [PATCH RFC v2] s390-qemu: cpu hotplug - Introduce post-cpu-init function

[Jason J. Herne]

From: "Jason J. Herne" 

In preparation for treating cpus as devices we need to separate machine
initialization into two stages:
1. Initialization that needs to be done before cpu devices can be created.
2. Initialization that requires cpu devices to already be created.

This is accomplished by creating an optional post-cpu initialization function
for QEMUMachine.

Signed-off-by: Jason J. Herne 
---
 include/hw/boards.h |3 ++-
 vl.c|4 
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/include/hw/boards.h b/include/hw/boards.h
index fb7c6f1..ed427a1 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -19,7 +19,7 @@ typedef struct QEMUMachineInitArgs {
 } QEMUMachineInitArgs;
 
 typedef void QEMUMachineInitFunc(QEMUMachineInitArgs *args);
-
+typedef void QEMUMachineInitPostCpusFunc(void);
 typedef void QEMUMachineResetFunc(void);
 
 typedef void QEMUMachineHotAddCPUFunc(const int64_t id, Error **errp);
@@ -29,6 +29,7 @@ typedef struct QEMUMachine {
 const char *alias;
 const char *desc;
 QEMUMachineInitFunc *init;
+QEMUMachineInitPostCpusFunc *post_cpu_init;
 QEMUMachineResetFunc *reset;
 QEMUMachineHotAddCPUFunc *hot_add_cpu;
 BlockInterfaceType block_default_type;
diff --git a/vl.c b/vl.c
index 47ab45d..71e1e6d 100644
--- a/vl.c
+++ b/vl.c
@@ -4305,6 +4305,10 @@ int main(int argc, char **argv, char **envp)
  .cpu_model = cpu_model };
 machine->init(&args);
 
+if (machine->post_cpu_init) {
+machine->post_cpu_init();
+}
+
 audio_init();
 
 cpu_synchronize_all_post_init();
-- 
1.7.10.4

[Qemu-devel] [PATCH 4/8] [PATCH RFC v2] s390-qemu: cpu hotplug - ipi_states enhancements

[Jason J. Herne]

From: "Jason J. Herne" 

Modify s390_cpu_addr2state to allow fetching state information for cpu addresses
above the number of online cpus.  Hotplug requires this capability.

Also add s390_cpu_set_state function to allow modification of ipi_state entries
during hotplug.

Finally, add function to find lowest unused ipi_state.

Signed-off-by: Jason J. Herne 
---
 hw/s390x/s390-virtio.c |   19 ---
 target-s390x/cpu.h |2 ++
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c
index 30d1118..ef4f1ae 100644
--- a/hw/s390x/s390-virtio.c
+++ b/hw/s390x/s390-virtio.c
@@ -54,13 +54,26 @@
 static VirtIOS390Bus *s390_bus;
 static S390CPU **ipi_states;
 
+void s390_cpu_set_state(uint16_t cpu_addr, S390CPU *state)
+{
+ipi_states[cpu_addr] = state;
+}
+
 S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)
 {
-if (cpu_addr >= smp_cpus) {
-return NULL;
+return ipi_states[cpu_addr];
+}
+
+uint16_t s390_cpu_get_free_state_idx(void)
+{
+int i;
+for (i = 0; i < max_cpus; i++)
+if (ipi_states[i] == NULL) {
+return i;
 }
 
-return ipi_states[cpu_addr];
+assert(0); /* BUG! */
+return -1;
 }
 
 static int s390_virtio_hcall_notify(const uint64_t *args)
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index 6304c4d..029d0c5 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -388,6 +388,8 @@ static inline void kvm_s390_interrupt_internal(S390CPU 
*cpu, int type,
 }
 #endif
 S390CPU *s390_cpu_addr2state(uint16_t cpu_addr);
+void s390_cpu_set_state(uint16_t cpu_addr, S390CPU *state);
+uint16_t s390_cpu_get_free_state_idx(void);
 void s390_add_running_cpu(S390CPU *cpu);
 unsigned s390_del_running_cpu(S390CPU *cpu);
 
-- 
1.7.10.4

[Qemu-devel] [PATCH 3/8] [PATCH RFC v2] s390-qemu: cpu hotplug - SCLP Event integration

[Jason J. Herne]

From: "Jason J. Herne" 

Add an sclp event for "cpu was hot plugged".  This allows Qemu to deliver an
SCLP interrupt to the guest stating that the requested cpu hotplug was
completed.

Signed-off-by: Jason J. Herne 
---
 hw/s390x/Makefile.objs|2 +-
 hw/s390x/event-facility.c |7 +++
 hw/s390x/sclpcpu.c|  119 +
 include/hw/s390x/event-facility.h |3 +
 include/hw/s390x/sclp.h   |1 +
 5 files changed, 131 insertions(+), 1 deletion(-)
 create mode 100644 hw/s390x/sclpcpu.c

diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs
index 77e1218..856b65d 100644
--- a/hw/s390x/Makefile.objs
+++ b/hw/s390x/Makefile.objs
@@ -2,7 +2,7 @@ obj-y = s390-virtio-bus.o s390-virtio.o
 obj-y += s390-virtio-hcall.o
 obj-y += sclp.o
 obj-y += event-facility.o
-obj-y += sclpquiesce.o
+obj-y += sclpquiesce.o  sclpcpu.o
 obj-y += ipl.o
 obj-y += css.o
 obj-y += s390-virtio-ccw.o
diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
index 0faade0..aec35cb 100644
--- a/hw/s390x/event-facility.c
+++ b/hw/s390x/event-facility.c
@@ -317,6 +317,7 @@ static int init_event_facility(S390SCLPDevice *sdev)
 {
 SCLPEventFacility *event_facility;
 DeviceState *quiesce;
+DeviceState *cpu_hotplug;
 
 event_facility = g_malloc0(sizeof(SCLPEventFacility));
 sdev->ef = event_facility;
@@ -335,6 +336,12 @@ static int init_event_facility(S390SCLPDevice *sdev)
 }
 qdev_init_nofail(quiesce);
 
+cpu_hotplug = qdev_create(&event_facility->sbus.qbus, "sclpcpuhotplug");
+if (!cpu_hotplug) {
+return -1;
+}
+qdev_init_nofail(cpu_hotplug);
+
 return 0;
 }
 
diff --git a/hw/s390x/sclpcpu.c b/hw/s390x/sclpcpu.c
new file mode 100644
index 000..227c8d0
--- /dev/null
+++ b/hw/s390x/sclpcpu.c
@@ -0,0 +1,119 @@
+/*
+ * SCLP event type
+ *Signal CPU - Trigger SCLP interrupt for system CPU configure or
+ *de-configure
+ *
+ * Copyright IBM, Corp. 2013
+ *
+ * Authors:
+ *  Thang Pham 
+ *
+ * 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 
+#include "sysemu/sysemu.h"
+#include "hw/s390x/sclp.h"
+#include "hw/s390x/event-facility.h"
+#include "cpu.h"
+#include "sysemu/cpus.h"
+#include "sysemu/kvm.h"
+
+typedef struct ConfigMgtData {
+EventBufferHeader ebh;
+uint8_t reserved;
+uint8_t event_qualifier;
+} QEMU_PACKED ConfigMgtData;
+
+static qemu_irq irq_cpu_hotplug; /* Only used in this file */
+
+#define EVENT_QUAL_CPU_CHANGE  1
+
+void raise_irq_cpu_hotplug(void)
+{
+qemu_irq_raise(irq_cpu_hotplug);
+}
+
+static int event_type(void)
+{
+return SCLP_EVENT_CONFIG_MGT_DATA;
+}
+
+static unsigned int send_mask(void)
+{
+return SCLP_EVENT_MASK_CONFIG_MGT_DATA;
+}
+
+static unsigned int receive_mask(void)
+{
+return 0;
+}
+
+static int read_event_data(SCLPEvent *event, EventBufferHeader *evt_buf_hdr,
+   int *slen)
+{
+ConfigMgtData *cdata = (ConfigMgtData *) evt_buf_hdr;
+if (*slen < sizeof(ConfigMgtData)) {
+return 0;
+}
+
+/* Event is no longer pending */
+if (!event->event_pending) {
+return 0;
+}
+event->event_pending = false;
+
+/* Event header data */
+cdata->ebh.length = cpu_to_be16(sizeof(ConfigMgtData));
+cdata->ebh.type = SCLP_EVENT_CONFIG_MGT_DATA;
+cdata->ebh.flags |= SCLP_EVENT_BUFFER_ACCEPTED;
+
+/* Trigger a rescan of CPUs by setting event qualifier */
+cdata->event_qualifier = EVENT_QUAL_CPU_CHANGE;
+*slen -= sizeof(ConfigMgtData);
+
+return 1;
+}
+
+static void trigger_signal(void *opaque, int n, int level)
+{
+SCLPEvent *event = opaque;
+event->event_pending = true;
+
+/* Trigger SCLP read operation */
+sclp_service_interrupt(0);
+}
+
+static int irq_cpu_hotplug_init(SCLPEvent *event)
+{
+irq_cpu_hotplug = *qemu_allocate_irqs(trigger_signal, event, 1);
+return 0;
+}
+
+static void cpu_class_init(ObjectClass *klass, void *data)
+{
+SCLPEventClass *k = SCLP_EVENT_CLASS(klass);
+
+k->init = irq_cpu_hotplug_init;
+k->get_send_mask = send_mask;
+k->get_receive_mask = receive_mask;
+k->event_type = event_type;
+k->read_event_data = read_event_data;
+k->write_event_data = NULL;
+}
+
+static TypeInfo sclp_cpu_info = {
+.name  = "sclpcpuhotplug",
+.parent= TYPE_SCLP_EVENT,
+.instance_size = sizeof(SCLPEvent),
+.class_init= cpu_class_init,
+.class_size= sizeof(SCLPEventClass),
+};
+
+static void register_types(void)
+{
+type_register_static(&sclp_cpu_info);
+}
+
+type_init(register_types)
diff --git a/include/hw/s390x/event-facility.h 
b/include/hw/s390x/event-facility.h
index 791ab2a..d63969f 100644
--- a/include/hw/s390x/event-facility.h
+++ b/include/hw/s390x/event-facility.h
@@ -17,14 +17,17 @@
 
 #include 
 #include "qemu/thread.h

Re: [Qemu-devel] [PATCH 2/2] sheepdog: support 'qemu-img snapshot -a'

[Liu Yuan]

On 06/08/2013 12:48 AM, Kevin Wolf wrote:
> Hm, yes, I was confused. :-)
> 
> Anyway, the options stay the same: Either C1 must somehow inherit the VM
> state from A on the Sheepdog level, or we must make sure to get this
> order:
> 
> 1. Switch to (read-only) snapshot A
> 2. Load the VM state
> 3. Create (writable) snapshot C1 and switch to it
> 
> This is a different order as required for qcow2 (where C1 would inherit
> the VM state from A, so our current first 1, then 3, then 2 works), but
> if we can't fix it on the Sheepdog side, we'll have to touch the core
> code of qemu. I don't like much to touch the block.c code for it, but if
> it's required to fix a bug, let's just do it.

Hmm, I found actually we can inherit A's vdi_id from C1. I've sent v3
and I think this version will fix the possible bug you pointed out and
do loadvm the way core code assumes.

Thanks,
Yuan

[Qemu-devel] [PATCH v3 2/2] sheepdog: support 'qemu-img snapshot -a'

[Liu Yuan]

Just call sd_create_branch() in the snapshot_goto to rollback the image is good
enough. With this patch, 'loadvm' process for sheepdog is modified:

Suppose we have a snapshot chain A --> B --> C, we do 'loadvm A' so as to get
a new chain,

A --> B
|
V
C1

in the old code:

1 reload inode of A (in snapshot_goto)
2 read vmstate via A's vdi_id (loadvm_state)
3 delete C and create C1, reload inode of C1 (sd_create_branch on write)

with this patch applied:

1 reload inode of A, delete C and create C1  (in snapshot_goto)
2 read vmstate via C1's parent, that is A's vdi_id (loadvm_state)

This will fix the possible bug that QEMU exit between 2 and 3 in the old code

Cc: qemu-devel@nongnu.org
Cc: MORITA Kazutaka 
Cc: Kevin Wolf 
Cc: Stefan Hajnoczi 
Signed-off-by: Liu Yuan 
---
 block/sheepdog.c |9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index 94218ac..6d2cb9f 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -2071,14 +2071,11 @@ static int sd_snapshot_goto(BlockDriverState *bs, const 
char *snapshot_id)
 goto out;
 }
 
-if (!s->inode.vm_state_size) {
-error_report("Invalid snapshot");
-ret = -ENOENT;
+ret = sd_create_branch(s);
+if (ret) {
 goto out;
 }
 
-s->is_snapshot = true;
-
 g_free(old_s);
 
 return 0;
@@ -2210,7 +2207,7 @@ static int do_load_save_vmstate(BDRVSheepdogState *s, 
uint8_t *data,
 
 data_len = MIN(remaining, SD_DATA_OBJ_SIZE - offset);
 
-vmstate_oid = vid_to_vmstate_oid(s->inode.vdi_id, vdi_index);
+vmstate_oid = vid_to_vmstate_oid(s->inode.parent_vdi_id, vdi_index);
 
 create = (offset == 0);
 if (load) {
-- 
1.7.9.5

[Qemu-devel] [PATCH v3 1/2] sheepdog: fix snapshot tag initialization

[Liu Yuan]

This is an old and obvious bug. We should pass snapshot_id to the
tag. Or simple command like 'qemu-img snapshot -a tag sheepdog:image' will fail

Cc: qemu-devel@nongnu.org
Cc: MORITA Kazutaka 
Cc: Kevin Wolf 
Cc: Stefan Hajnoczi 
Reviewed-by: Kevin Wolf 
Signed-off-by: Liu Yuan 
---
 block/sheepdog.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index 21a4edf..94218ac 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -2063,7 +2063,7 @@ static int sd_snapshot_goto(BlockDriverState *bs, const 
char *snapshot_id)
 if (snapid) {
 tag[0] = 0;
 } else {
-pstrcpy(tag, sizeof(tag), s->name);
+pstrcpy(tag, sizeof(tag), snapshot_id);
 }
 
 ret = reload_inode(s, snapid, tag);
-- 
1.7.9.5

[Qemu-devel] [PATCH v3 0/2] fix 'qemu-img snapshot -a' operation for sheepdog

[Liu Yuan]

v3:
 - fix sheepdog's loadvm handling, don't rely on the write to create branch

v2:
 - add the comment to make things more clear
 - call sd_create_branch() after s->is_snapshot = true because after calling
   sd_create_branch, it is not snapshot anymore.

Nothing big, just two simple patches to enable this commind for sheepdog.

Cc: qemu-devel@nongnu.org
Cc: MORITA Kazutaka 
Cc: Kevin Wolf 
Cc: Stefan Hajnoczi 

Liu Yuan (2):
  sheepdog: fix snapshot tag initialization
  sheepdog: support 'qemu-img snapshot -a'

 block/sheepdog.c |   10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

-- 
1.7.9.5

Re: [Qemu-devel] [PATCH] script: git script to compile every commit in a range of commits

[Anthony Liguori]

Laszlo Ersek  writes:

> On 06/07/13 16:44, Jeff Cody wrote:
>
>> Thanks.  I can either do the above changes for a v2, or as follow on
>> patches.
>
> Whichever is easier for you, certainly! I'm fine with the script
> going-in as is.

A suggestion I'll make is to split the script into two parts.
git-bisect run is a terribly useful command and I use a bisect script
that looks like this:

#!/bin/sh

set -e

pushd ~/build/qemu
make -j1 || exit 1
popd

# Add right seed here
if test "$1"; then
"$@"
fi

I'm sure we all have bisect scripts like this.

What you're proposing is very similar to bisect but instead of doing a
binary search, it does a linear search starting from the oldest commit.
Basically:

#!/bin/sh

refspec="$1"
shift

git rev-list $refspec | while read commit; do
git checkout $commit
"$@" || exit $?
done

And indeed, I have a local script called git-foreach to do exactly
this.  I suspect a nicer version would make a very good addition to the
git project.

So to bisect for a make check failure, I do:

  git bisect run bisect.sh make check

Or to bisect for a qemu-test failure:

  git bisect run bisect.sh qemu-test-regress.sh

With git-foreach, you can do:

  git-foreach bisect.sh

To do a simple build test.  Or you can do:

  git-foreach git show checkpatch-head.sh

etc.

Regards,

Anthony Liguori

>
> Cheers,
> Laszlo

Re: [Qemu-devel] [PATCH 2/2] sheepdog: support 'qemu-img snapshot -a'

[Kevin Wolf]

Am 07.06.2013 um 18:14 hat Liu Yuan geschrieben:
> On 06/07/2013 11:22 PM, Kevin Wolf wrote:
> > Am 07.06.2013 um 15:48 hat Liu Yuan geschrieben:
> >> On 06/07/2013 03:31 PM, Kevin Wolf wrote:
> >>> Am 06.06.2013 um 15:09 hat Liu Yuan geschrieben:
>  On 06/06/2013 08:46 PM, Kevin Wolf wrote:
> > Am 06.06.2013 um 13:57 hat Liu Yuan geschrieben:
>  Only when the write comes from VM, we do the following stuff
>   - delete active vdi A
>   - created a new inode based on the previously reloaded As1's inode
> >>>
> >>> Thanks, this is the part that I missed.
> >>>
> >>> I'm not sure however why the actual switch is delayed until the first
> >>> write. This seems inconsistent with qcow2 snapshots.
> >>>
> >>> Do you know if there is a reason why we can't always do this already
> >>> during bdrv_snapshot_goto()?
> >>>
> >>
> >> I think the reason is sd_load_vmstate() need to load vm state objects
> >> with the correct inode object.
> >>
> >> I tried to remove
> >>
> >>   if (!s->inode.vm_state_size)
> >>
> >> and make sd_create_branch unconditional. This means 'loadvm' command
> >> will try to call sd_create_branch() inside sd_snapshot_goto(). But
> >> failed with reading the wrong snapshot because the vdi's inode object is
> >> changed by sd_create_branch().
> > 
> > Ok, I think I start to understand how these things work. Basically,
> > qemu's savevm functionality is designed to work with qcow2 like this:
> > 
> > 1. Save the VM state to the active layer
> > 2. create_snapshot saves the active layer including VM state
> > 3. [ Forget to remove the VM state from the active layer :-) ]
> > 4. loadvm loads the snapshot again, including VM state
> > 5. VM state is restored from the active layer
> > 
> > So for Sheepdog, the problem is that step 2 doesn't include the VM state,
> > right? So our options are either to change Sheepdog so that step 2 does
> > involve moving the VM state (might end up rather ugly) or you need to
> > swap steps 1 and 2, so that you first switch to the new snapshot and
> > then write the VM state.
> > 
> 
> Sorry, I didn't fully understand the above example. If sheepdog takes
> snapshots, vmstate will go with the exact current active disk into the
> cluster storage, for e.g
> 
> 1 we take two snapshots on the disk 'test' with tag A and B respectively
> 2 disk(A) + vmstate(A) will be stored as indexed by A's vdi
>   disk(B) + vmstate(B) will be stored as indexed by B's vdi
> 
> The chain is A --> B --> C, C is the current active vdi. Note, both A, B
> and C has different vdi_id.
> 
> The problem show up when we do loadvm A, the process is:
> 
> 1 reload inode of A (in snapshot_goto)
> 2 read vmstate via A's vdi_id (loadvm_state)
> 3 delete C and create C1, reload inode of C1 (sd_create_branch on write)
> 
> So if at stage 1, we call sd_create_branch(), the inode will point to C1
> and stage 2 will fail to read vmstate because vdi_id is C1's.
> 
> This is why we rely on the write to call sd_create_branch(). I am not
> sure if we can fix sheepdog's loadvm without touching the core code of QEMU.

Hm, yes, I was confused. :-)

Anyway, the options stay the same: Either C1 must somehow inherit the VM
state from A on the Sheepdog level, or we must make sure to get this
order:

1. Switch to (read-only) snapshot A
2. Load the VM state
3. Create (writable) snapshot C1 and switch to it

This is a different order as required for qcow2 (where C1 would inherit
the VM state from A, so our current first 1, then 3, then 2 works), but
if we can't fix it on the Sheepdog side, we'll have to touch the core
code of qemu. I don't like much to touch the block.c code for it, but if
it's required to fix a bug, let's just do it.

> > Because I think what we're doing currently is not only hard to
> > understand, but actually wrong. Consider this:
> > 
> > 1. savevm with a Sheepdog image
> > 2. Exit qemu before any write request is sent
> > 3. Restart qemu with the Sheepdog image and notice how the wrong
> >snapshot is active. Oops.
> > 
> 
> Sheepdog indeed has this problem.

Thanks for confirming.

Kevin

Re: [Qemu-devel] [PATCH] script: git script to compile every commit in a range of commits

[Peter Crosthwaite]

Hi Jeff,

On Sat, Jun 1, 2013 at 2:39 AM, Jeff Cody  wrote:
> This is a git script that will iterate through every commit in a
> specified range, and perform a configure and make.  The intention of
> this script is not to act as a check of code correctness, but to see if
> any commit breaks compilation of the tree.
>

Should we possibly throw a checkpatch in there?

> The idea is that prior to submitting a patch or patch series, the
> submitter should verify that no patch in the series breaks the build,
> thereby breaking git-bisect.  This script may also be useful for
> reviewers/maintainers dealing with multi-patch series.
>
> The range passed in to the script is in the form of a standard git range;
> the default is HEAD^!  (i.e. just the latest commit).
>
> Options may be passed in for configure, as well as make.  The log output
> filename and directory may also be optionally specified.  All options
> are stored via git-config.
>
> If everything compiled without error, then the exit code from the
> program will be 0.  Upon error, the stats for the offending
> commit will be shown.
>
> Signed-off-by: Jeff Cody 
> ---
>  scripts/git-compile-check | 202 
> ++
>  1 file changed, 202 insertions(+)
>  create mode 100755 scripts/git-compile-check
>
> diff --git a/scripts/git-compile-check b/scripts/git-compile-check
> new file mode 100755
> index 000..4b90f91
> --- /dev/null
> +++ b/scripts/git-compile-check
> @@ -0,0 +1,202 @@
> +#!/bin/bash
> +#
> +# See $desc, below, for program description
> +#
> +# Copyright (c) 2013 Red Hat, Inc.
> +#
> +# Author(s):
> +#   Jeff Cody 
> +#
> +# This program is free software; you can redistribute it and/or modify it 
> under
> +# the terms of the GNU General Public License as published by the Free 
> Software
> +# Foundation; under version 2 of the license
> +#
> +# This program is distributed in the hope that it will be useful, but WITHOUT
> +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or 
> FITNESS
> +# FOR A PARTICULAR PURPOSE.  See the GNU General Public License for more
> +# details.
> +#
> +# You should have received a copy of the GNU General Public License along 
> with
> +# this program; if not, see .
> +#
> +
> +set -C -u -e
> +set -o pipefail
> +
> +desc="
> +$0 iterates through a git commit range, and performs the
> +following on each commit:
> +  - git checkout
> +  - make clean
> +  - configure
> +  - make
> +
> +It will also optionally perform a git-reset and git-clean between
> +checkouts, if requested via the '-f' option.
> +
> +The script will exit and report on first error on any of the above steps,
> +(except no error checking is performed on 'make clean')
> +
> +NOTE: While executing, the script will checkout out each commit
> +  in the range in the current git tree.  On exit, the HEAD
> +  at the time the script was called is checked out"
> +
> +
> +# default range is the last commit
> +def_range="HEAD^!"
> +def_config_opt="--target-list=x86_64-softmmu"
> +# you may want to have make perform multiple jobs, e.g. -j4
> +# this is ommitted as the default in case the project makefile
> +# is not safe for parallel make processes
> +def_make_opt=""
> +def_log="output-$$.log"
> +def_logdir=""
> +force_clean='n'
> +
> +logfile=$def_log
> +range=`git config compile-check.range || true`
> +config_opt=`git config compile-check.configopt || true`
> +make_opt=`git config compile-check.makeopt || true`
> +logdir=`git config compile-check.logdir || true`
> +
> +if [[ -z "$range" ]]
> +then
> +range=$def_range
> +git config compile-check.range $range || true
> +fi
> +if [[ -z "$config_opt" ]]
> +then
> +config_opt=$def_config_opt
> +git config compile-check.configopt $config_opt || true
> +fi
> +if [[ -z "$make_opt" ]]
> +then
> +make_opt=$def_make_opt
> +git config compile-check.makeopt $make_opt || true
> +fi
> +if [[ -z "$logdir" ]]
> +then
> +logdir=$def_logdir
> +git config compile-check.logdir $logdir || true
> +fi
> +
> +usage() {
> +echo ""
> +echo "$0 [OPTIONS]"
> +echo "$desc"
> +echo ""
> +echo "OPTIONS:"
> +echo "  -r git range
> +optional; default is '$range'
> +"
> +echo "  -c configure options
> +optional; default is '$config_opt'
> +"
> +echo "  -m make options
> +optional; default is '$make_opt'
> +"
> +echo "  -d log dir
> +optional; default is '$logdir'
> +"
> +echo "  -l log filename
> +optional; default is output-PROCID, where PROCID is the bash 
> process id
> +note: you may specify a full path for the log filename here, and 
> exclude the
> +-d option.
> +"
> +echo "  -f force a git reset and clean
> +this will cause a 'git reset --h

[Qemu-devel] [PATCH 2/3] target-arm: implement LDA/STL instructions

[Mans Rullgard]

This adds support for the ARMv8 load acquire/store release instructions.
Since qemu does nothing special for memory barriers, these can be
emulated like their non-acquire/release counterparts.

Signed-off-by: Mans Rullgard 
---
 target-arm/translate.c | 91 ++
 1 file changed, 85 insertions(+), 6 deletions(-)

diff --git a/target-arm/translate.c b/target-arm/translate.c
index 96ac5bc..f529257 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -7274,14 +7274,54 @@ static void disas_arm_insn(CPUARMState * env, 
DisasContext *s)
 rd = (insn >> 12) & 0xf;
 if (insn & (1 << 23)) {
 /* load/store exclusive */
+int excl = (insn >> 9) & 1;
 op1 = (insn >> 21) & 0x3;
-if (op1)
+if (!excl)
+ARCH(8);
+else if (op1)
 ARCH(6K);
 else
 ARCH(6);
 addr = tcg_temp_local_new_i32();
 load_reg_var(s, addr, rn);
-if (insn & (1 << 20)) {
+if (!excl) {
+if (op1 == 1)
+goto illegal_op;
+tmp = tcg_temp_new_i32();
+if (insn & (1 << 20)) {
+switch (op1) {
+case 0: /* lda */
+tcg_gen_qemu_ld32u(tmp, addr, IS_USER(s));
+break;
+case 2: /* ldab */
+tcg_gen_qemu_ld8u(tmp, addr, IS_USER(s));
+break;
+case 3: /* ldah */
+tcg_gen_qemu_ld16u(tmp, addr, IS_USER(s));
+break;
+default:
+abort();
+}
+store_reg(s, rd, tmp);
+} else {
+rm = insn & 0xf;
+tmp = load_reg(s, rm);
+switch (op1) {
+case 0: /* stl */
+tcg_gen_qemu_st32(tmp, addr, IS_USER(s));
+break;
+case 2: /* stlb */
+tcg_gen_qemu_st8(tmp, addr, IS_USER(s));
+break;
+case 3: /* stlh */
+tcg_gen_qemu_st16(tmp, addr, IS_USER(s));
+break;
+default:
+abort();
+}
+tcg_temp_free_i32(tmp);
+}
+} else if (insn & (1 << 20)) {
 switch (op1) {
 case 0: /* ldrex */
 gen_load_exclusive(s, rd, 15, addr, 2);
@@ -8126,7 +8166,7 @@ static int disas_thumb2_insn(CPUARMState *env, 
DisasContext *s, uint16_t insn_hw
 gen_store_exclusive(s, rd, rs, 15, addr, 2);
 }
 tcg_temp_free_i32(addr);
-} else if ((insn & (1 << 6)) == 0) {
+} else if ((insn & (3 << 6)) == 0) {
 /* Table Branch.  */
 if (rn == 15) {
 addr = tcg_temp_new_i32();
@@ -8153,14 +8193,53 @@ static int disas_thumb2_insn(CPUARMState *env, 
DisasContext *s, uint16_t insn_hw
 store_reg(s, 15, tmp);
 } else {
 /* Load/store exclusive byte/halfword/doubleword.  */
-ARCH(7);
+if (((insn >> 6) & 3) != 1)
+ARCH(8);
+else
+ARCH(7);
 op = (insn >> 4) & 0x3;
-if (op == 2) {
+if (((insn >> 7) & 1) == 0 && op == 2) {
 goto illegal_op;
 }
 addr = tcg_temp_local_new_i32();
 load_reg_var(s, addr, rn);
-if (insn & (1 << 20)) {
+if ((insn & (1 << 6)) == 0) {
+if (op == 3)
+goto illegal_op;
+tmp = tcg_temp_new_i32();
+if (insn & (1 << 20)) {
+switch (op) {
+case 0: /* ldab */
+tcg_gen_qemu_ld8u(tmp, addr, IS_USER(s));
+break;
+case 1: 

[Qemu-devel] [PATCH 3/3] target-arm: explicitly decode SEVL instruction

[Mans Rullgard]

The ARMv8 SEVL instruction is in the architectural hint space already
emulated as nop.  This makes the decoding of SEVL explicit for clarity.

Signed-off-by: Mans Rullgard 
---
 target-arm/translate.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target-arm/translate.c b/target-arm/translate.c
index f529257..cfd148e 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -3501,6 +3501,7 @@ static void gen_nop_hint(DisasContext *s, int val)
 break;
 case 2: /* wfe */
 case 4: /* sev */
+case 5: /* sevl */
 /* TODO: Implement SEV and WFE.  May help SMP performance.  */
 default: /* nop */
 break;
-- 
1.8.2.1

Re: [Qemu-devel] [PATCH, resend] linux-user: environment variables

[Thomas Schwinge]

Hi!

On Wed, 29 May 2013 15:50:30 +0200, I wrote:
> I'm resending here the patches previously posted and described in the thread
> starting at
> .
> In short, fix a bug in util/envlist, code simplification, and then restore the
> original behavior of the -E and -U command-line options.

Ping.


Grüße,
 Thomas


pgpoYCVXxuGkf.pgp
Description: PGP signature

[Qemu-devel] [PATCH 1/3] target-arm: add feature flag for ARMv8

[Mans Rullgard]

Signed-off-by: Mans Rullgard 
---
 target-arm/cpu.c   | 5 -
 target-arm/cpu.h   | 1 +
 target-arm/translate.c | 1 +
 3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 496a59f..f5a1314 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -162,6 +162,9 @@ static void arm_cpu_realizefn(DeviceState *dev, Error 
**errp)
 CPUARMState *env = &cpu->env;
 
 /* Some features automatically imply others: */
+if (arm_feature(env, ARM_FEATURE_V8)) {
+set_feature(env, ARM_FEATURE_V7);
+}
 if (arm_feature(env, ARM_FEATURE_V7)) {
 set_feature(env, ARM_FEATURE_VAPA);
 set_feature(env, ARM_FEATURE_THUMB2);
@@ -748,7 +751,7 @@ static void pxa270c5_initfn(Object *obj)
 static void arm_any_initfn(Object *obj)
 {
 ARMCPU *cpu = ARM_CPU(obj);
-set_feature(&cpu->env, ARM_FEATURE_V7);
+set_feature(&cpu->env, ARM_FEATURE_V8);
 set_feature(&cpu->env, ARM_FEATURE_VFP4);
 set_feature(&cpu->env, ARM_FEATURE_VFP_FP16);
 set_feature(&cpu->env, ARM_FEATURE_NEON);
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 5438444..b3be588 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -392,6 +392,7 @@ enum arm_features {
 ARM_FEATURE_MPIDR, /* has cp15 MPIDR */
 ARM_FEATURE_PXN, /* has Privileged Execute Never bit */
 ARM_FEATURE_LPAE, /* has Large Physical Address Extension */
+ARM_FEATURE_V8,
 };
 
 static inline int arm_feature(CPUARMState *env, int feature)
diff --git a/target-arm/translate.c b/target-arm/translate.c
index b3f26d6..96ac5bc 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -42,6 +42,7 @@
 #define ENABLE_ARCH_6K   arm_feature(env, ARM_FEATURE_V6K)
 #define ENABLE_ARCH_6T2   arm_feature(env, ARM_FEATURE_THUMB2)
 #define ENABLE_ARCH_7 arm_feature(env, ARM_FEATURE_V7)
+#define ENABLE_ARCH_8 arm_feature(env, ARM_FEATURE_V8)
 
 #define ARCH(x) do { if (!ENABLE_ARCH_##x) goto illegal_op; } while(0)
 
-- 
1.8.2.1

Re: [Qemu-devel] [PATCH 2/2] sheepdog: support 'qemu-img snapshot -a'

[Liu Yuan]

On 06/07/2013 11:22 PM, Kevin Wolf wrote:
> Am 07.06.2013 um 15:48 hat Liu Yuan geschrieben:
>> On 06/07/2013 03:31 PM, Kevin Wolf wrote:
>>> Am 06.06.2013 um 15:09 hat Liu Yuan geschrieben:
 On 06/06/2013 08:46 PM, Kevin Wolf wrote:
> Am 06.06.2013 um 13:57 hat Liu Yuan geschrieben:
 Only when the write comes from VM, we do the following stuff
  - delete active vdi A
  - created a new inode based on the previously reloaded As1's inode
>>>
>>> Thanks, this is the part that I missed.
>>>
>>> I'm not sure however why the actual switch is delayed until the first
>>> write. This seems inconsistent with qcow2 snapshots.
>>>
>>> Do you know if there is a reason why we can't always do this already
>>> during bdrv_snapshot_goto()?
>>>
>>
>> I think the reason is sd_load_vmstate() need to load vm state objects
>> with the correct inode object.
>>
>> I tried to remove
>>
>>   if (!s->inode.vm_state_size)
>>
>> and make sd_create_branch unconditional. This means 'loadvm' command
>> will try to call sd_create_branch() inside sd_snapshot_goto(). But
>> failed with reading the wrong snapshot because the vdi's inode object is
>> changed by sd_create_branch().
> 
> Ok, I think I start to understand how these things work. Basically,
> qemu's savevm functionality is designed to work with qcow2 like this:
> 
> 1. Save the VM state to the active layer
> 2. create_snapshot saves the active layer including VM state
> 3. [ Forget to remove the VM state from the active layer :-) ]
> 4. loadvm loads the snapshot again, including VM state
> 5. VM state is restored from the active layer
> 
> So for Sheepdog, the problem is that step 2 doesn't include the VM state,
> right? So our options are either to change Sheepdog so that step 2 does
> involve moving the VM state (might end up rather ugly) or you need to
> swap steps 1 and 2, so that you first switch to the new snapshot and
> then write the VM state.
> 

Sorry, I didn't fully understand the above example. If sheepdog takes
snapshots, vmstate will go with the exact current active disk into the
cluster storage, for e.g

1 we take two snapshots on the disk 'test' with tag A and B respectively
2 disk(A) + vmstate(A) will be stored as indexed by A's vdi
  disk(B) + vmstate(B) will be stored as indexed by B's vdi

The chain is A --> B --> C, C is the current active vdi. Note, both A, B
and C has different vdi_id.

The problem show up when we do loadvm A, the process is:

1 reload inode of A (in snapshot_goto)
2 read vmstate via A's vdi_id (loadvm_state)
3 delete C and create C1, reload inode of C1 (sd_create_branch on write)

So if at stage 1, we call sd_create_branch(), the inode will point to C1
and stage 2 will fail to read vmstate because vdi_id is C1's.

This is why we rely on the write to call sd_create_branch(). I am not
sure if we can fix sheepdog's loadvm without touching the core code of QEMU.


> Because I think what we're doing currently is not only hard to
> understand, but actually wrong. Consider this:
> 
> 1. savevm with a Sheepdog image
> 2. Exit qemu before any write request is sent
> 3. Restart qemu with the Sheepdog image and notice how the wrong
>snapshot is active. Oops.
> 

Sheepdog indeed has this problem.

> Or doesn't Sheepdog have any notion of an "active snapshot" and this is
> only a runtime decision?
> 


 The chain will look like:

 As1 --> As2
  |
  V
  A

 This is how sheepdog handles savevm/loadvm.

 So for 'qemu-img snapshot -a', we should create the branch in the
 .bdrv_snapshot_goto.

 As you pointed out, we need to consider vm state even for 'snapshot -a',
 so I need to rework the patch 2/2.
>>>
>>> Yes, the presence of VM state is independent from whether you're using
>>> qemu-img or loadvm. And it actually goes both ways: qemu-img can revert
>>> to snapshots that have a VM state, and loadvm can be used with images
>>> that don't have a VM state (if you have multiple images, only one of
>>> them has the VM state).
>>>
>>
>> Seems not true of current code. If I 'loadvm' a snapshot without a
>> vmstate, I'll get 'qemu-system-x86_64: This is a disk-only snapshot.
>> Revert to it offline using qemu-img'.
> 
> Try again with multiple disks. Only one disk gets the VM state, the
> other ones get only the disk snapshot.
> 
>> But 'qemu-img snapshot -a' works as you said, it can rollback to the
>> snapshot regardless of vmstate.
>>
>> Also this is a difference to store vmstate for sheepdog images. *Every*
>> snapshot image can have its own vmstate stored in sheepdog cluster. That
>> is, we can have multiple snapshot with its own private vmstate for sheepdog.
> 
> Sorry, can't parse. :-(
> 

I misunderstood your statements, please ignore it.

Thanks,
Yuan

Re: [Qemu-devel] [PATCH] script: git script to compile every commit in a range of commits

[Laszlo Ersek]

On 06/07/13 16:44, Jeff Cody wrote:

> Thanks.  I can either do the above changes for a v2, or as follow on
> patches.

Whichever is easier for you, certainly! I'm fine with the script
going-in as is.

Cheers,
Laszlo

Re: [Qemu-devel] [PATCH v4] qemu-char: don't issue CHR_EVENT_OPEN in a BH

[Luiz Capitulino]

On Fri, 7 Jun 2013 09:56:00 -0500
mdroth  wrote:

> On Fri, Jun 07, 2013 at 09:50:39AM -0400, Luiz Capitulino wrote:
> > On Tue,  4 Jun 2013 16:35:09 -0500
> > Michael Roth  wrote:
> > 
> > > When CHR_EVENT_OPENED was initially added, it was CHR_EVENT_RESET,
> > > and it was issued as a bottom-half:
> > > 
> > > 86e94dea5b740dad65446c857f6959eae43e0ba6
> > > 
> > > Which we basically used to print out a greeting/prompt for the
> > > monitor.
> > > 
> > > AFAICT the only reason this was ever done in a BH was because in
> > > some cases we'd modify the chr_write handler for a new chardev
> > > backend *after* the site where we issued the reset (see:
> > > 86e94d:qemu_chr_open_stdio())
> > > 
> > > At some point this event was renamed to CHR_EVENT_OPENED, and we've
> > > maintained the use of this BH ever since.
> > > 
> > > However, due to 9f939df955a4152aad69a19a77e0898631bb2c18, we schedule
> > > the BH via g_idle_add(), which is causing events to sometimes be
> > > delivered after we've already begun processing data from backends,
> > > leading to:
> > > 
> > >  known bugs:
> > > 
> > >   QMP:
> > > session negotation resets with OPENED event, in some cases this
> > > is causing new sessions to get sporadically reset
> > > 
> > >  potential bugs:
> > > 
> > >   hw/usb/redirect.c:
> > > can_read handler checks for dev->parser != NULL, which may be
> > > true if CLOSED BH has not been executed yet. In the past, OPENED
> > > quiesced outstanding CLOSED events prior to us reading client
> > > data. If it's delayed, our check may allow reads to occur even
> > > though we haven't processed the OPENED event yet, and when we
> > > do finally get the OPENED event, our state may get reset.
> > > 
> > >   qtest.c:
> > > can begin session before OPENED event is processed, leading to
> > > a spurious reset of the system and irq_levels
> > > 
> > >   gdbstub.c:
> > > may start a gdb session prior to the machine being paused
> > > 
> > > To fix these, let's just drop the BH.
> > > 
> > > Since the initial reasoning for using it still applies to an extent,
> > > work around that by deferring the delivery of CHR_EVENT_OPENED until
> > > after the chardevs have been fully initialized, toward the end of
> > > qmp_chardev_add() (or some cases, qemu_chr_new_from_opts()). This
> > > defers delivery long enough that we can be assured a CharDriverState
> > > is fully initialized before CHR_EVENT_OPENED is sent.
> > > 
> > > Also, rather than requiring each chardev to do an explicit open, do it
> > > automatically, and allow the small few who don't desire such behavior to
> > > suppress the OPENED-on-init behavior by setting a 'explicit_be_open'
> > > flag.
> > > 
> > > We additionally add missing OPENED events for stdio backends on w32,
> > > which were previously not being issued, causing us to not recieve the
> > > banner and initial prompts for qmp/hmp.
> > > 
> > > Reported-by: Stefan Priebe 
> > > Cc: qemu-sta...@nongnu.org
> > > Signed-off-by: Michael Roth 
> > 
> > I don't know if the QMP queue is the ideal queue for this patch, but
> > I'd happily apply it if I get at least one Reviewed-by from the CC'ed
> > people.
> 
> Anthony actually added his Reviewed-by for v3, but I forgot to roll it
> in the commit. If you do take it in through your tree can you add that?

Sorry for the bureaucracy, but I don't like to add other people's tags
when the patch changes. Even when I'm sure they would be OK with the
new version.

Anthony, can you please add your Reviewed-by again?

> 
> Thanks!
> 
> > 
> > 
> > > ---
> > > v3->v4:
> > >  * renamed 'suppress_be_open_on_init' to 'explicit_be_open' to match
> > >existing 'explicit_fe_open' flag (Hans)
> > >  * added missing 'explicit_be_open' flags for spice vmc/port and
> > >msmouse backends
> > > 
> > > v2->v3:
> > >  * removed artifact in from v1 in backends/baum.c, test build with
> > >BRLAPI=y (Anthony)
> > >  * rebased on latest origin/master
> > > 
> > > v1->v2:
> > >  * default to sending OPENED on backend init, add flag to suppress
> > >it (Anthony)
> > >  * fix missing OPENED for stdio backends on w32
> > >  * fix missing OPENED when qemu_chr_new_from_opts() doesn't use
> > >qmp_chardev_add()
> > >  * clean up/update commit message
> > > 
> > >  backends/baum.c   |2 --
> > >  backends/msmouse.c|1 +
> > >  include/sysemu/char.h |2 +-
> > >  qemu-char.c   |   38 +-
> > >  spice-qemu-char.c |1 +
> > >  ui/console.c  |1 -
> > >  ui/gtk.c  |1 -
> > >  7 files changed, 20 insertions(+), 26 deletions(-)
> > > 
> > > diff --git a/backends/baum.c b/backends/baum.c
> > > index 4cba79f..62aa784 100644
> > > --- a/backends/baum.c
> > > +++ b/backends/baum.c
> > > @@ -611,8 +611,6 @@ CharDriverState *chr_baum_init(void)
> > >  
> > >  qemu_set_fd_handler(baum->brlapi_fd, baum_chr_read, NULL, baum);
> > >  
> > > -qemu_chr_

Re: [Qemu-devel] [PATCH 2/2] sheepdog: support 'qemu-img snapshot -a'

[Kevin Wolf]

Am 07.06.2013 um 15:48 hat Liu Yuan geschrieben:
> On 06/07/2013 03:31 PM, Kevin Wolf wrote:
> > Am 06.06.2013 um 15:09 hat Liu Yuan geschrieben:
> >> On 06/06/2013 08:46 PM, Kevin Wolf wrote:
> >>> Am 06.06.2013 um 13:57 hat Liu Yuan geschrieben:
> >> Only when the write comes from VM, we do the following stuff
> >>  - delete active vdi A
> >>  - created a new inode based on the previously reloaded As1's inode
> > 
> > Thanks, this is the part that I missed.
> > 
> > I'm not sure however why the actual switch is delayed until the first
> > write. This seems inconsistent with qcow2 snapshots.
> > 
> > Do you know if there is a reason why we can't always do this already
> > during bdrv_snapshot_goto()?
> > 
> 
> I think the reason is sd_load_vmstate() need to load vm state objects
> with the correct inode object.
> 
> I tried to remove
> 
>   if (!s->inode.vm_state_size)
> 
> and make sd_create_branch unconditional. This means 'loadvm' command
> will try to call sd_create_branch() inside sd_snapshot_goto(). But
> failed with reading the wrong snapshot because the vdi's inode object is
> changed by sd_create_branch().

Ok, I think I start to understand how these things work. Basically,
qemu's savevm functionality is designed to work with qcow2 like this:

1. Save the VM state to the active layer
2. create_snapshot saves the active layer including VM state
3. [ Forget to remove the VM state from the active layer :-) ]
4. loadvm loads the snapshot again, including VM state
5. VM state is restored from the active layer

So for Sheepdog, the problem is that step 2 doesn't include the VM state,
right? So our options are either to change Sheepdog so that step 2 does
involve moving the VM state (might end up rather ugly) or you need to
swap steps 1 and 2, so that you first switch to the new snapshot and
then write the VM state.

Because I think what we're doing currently is not only hard to
understand, but actually wrong. Consider this:

1. savevm with a Sheepdog image
2. Exit qemu before any write request is sent
3. Restart qemu with the Sheepdog image and notice how the wrong
   snapshot is active. Oops.

Or doesn't Sheepdog have any notion of an "active snapshot" and this is
only a runtime decision?

> >> The chain will look like:
> >>
> >> As1 --> As2
> >>  |
> >>  V
> >>  A
> >>
> >> This is how sheepdog handles savevm/loadvm.
> >>
> >> So for 'qemu-img snapshot -a', we should create the branch in the
> >> .bdrv_snapshot_goto.
> >>
> >> As you pointed out, we need to consider vm state even for 'snapshot -a',
> >> so I need to rework the patch 2/2.
> > 
> > Yes, the presence of VM state is independent from whether you're using
> > qemu-img or loadvm. And it actually goes both ways: qemu-img can revert
> > to snapshots that have a VM state, and loadvm can be used with images
> > that don't have a VM state (if you have multiple images, only one of
> > them has the VM state).
> > 
> 
> Seems not true of current code. If I 'loadvm' a snapshot without a
> vmstate, I'll get 'qemu-system-x86_64: This is a disk-only snapshot.
> Revert to it offline using qemu-img'.

Try again with multiple disks. Only one disk gets the VM state, the
other ones get only the disk snapshot.

> But 'qemu-img snapshot -a' works as you said, it can rollback to the
> snapshot regardless of vmstate.
> 
> Also this is a difference to store vmstate for sheepdog images. *Every*
> snapshot image can have its own vmstate stored in sheepdog cluster. That
> is, we can have multiple snapshot with its own private vmstate for sheepdog.

Sorry, can't parse. :-(

> I think my patch did the correct thing, just rollback the disk state of
> the snapshot for 'qemu-img snapshot -a'. Anyway, I found a minor issue
> of 2/2 patch, so I'll resend the set.

I think I agree that the problem is less with your patch, but more with
the preexisting code. We should get it sorted out anyway.

Kevin

Re: [Qemu-devel] [PATCH v2] create qemu_openpty_raw() helper function and move it to a separate file

[Brad Smith]

On 05/06/13 11:25 AM, Michael Tokarev wrote:

In two places qemu uses openpty() which is very system-dependent,
and in both places the pty is switched to raw mode as well.
Make a wrapper function which does both steps, and move all the
system-dependent complexity into a separate file, together
with static/local implementations of openpty() and cfmakeraw()
from qemu-char.c.

It is in a separate file, not part of oslib-posix.c, because
openpty() often resides in -lutil which is not linked to
every program qemu builds.

This change removes #including of , 
and other rather specific system headers out of qemu-common.h,
which isn't a place for such specific headers really.

Signed-off-by: Michael Tokarev 
---
Changes since v1:

- added a forgotten #include  for *BSD,
   which was recently added into qemu-common.h by
   Brad Smith, and which I intended to use in
   qemu-openpty.c too, but somehow forgot.

And one extra comment.  I especially took existing
code and used it almost unmodified, to have one
code moving change, with all other possible
improvements/cleanups to follow later.


This builds Ok on OpenBSD.


--
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.

Re: [Qemu-devel] [PATCH v4] qemu-char: don't issue CHR_EVENT_OPEN in a BH

[mdroth]

On Fri, Jun 07, 2013 at 09:50:39AM -0400, Luiz Capitulino wrote:
> On Tue,  4 Jun 2013 16:35:09 -0500
> Michael Roth  wrote:
> 
> > When CHR_EVENT_OPENED was initially added, it was CHR_EVENT_RESET,
> > and it was issued as a bottom-half:
> > 
> > 86e94dea5b740dad65446c857f6959eae43e0ba6
> > 
> > Which we basically used to print out a greeting/prompt for the
> > monitor.
> > 
> > AFAICT the only reason this was ever done in a BH was because in
> > some cases we'd modify the chr_write handler for a new chardev
> > backend *after* the site where we issued the reset (see:
> > 86e94d:qemu_chr_open_stdio())
> > 
> > At some point this event was renamed to CHR_EVENT_OPENED, and we've
> > maintained the use of this BH ever since.
> > 
> > However, due to 9f939df955a4152aad69a19a77e0898631bb2c18, we schedule
> > the BH via g_idle_add(), which is causing events to sometimes be
> > delivered after we've already begun processing data from backends,
> > leading to:
> > 
> >  known bugs:
> > 
> >   QMP:
> > session negotation resets with OPENED event, in some cases this
> > is causing new sessions to get sporadically reset
> > 
> >  potential bugs:
> > 
> >   hw/usb/redirect.c:
> > can_read handler checks for dev->parser != NULL, which may be
> > true if CLOSED BH has not been executed yet. In the past, OPENED
> > quiesced outstanding CLOSED events prior to us reading client
> > data. If it's delayed, our check may allow reads to occur even
> > though we haven't processed the OPENED event yet, and when we
> > do finally get the OPENED event, our state may get reset.
> > 
> >   qtest.c:
> > can begin session before OPENED event is processed, leading to
> > a spurious reset of the system and irq_levels
> > 
> >   gdbstub.c:
> > may start a gdb session prior to the machine being paused
> > 
> > To fix these, let's just drop the BH.
> > 
> > Since the initial reasoning for using it still applies to an extent,
> > work around that by deferring the delivery of CHR_EVENT_OPENED until
> > after the chardevs have been fully initialized, toward the end of
> > qmp_chardev_add() (or some cases, qemu_chr_new_from_opts()). This
> > defers delivery long enough that we can be assured a CharDriverState
> > is fully initialized before CHR_EVENT_OPENED is sent.
> > 
> > Also, rather than requiring each chardev to do an explicit open, do it
> > automatically, and allow the small few who don't desire such behavior to
> > suppress the OPENED-on-init behavior by setting a 'explicit_be_open'
> > flag.
> > 
> > We additionally add missing OPENED events for stdio backends on w32,
> > which were previously not being issued, causing us to not recieve the
> > banner and initial prompts for qmp/hmp.
> > 
> > Reported-by: Stefan Priebe 
> > Cc: qemu-sta...@nongnu.org
> > Signed-off-by: Michael Roth 
> 
> I don't know if the QMP queue is the ideal queue for this patch, but
> I'd happily apply it if I get at least one Reviewed-by from the CC'ed
> people.

Anthony actually added his Reviewed-by for v3, but I forgot to roll it
in the commit. If you do take it in through your tree can you add that?

Thanks!

> 
> 
> > ---
> > v3->v4:
> >  * renamed 'suppress_be_open_on_init' to 'explicit_be_open' to match
> >existing 'explicit_fe_open' flag (Hans)
> >  * added missing 'explicit_be_open' flags for spice vmc/port and
> >msmouse backends
> > 
> > v2->v3:
> >  * removed artifact in from v1 in backends/baum.c, test build with
> >BRLAPI=y (Anthony)
> >  * rebased on latest origin/master
> > 
> > v1->v2:
> >  * default to sending OPENED on backend init, add flag to suppress
> >it (Anthony)
> >  * fix missing OPENED for stdio backends on w32
> >  * fix missing OPENED when qemu_chr_new_from_opts() doesn't use
> >qmp_chardev_add()
> >  * clean up/update commit message
> > 
> >  backends/baum.c   |2 --
> >  backends/msmouse.c|1 +
> >  include/sysemu/char.h |2 +-
> >  qemu-char.c   |   38 +-
> >  spice-qemu-char.c |1 +
> >  ui/console.c  |1 -
> >  ui/gtk.c  |1 -
> >  7 files changed, 20 insertions(+), 26 deletions(-)
> > 
> > diff --git a/backends/baum.c b/backends/baum.c
> > index 4cba79f..62aa784 100644
> > --- a/backends/baum.c
> > +++ b/backends/baum.c
> > @@ -611,8 +611,6 @@ CharDriverState *chr_baum_init(void)
> >  
> >  qemu_set_fd_handler(baum->brlapi_fd, baum_chr_read, NULL, baum);
> >  
> > -qemu_chr_be_generic_open(chr);
> > -
> >  return chr;
> >  
> >  fail:
> > diff --git a/backends/msmouse.c b/backends/msmouse.c
> > index 0ac05a0..c0dbfcd 100644
> > --- a/backends/msmouse.c
> > +++ b/backends/msmouse.c
> > @@ -70,6 +70,7 @@ CharDriverState *qemu_chr_open_msmouse(void)
> >  chr = g_malloc0(sizeof(CharDriverState));
> >  chr->chr_write = msmouse_chr_write;
> >  chr->chr_close = msmouse_chr_close;
> > +chr->explicit_be_open = true;
> >  
> >  qemu_add_mouse_even

Re: [Qemu-devel] [PATCH] script: git script to compile every commit in a range of commits

[Jeff Cody]

On Thu, Jun 06, 2013 at 10:58:25AM +0200, Laszlo Ersek wrote:
> comments below
> 
> On 05/31/13 18:39, Jeff Cody wrote:
> 
> > +usage() {
> > +echo ""
> > +echo "$0 [OPTIONS]"
> > +echo "$desc"
> > +echo ""
> > +echo "OPTIONS:"
> > +echo "  -r git range
> > +optional; default is '$range'
> > +"
> > +echo "  -c configure options
> > +optional; default is '$config_opt'
> > +"
> > +echo "  -m make options
> > +optional; default is '$make_opt'
> > +"
> > +echo "  -d log dir
> > +optional; default is '$logdir'
> > +"
> > +echo "  -l log filename
> > +optional; default is output-PROCID, where PROCID is the bash 
> > process id
> > +note: you may specify a full path for the log filename here, 
> > and exclude the
> > +-d option.
> > +"
> > +echo "  -f force a git reset and clean
> > +this will cause a 'git reset --hard; git clean -fdx' to be run 
> > between checkouts.
> > +!! WARNING: This may cause data loss in your git tree.
> > +READ THE git-clean and git-reset man pages and make
> > +sure you understand the implications of
> > +'git clean -fdx' and 'git reset --hard' before 
> > using !!
> > +"
> > +echo "  -h help"
> > +}
> 
> Sorry for not noticing this before: is there any reason for the trailing
> spaces on most lines in the usage text?
>

No, not particularly, mainly just formatting with an extra newline,
and trying to keep the code somewhat lined up.

> Plus I suggest lower-casing the "THE" in "READ THE".
> 

I agree.

> > +
> > +while getopts ":r:c:m:l:d:hf" opt
> > +do
> > +case $opt in
> > +r) range=$OPTARG
> > +;;
> > +c) config_opt=$OPTARG
> > +;;
> > +m) make_opt=$OPTARG
> > +;;
> > +d) logdir=$OPTARG
> > +;;
> > +l) logfile=$OPTARG
> > +;;
> > +f) force_clean='y'
> > +;;
> > +h) usage
> > +   exit
> > +;;
> > +\?) echo "Unknown option: -$OPTARG" >&2
> > +usage
> > +exit 1
> > +;;
> > +esac
> > +done
> > +
> > +# append a '/' to logdir if $logdir was specified without one
> > +[[ -n "$logdir" ]] && [[ ${logdir:${#logdir}-1} != "/" ]] && 
> > logdir="${logdir}/"
> > +
> > +logfile="${logdir}${logfile}"
> > +
> > +head=`git rev-parse HEAD`
> > +total=`git rev-list "$range" |wc -l`
> > +
> > +echo "log output: $logfile"
> > +
> > +rm -f "$logfile"
> 
> rm -f -- "$logfile" is safer, but I doubt anyone would pass a pathname
> starting with "-"...

You are right, and no reason not to use '--', so I should add that in.

> 
> > +date > "$logfile"
> > +echo "git compile check for $range." >> "$logfile"
> > +echo "* configure options='$config_opt'" >> "$logfile"
> > +echo "* make options='$make_opt'" >> "$logfile"
> > +echo "Performing a test compile on $total patches" | tee -a "$logfile"
> > +echo "-" >> 
> > "$logfile"
> > +echo "" | tee -a "$logfile"
> > +
> > +clean_repo() {
> > +if [[ $force_clean == 'y' ]]
> > +then
> > +git reset --hard >> "$logfile" 2>&1 || true
> > +git clean -fdx -e "$logfile" >> "$logfile" 2>&1 || true
> > +fi
> > +}
> 
> Does "-e" mean "except"? It's not supported on RHEL-6.
> 

Yes - this way it will preserve the log output (the default log path
is in the current working directory).  Looks like the -e option was
not introduced until 1.7.3, and RHEL-6 is on 1.7.1.

I'll add a check for the git version, and drop the -e if git is too
old to support it (and in that case, the user will need to make sure
if they supply the -f option to this script that they specify a
different log location than the cwd).

> > +
> > +# we want to cleanup and return the git tree back to the previous head
> > +trap cleanup EXIT
> > +
> > +cleanup() {
> > +echo ""
> > +echo -n "Cleaning up..."
> > +clean_repo
> > +git checkout $head > /dev/null 2>&1
> > +echo "done."
> > +}
> > +
> > +cnt=1
> > +# don't pipe the git job into read, to avoid subshells
> > +while read hash
> > +do
> > +txt=`git log --pretty=tformat:"%h: %s" $hash^!`
> > +echo "${cnt}/${total}: compiling: $txt" | tee -a "$logfile"
> > +let cnt=$cnt+1;
> > +echo "" >> "$logfile"
> > +clean_repo
> > +make clean > /dev/null 2>&1 || true
> > +git checkout $hash  >> "$logfile" 2>&1 && \
> > +./configure $config_opt >> "$logfile" 2>&1 && \
> > +make $make_opt  >> "$logfile" 2>&1 ||
> > +(
> > +echo "" | tee -a "$logfile"
> > +echo "ERROR: commit $hash failed to build!" | tee -a

[Qemu-devel] Qemu crashed while unpluging IDE disk

[Gonglei (Arei)]

While starting a Fedora_14 guest, we came across a segfault of qemu:

the logs in /var/log/messages are: 
Jun  1 02:38:56 NC587 kernel: [403549.565754] show_signal_msg: 136 callbacks 
suppressed
Jun  1 02:38:56 NC587 kernel: [403549.565758] qemu-system-i38[25840]: segfault 
at 28 ip 00418d91 sp 7fe02aef4f00 error 4 in 
qemu-system-i386[40+35]

the very segfault refers to the code:
/*
 * Handle a read request in coroutine context
 */
static int coroutine_fn bdrv_co_do_readv(BlockDriverState *bs,
int64_t sector_num, int nb_sectors, QEMUIOVector *qiov,
BdrvRequestFlags flags)
{
BlockDriver *drv = bs->drv;//The segfault occurs when bs equals to NULL.
BdrvTrackedRequest req;
int ret;


NOTE: we are running on a XEN hypervisor with qemu 1.2.0

It's just serveral seconds after we start the guest. 
It seems that the guest is just begining to take charge of blockdev's io by 
itself from qemu, 
meanwhile,  qemu had just flushed disk io and unplugged the disk.
FYR: Our provided a pvdriver for the guest which takes charge of blkfront and 
netfront, 
thus, it triggered qemu to unplug the disk here.

We're confused why qemu would read disk io, after it had unplugged it. 

By checking the UNPLUG codes, we see a comment as shown below:

/*
 * Wait for pending requests to complete across all BlockDriverStates
 *
 * This function does not flush data to disk, use bdrv_flush_all() for that
 * after calling this function.
 *
 * Note that completion of an asynchronous I/O operation can trigger any
 * number of other I/O operations on other devices---for example a coroutine
 * can be arbitrarily complex and a constant flow of I/O can come until the
 * coroutine is complete.  Because of this, it is not possible to have a
 * function to drain a single device's I/O queue.
 */
void bdrv_drain_all(void)
{
BlockDriverState *bs;

Does that mean, as that we're now using coroutine mechnism to deal with disk 
io, there's possibility that we could come cross the problem described above?

Any ideas?  Thanks!

[Qemu-devel] [PATCH v2 1/2] sheepdog: fix snapshot tag initialization

[Liu Yuan]

This is an old and obvious bug. We should pass snapshot_id to the
tag. Or simple command like 'qemu-img snapshot -a tag sheepdog:image' will fail

Cc: qemu-devel@nongnu.org
Cc: MORITA Kazutaka 
Cc: Kevin Wolf 
Cc: Stefan Hajnoczi 
Reviewed-by: Kevin Wolf 
Signed-off-by: Liu Yuan 
---
 block/sheepdog.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index 21a4edf..94218ac 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -2063,7 +2063,7 @@ static int sd_snapshot_goto(BlockDriverState *bs, const 
char *snapshot_id)
 if (snapid) {
 tag[0] = 0;
 } else {
-pstrcpy(tag, sizeof(tag), s->name);
+pstrcpy(tag, sizeof(tag), snapshot_id);
 }
 
 ret = reload_inode(s, snapid, tag);
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH v2 16/16] Make qemu-io commands available in HMP

[Luiz Capitulino]

On Wed,  5 Jun 2013 14:19:41 +0200
Kevin Wolf  wrote:

> It was decided to not make this command available in QMP in order to
> make clear that this is not supposed to be a stable API and should be
> used only for testing and debugging purposes.

I like this as a temporary solution.

But I also see that commands seem to print to stderr, so this needs
work before being available in QMP. Even for HMP or the -debug
extension this is not desirable.

> 
> Signed-off-by: Kevin Wolf 
> ---
>  Makefile|  2 +-
>  Makefile.objs   |  1 +
>  hmp-commands.hx | 16 
>  hmp.c   | 18 ++
>  hmp.h   |  1 +
>  5 files changed, 37 insertions(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index 87298e5..9a77ae0 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -186,7 +186,7 @@ qemu-img.o: qemu-img-cmds.h
>  
>  qemu-img$(EXESUF): qemu-img.o $(block-obj-y) libqemuutil.a libqemustub.a
>  qemu-nbd$(EXESUF): qemu-nbd.o $(block-obj-y) libqemuutil.a libqemustub.a
> -qemu-io$(EXESUF): qemu-io.o qemu-io-cmds.o $(block-obj-y) libqemuutil.a 
> libqemustub.a
> +qemu-io$(EXESUF): qemu-io.o $(block-obj-y) libqemuutil.a libqemustub.a
>  
>  qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o
>  
> diff --git a/Makefile.objs b/Makefile.objs
> index 286ce06..5b288ba 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -13,6 +13,7 @@ block-obj-$(CONFIG_POSIX) += aio-posix.o
>  block-obj-$(CONFIG_WIN32) += aio-win32.o
>  block-obj-y += block/
>  block-obj-y += qapi-types.o qapi-visit.o
> +block-obj-y += qemu-io-cmds.o
>  
>  block-obj-y += qemu-coroutine.o qemu-coroutine-lock.o qemu-coroutine-io.o
>  block-obj-y += qemu-coroutine-sleep.o
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 9cea415..a6167bd 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1551,6 +1551,22 @@ Removes the chardev @var{id}.
>  ETEXI
>  
>  {
> +.name   = "qemu-io",
> +.args_type  = "device:B,command:s",
> +.params = "[device] \"[command]\"",
> +.help   = "run a qemu-io command on a block device",
> +.mhandler.cmd = hmp_qemu_io,
> +},
> +
> +STEXI
> +@item qemu-io @var{device} @var{command}
> +@findex qemu-io
> +
> +Executes a qemu-io command on the given block device.
> +
> +ETEXI
> +
> +{
>  .name   = "info",
>  .args_type  = "item:s?",
>  .params = "[subcommand]",
> diff --git a/hmp.c b/hmp.c
> index 4fb76ec..64e0baa 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -22,6 +22,7 @@
>  #include "qemu/sockets.h"
>  #include "monitor/monitor.h"
>  #include "ui/console.h"
> +#include "qemu-io.h"
>  
>  static void hmp_handle_error(Monitor *mon, Error **errp)
>  {
> @@ -1425,3 +1426,20 @@ void hmp_chardev_remove(Monitor *mon, const QDict 
> *qdict)
>  qmp_chardev_remove(qdict_get_str(qdict, "id"), &local_err);
>  hmp_handle_error(mon, &local_err);
>  }
> +
> +void hmp_qemu_io(Monitor *mon, const QDict *qdict)
> +{
> +BlockDriverState *bs;
> +const char* device = qdict_get_str(qdict, "device");
> +const char* command = qdict_get_str(qdict, "command");
> +Error *err = NULL;
> +
> +bs = bdrv_find(device);
> +if (bs) {
> +qemuio_command(bs, command);
> +} else {
> +error_set(&err, QERR_DEVICE_NOT_FOUND, device);
> +}
> +
> +hmp_handle_error(mon, &err);
> +}
> diff --git a/hmp.h b/hmp.h
> index 95fe76e..56d2e92 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -85,5 +85,6 @@ void hmp_nbd_server_add(Monitor *mon, const QDict *qdict);
>  void hmp_nbd_server_stop(Monitor *mon, const QDict *qdict);
>  void hmp_chardev_add(Monitor *mon, const QDict *qdict);
>  void hmp_chardev_remove(Monitor *mon, const QDict *qdict);
> +void hmp_qemu_io(Monitor *mon, const QDict *qdict);
>  
>  #endif

[Qemu-devel] [PATCH v2 2/2] sheepdog: support 'qemu-img snapshot -a'

[Liu Yuan]

Just call sd_create_branch() to rollback the image is good enough

Cc: qemu-devel@nongnu.org
Cc: MORITA Kazutaka 
Cc: Kevin Wolf 
Cc: Stefan Hajnoczi 
Signed-off-by: Liu Yuan 
---
 block/sheepdog.c |   17 -
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index 94218ac..c128b37 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -2071,14 +2071,21 @@ static int sd_snapshot_goto(BlockDriverState *bs, const 
char *snapshot_id)
 goto out;
 }
 
+s->is_snapshot = true;
+
 if (!s->inode.vm_state_size) {
-error_report("Invalid snapshot");
-ret = -ENOENT;
-goto out;
+/*
+ * qemu-img asks us to rollback, we can't rely on the write to create
+ * the branch, so do it right now. We can call sd_create_branch() right
+ * here only when vm_state_size == 0 because it changes vdi_id
+ * internally, otherwise we will read the wrong vmstate if any.
+ */
+ret = sd_create_branch(s);
+if (ret) {
+goto out;
+}
 }
 
-s->is_snapshot = true;
-
 g_free(old_s);
 
 return 0;
-- 
1.7.9.5

[Qemu-devel] [PATCH v2 0/2] fix 'qemu-img snapshot -a' operation for sheepdog

[Liu Yuan]

v2:
 - add the comment to make things more clear
 - call sd_create_branch() after s->is_snapshot = true because after calling
   sd_create_branch, it is not snapshot anymore.

Nothing big, just two simple patches to enable this commind for sheepdog.

Cc: qemu-devel@nongnu.org
Cc: MORITA Kazutaka 
Cc: Kevin Wolf 
Cc: Stefan Hajnoczi 

Liu Yuan (2):
  sheepdog: fix snapshot tag initialization
  sheepdog: support 'qemu-img snapshot -a'

 block/sheepdog.c |   10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

-- 
1.7.9.5

Re: [Qemu-devel] [PATCH 2/2] usb/host-libusb: Fix building with libusb git master code

[Ed Maste]

On 6 June 2013 10:39, Hans de Goede  wrote:
> +#if LIBUSBX_API_VERSION >= 0x01000102
> +rc = libusb_get_port_numbers(dev, path, 7);
> +#else
>  rc = libusb_get_port_path(ctx, dev, path, 7);
> +#endif

I just added libusb_get_port_numbers to FreeBSD's libusb, but we don't
have LIBUSBX_API_VERSION defined; we don't (yet) implement all of the
functionality that would be implied by given version.

I'm not sure if it'd be better to add the definition in FreeBSD's
libusb (and fault in missing functionality as needed, presumably), or
address it in libusb consumers with additional #ifdefs.

Re: [Qemu-devel] [PATCH 03/39] pci: split exit and finalize

[Anthony Liguori]

Paolo Bonzini  writes:

> To properly support devices that do DMA out of the BQL, destruction
> needs to be done in two phases.  First, the device is unrealized;
> at this point, pending memory accesses can still be completed, but
> no new accesses will be started.  The second part is freeing the
> device, which happens only after the reference count drops to zero;
> this means that all memory accesses are complete.
>
> This patch changes the PCI core to delay destruction of the
> bus-master address space and root region.
>
> Signed-off-by: Paolo Bonzini 

Reviewed-by: Anthony Liguori 

Regards,

Anthony Liguori

> ---
>  hw/pci/pci.c | 15 +++
>  1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 776ad96..b60d9d0d 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -779,6 +779,16 @@ static void pci_config_free(PCIDevice *pci_dev)
>  g_free(pci_dev->used);
>  }
>  
> +static void pci_device_instance_finalize(Object *obj)
> +{
> +PCIDevice *pci_dev = PCI_DEVICE(obj);
> +
> +qemu_free_irqs(pci_dev->irq);
> +
> +address_space_destroy(&pci_dev->bus_master_as);
> +memory_region_destroy(&pci_dev->bus_master_enable_region);
> +}
> +
>  /* -1 for devfn means auto assign */
>  static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
>   const char *name, int devfn)
> @@ -866,12 +876,8 @@ static PCIDevice *do_pci_register_device(PCIDevice 
> *pci_dev, PCIBus *bus,
>  
>  static void do_pci_unregister_device(PCIDevice *pci_dev)
>  {
> -qemu_free_irqs(pci_dev->irq);
>  pci_dev->bus->devices[pci_dev->devfn] = NULL;
>  pci_config_free(pci_dev);
> -
> -address_space_destroy(&pci_dev->bus_master_as);
> -memory_region_destroy(&pci_dev->bus_master_enable_region);
>  }
>  
>  static void pci_unregister_io_regions(PCIDevice *pci_dev)
> @@ -2243,6 +2249,7 @@ static const TypeInfo pci_device_type_info = {
>  .abstract = true,
>  .class_size = sizeof(PCIDeviceClass),
>  .class_init = pci_device_class_init,
> +.instance_finalize = pci_device_instance_finalize,
>  };
>  
>  static void pci_register_types(void)
> -- 
> 1.8.1.4

Re: [Qemu-devel] [PATCH 02/39] dma: keep a device alive while it has SGLists

[Anthony Liguori]

Andreas Färber  writes:

> Am 04.06.2013 20:51, schrieb Paolo Bonzini:
>> A QEMUSGList has a reference to a device's address space.  Keep
>> the device alive while the QEMUSGList exists.
>> 
>> Signed-off-by: Paolo Bonzini 
>> ---
>>  dma-helpers.c |  6 +-
>>  hw/ide/ahci.c |  3 ++-
>>  hw/ide/macio.c|  4 ++--
>>  hw/scsi/megasas.c |  4 ++--
>>  hw/scsi/virtio-scsi.c | 10 ++
>>  hw/usb/hcd-ehci.c |  4 ++--
>>  include/hw/pci/pci.h  |  2 +-
>>  include/sysemu/dma.h  |  4 +++-
>>  8 files changed, 23 insertions(+), 14 deletions(-)
> [...]
>> diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
>> index 7ee7d97..65ccb09 100644
>> --- a/hw/scsi/megasas.c
>> +++ b/hw/scsi/megasas.c
>> @@ -232,7 +232,7 @@ static int megasas_map_sgl(MegasasState *s, MegasasCmd 
>> *cmd, union mfi_sgl *sgl)
>>   MEGASAS_MAX_SGE);
>>  return iov_count;
>>  }
>> -qemu_sglist_init(&cmd->qsg, iov_count, pci_get_address_space(&s->dev));
>> +pci_dma_sglist_init(&cmd->qsg, &s->dev, iov_count);
>
> PCI_DEVICE(s)?

With same caveat at patch 1.

Reviewed-by: Anthony Liguori 

Regards,

Anthony Liguori

>
>>  for (i = 0; i < iov_count; i++) {
>>  dma_addr_t iov_pa, iov_size_p;
>>  
>> @@ -628,7 +628,7 @@ static int megasas_map_dcmd(MegasasState *s, MegasasCmd 
>> *cmd)
>>  }
>>  iov_pa = megasas_sgl_get_addr(cmd, &cmd->frame->dcmd.sgl);
>>  iov_size = megasas_sgl_get_len(cmd, &cmd->frame->dcmd.sgl);
>> -qemu_sglist_init(&cmd->qsg, 1, pci_get_address_space(&s->dev));
>> +pci_dma_sglist_init(&cmd->qsg, &s->dev, 1);
>
> Ditto?
>
>>  qemu_sglist_add(&cmd->qsg, iov_pa, iov_size);
>>  cmd->iov_size = iov_size;
>>  return cmd->iov_size;
> [snip]
>
> 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 01/39] scsi: keep device alive while it has requests

[Anthony Liguori]

Andreas Färber  writes:

> Am 04.06.2013 20:51, schrieb Paolo Bonzini:
>> Signed-off-by: Paolo Bonzini 
>> ---
>>  hw/scsi/scsi-bus.c | 4 
>>  1 file changed, 4 insertions(+)
>> 
>> diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
>> index 53ea906..e443193 100644
>> --- a/hw/scsi/scsi-bus.c
>> +++ b/hw/scsi/scsi-bus.c
>> @@ -516,6 +516,8 @@ SCSIRequest *scsi_req_alloc(const SCSIReqOps *reqops, 
>> SCSIDevice *d,
>>  req->status = -1;
>>  req->sense_len = 0;
>>  req->ops = reqops;
>> +object_ref(OBJECT(d));
>> +object_ref(OBJECT(req->bus->qbus.parent));
>
> BusState *bus = BUS(req->bus);
> ...
> object_ref(OBJECT(bus->parent));
>
> Same below.

If Paolo has to respin, ack.  But for both ways:

Reviewed-by: Anthony Liguori 

Regards,

Anthony Liguori

>
> Andreas
>
>>  trace_scsi_req_alloc(req->dev->id, req->lun, req->tag);
>>  return req;
>>  }
>> @@ -1505,6 +1507,8 @@ void scsi_req_unref(SCSIRequest *req)
>>  if (req->ops->free_req) {
>>  req->ops->free_req(req);
>>  }
>> +object_unref(OBJECT(req->dev));
>> +object_unref(OBJECT(bus->qbus.parent));
>>  g_free(req);
>>  }
>>  }
>> 
>
>
> -- 
> 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] [PULL 2/2] tap: fix NULL dereference when passing invalid parameters to tap

[Stefan Hajnoczi]

From: Jason Wang 

This patch forbid the following invalid parameters to tap:

1) fd and vhostfds were specified but vhostfd were not specified
2) vhostfds were specified but fds were not specified
3) fds and vhostfd were specified

For 1 and 2, net_init_tap_one() will still pass NULL as vhostfdname to
monitor_handle_fd_param(), which may crash the qemu.

Also remove the unnecessary has_fd check.

Cc: Paolo Bonzini 
Cc: Stefan Hajnoczi 
Cc: Laszlo Ersek 
Cc: qemu-sta...@nongnu.org
Signed-off-by: Jason Wang 
Signed-off-by: Stefan Hajnoczi 
---
 net/tap.c | 18 --
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/net/tap.c b/net/tap.c
index e0b7a2a..39c1cda 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -698,9 +698,10 @@ int net_init_tap(const NetClientOptions *opts, const char 
*name,
 if (tap->has_fd) {
 if (tap->has_ifname || tap->has_script || tap->has_downscript ||
 tap->has_vnet_hdr || tap->has_helper || tap->has_queues ||
-tap->has_fds) {
+tap->has_fds || tap->has_vhostfds) {
 error_report("ifname=, script=, downscript=, vnet_hdr=, "
- "helper=, queues=, and fds= are invalid with fd=");
+ "helper=, queues=, fds=, and vhostfds= "
+ "are invalid with fd=");
 return -1;
 }
 
@@ -725,9 +726,10 @@ int net_init_tap(const NetClientOptions *opts, const char 
*name,
 
 if (tap->has_ifname || tap->has_script || tap->has_downscript ||
 tap->has_vnet_hdr || tap->has_helper || tap->has_queues ||
-tap->has_fd) {
+tap->has_vhostfd) {
 error_report("ifname=, script=, downscript=, vnet_hdr=, "
- "helper=, queues=, and fd= are invalid with fds=");
+ "helper=, queues=, and vhostfd= "
+ "are invalid with fds=");
 return -1;
 }
 
@@ -765,9 +767,9 @@ int net_init_tap(const NetClientOptions *opts, const char 
*name,
 }
 } else if (tap->has_helper) {
 if (tap->has_ifname || tap->has_script || tap->has_downscript ||
-tap->has_vnet_hdr || tap->has_queues || tap->has_fds) {
+tap->has_vnet_hdr || tap->has_queues || tap->has_vhostfds) {
 error_report("ifname=, script=, downscript=, and vnet_hdr= "
- "queues=, and fds= are invalid with helper=");
+ "queues=, and vhostfds= are invalid with helper=");
 return -1;
 }
 
@@ -785,6 +787,10 @@ int net_init_tap(const NetClientOptions *opts, const char 
*name,
 return -1;
 }
 } else {
+if (tap->has_vhostfds) {
+error_report("vhostfds= is invalid if fds= wasn't specified");
+return -1;
+}
 script = tap->has_script ? tap->script : DEFAULT_NETWORK_SCRIPT;
 downscript = tap->has_downscript ? tap->downscript :
 DEFAULT_NETWORK_DOWN_SCRIPT;
-- 
1.8.1.4

[Qemu-devel] [PULL 1/2] vmxnet3: fix NICState cleanup

[Stefan Hajnoczi]

Use qemu_del_nic() instead of qemu_del_net_client() to correctly free
the entire NICState.

Cc: qemu-sta...@nongnu.org
Reported-by: Paolo Bonzini 
Signed-off-by: Stefan Hajnoczi 
---
 hw/net/vmxnet3.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index 5f483e7..4c575e5 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -1892,7 +1892,7 @@ static void vmxnet3_net_uninit(VMXNET3State *s)
 vmxnet_tx_pkt_reset(s->tx_pkt);
 vmxnet_tx_pkt_uninit(s->tx_pkt);
 vmxnet_rx_pkt_uninit(s->rx_pkt);
-qemu_del_net_client(qemu_get_queue(s->nic));
+qemu_del_nic(s->nic);
 }
 
 static void vmxnet3_net_init(VMXNET3State *s)
-- 
1.8.1.4

[Qemu-devel] [PULL 0/2] Net patches

[Stefan Hajnoczi]

The following changes since commit 8819c10b5d55d537d59a0ffd5d623f348fc36c47:

  Merge remote-tracking branch 'sstabellini/xen_fixes_20130603' into staging 
(2013-06-04 14:58:58 -0500)

are available in the git repository at:


  git://github.com/stefanha/qemu.git net

for you to fetch changes up to c87826a878be05208c3906eb9d5e1f37cff5e98e:

  tap: fix NULL dereference when passing invalid parameters to tap (2013-06-07 
15:48:11 +0200)


Jason Wang (1):
  tap: fix NULL dereference when passing invalid parameters to tap

Stefan Hajnoczi (1):
  vmxnet3: fix NICState cleanup

 hw/net/vmxnet3.c |  2 +-
 net/tap.c| 18 --
 2 files changed, 13 insertions(+), 7 deletions(-)

-- 
1.8.1.4

Re: [Qemu-devel] [PATCH v4] qemu-char: don't issue CHR_EVENT_OPEN in a BH

[Luiz Capitulino]

On Tue,  4 Jun 2013 16:35:09 -0500
Michael Roth  wrote:

> When CHR_EVENT_OPENED was initially added, it was CHR_EVENT_RESET,
> and it was issued as a bottom-half:
> 
> 86e94dea5b740dad65446c857f6959eae43e0ba6
> 
> Which we basically used to print out a greeting/prompt for the
> monitor.
> 
> AFAICT the only reason this was ever done in a BH was because in
> some cases we'd modify the chr_write handler for a new chardev
> backend *after* the site where we issued the reset (see:
> 86e94d:qemu_chr_open_stdio())
> 
> At some point this event was renamed to CHR_EVENT_OPENED, and we've
> maintained the use of this BH ever since.
> 
> However, due to 9f939df955a4152aad69a19a77e0898631bb2c18, we schedule
> the BH via g_idle_add(), which is causing events to sometimes be
> delivered after we've already begun processing data from backends,
> leading to:
> 
>  known bugs:
> 
>   QMP:
> session negotation resets with OPENED event, in some cases this
> is causing new sessions to get sporadically reset
> 
>  potential bugs:
> 
>   hw/usb/redirect.c:
> can_read handler checks for dev->parser != NULL, which may be
> true if CLOSED BH has not been executed yet. In the past, OPENED
> quiesced outstanding CLOSED events prior to us reading client
> data. If it's delayed, our check may allow reads to occur even
> though we haven't processed the OPENED event yet, and when we
> do finally get the OPENED event, our state may get reset.
> 
>   qtest.c:
> can begin session before OPENED event is processed, leading to
> a spurious reset of the system and irq_levels
> 
>   gdbstub.c:
> may start a gdb session prior to the machine being paused
> 
> To fix these, let's just drop the BH.
> 
> Since the initial reasoning for using it still applies to an extent,
> work around that by deferring the delivery of CHR_EVENT_OPENED until
> after the chardevs have been fully initialized, toward the end of
> qmp_chardev_add() (or some cases, qemu_chr_new_from_opts()). This
> defers delivery long enough that we can be assured a CharDriverState
> is fully initialized before CHR_EVENT_OPENED is sent.
> 
> Also, rather than requiring each chardev to do an explicit open, do it
> automatically, and allow the small few who don't desire such behavior to
> suppress the OPENED-on-init behavior by setting a 'explicit_be_open'
> flag.
> 
> We additionally add missing OPENED events for stdio backends on w32,
> which were previously not being issued, causing us to not recieve the
> banner and initial prompts for qmp/hmp.
> 
> Reported-by: Stefan Priebe 
> Cc: qemu-sta...@nongnu.org
> Signed-off-by: Michael Roth 

I don't know if the QMP queue is the ideal queue for this patch, but
I'd happily apply it if I get at least one Reviewed-by from the CC'ed
people.


> ---
> v3->v4:
>  * renamed 'suppress_be_open_on_init' to 'explicit_be_open' to match
>existing 'explicit_fe_open' flag (Hans)
>  * added missing 'explicit_be_open' flags for spice vmc/port and
>msmouse backends
> 
> v2->v3:
>  * removed artifact in from v1 in backends/baum.c, test build with
>BRLAPI=y (Anthony)
>  * rebased on latest origin/master
> 
> v1->v2:
>  * default to sending OPENED on backend init, add flag to suppress
>it (Anthony)
>  * fix missing OPENED for stdio backends on w32
>  * fix missing OPENED when qemu_chr_new_from_opts() doesn't use
>qmp_chardev_add()
>  * clean up/update commit message
> 
>  backends/baum.c   |2 --
>  backends/msmouse.c|1 +
>  include/sysemu/char.h |2 +-
>  qemu-char.c   |   38 +-
>  spice-qemu-char.c |1 +
>  ui/console.c  |1 -
>  ui/gtk.c  |1 -
>  7 files changed, 20 insertions(+), 26 deletions(-)
> 
> diff --git a/backends/baum.c b/backends/baum.c
> index 4cba79f..62aa784 100644
> --- a/backends/baum.c
> +++ b/backends/baum.c
> @@ -611,8 +611,6 @@ CharDriverState *chr_baum_init(void)
>  
>  qemu_set_fd_handler(baum->brlapi_fd, baum_chr_read, NULL, baum);
>  
> -qemu_chr_be_generic_open(chr);
> -
>  return chr;
>  
>  fail:
> diff --git a/backends/msmouse.c b/backends/msmouse.c
> index 0ac05a0..c0dbfcd 100644
> --- a/backends/msmouse.c
> +++ b/backends/msmouse.c
> @@ -70,6 +70,7 @@ CharDriverState *qemu_chr_open_msmouse(void)
>  chr = g_malloc0(sizeof(CharDriverState));
>  chr->chr_write = msmouse_chr_write;
>  chr->chr_close = msmouse_chr_close;
> +chr->explicit_be_open = true;
>  
>  qemu_add_mouse_event_handler(msmouse_event, chr, 0, "QEMU Microsoft 
> Mouse");
>  
> diff --git a/include/sysemu/char.h b/include/sysemu/char.h
> index 5e42c90..066c216 100644
> --- a/include/sysemu/char.h
> +++ b/include/sysemu/char.h
> @@ -70,12 +70,12 @@ struct CharDriverState {
>  void (*chr_set_echo)(struct CharDriverState *chr, bool echo);
>  void (*chr_set_fe_open)(struct CharDriverState *chr, int fe_open);
>  void *opaque;
> -int idle_tag;
>  char *label;
>  

Re: [Qemu-devel] [PATCH 2/2] sheepdog: support 'qemu-img snapshot -a'

[Liu Yuan]

On 06/07/2013 03:31 PM, Kevin Wolf wrote:
> Am 06.06.2013 um 15:09 hat Liu Yuan geschrieben:
>> On 06/06/2013 08:46 PM, Kevin Wolf wrote:
>>> Am 06.06.2013 um 13:57 hat Liu Yuan geschrieben:
 Just call sd_create_branch() to rollback the image is good enough

 Cc: qemu-devel@nongnu.org
 Cc: MORITA Kazutaka 
 Cc: Kevin Wolf 
 Cc: Stefan Hajnoczi 
 Signed-off-by: Liu Yuan 
 ---
  block/sheepdog.c |8 +---
  1 file changed, 5 insertions(+), 3 deletions(-)

 diff --git a/block/sheepdog.c b/block/sheepdog.c
 index 94218ac..cb5ca4a 100644
 --- a/block/sheepdog.c
 +++ b/block/sheepdog.c
 @@ -2072,9 +2072,11 @@ static int sd_snapshot_goto(BlockDriverState *bs, 
 const char *snapshot_id)
  }
  
  if (!s->inode.vm_state_size) {
 -error_report("Invalid snapshot");
 -ret = -ENOENT;
 -goto out;
 +/* qemu-img asks us to rollback, we need to do it right now */
 +ret = sd_create_branch(s);
 +if (ret) {
 +goto out;
 +}
  }
>>>
>>> I'm not sure how snapshots work internally for Sheepdog, but it seems
>>> odd to me that you need to do this only for disk-only snapshots, but not
>>> when the snapshot has VM state. (Also, note that 'qemu-img snapshot -a'
>>> works on images with a VM state, so the comment doesn't seem to be
>>> completely accurate)
>>>
>>> Would you mind explaining to me how this works in detail?
>>>
>>
>> Hmm, the original code isn't written by me and this snapshot mechanism
>> exists since day 0. I just hacks it to work now. So I'll try to explain
>> on my understanding.
>>
>> When we do a snapshot such as 'savedvm' or 'qemu-img snapshot', the
>> active vdi is snapshotted and marked as snapshot and a new vdi is
>> created as copy-on-write on the previous active vdi, then this new vdi
>> becomes active vdi. For e.g,
>>
>> As1 --> As2 --> A
>>
>> We take snapshot of vdi A twice, tagged s1 and s2 respectively. I guess
>> this is quit similar to qcow2 snapshots, only inode object with a bitmap
>> is created.
>>
>> So when we 'loadvm' or 'qemu-img snapshot -a' to A, current logic just
>> handle 'loadvm', that .bdrv_snapshot_goto only reloads inode object,
>> that is, for e.g, we 'savevm s1', and mark it as snapshot, the chain
>> would like
>>
>> As1 --> As2 --> A
>>  |
>>  v
>>  just reload As1's inode object
>>
>> Only when the write comes from VM, we do the following stuff
>>  - delete active vdi A
>>  - created a new inode based on the previously reloaded As1's inode
> 
> Thanks, this is the part that I missed.
> 
> I'm not sure however why the actual switch is delayed until the first
> write. This seems inconsistent with qcow2 snapshots.
> 
> Do you know if there is a reason why we can't always do this already
> during bdrv_snapshot_goto()?
> 

I think the reason is sd_load_vmstate() need to load vm state objects
with the correct inode object.

I tried to remove

  if (!s->inode.vm_state_size)

and make sd_create_branch unconditional. This means 'loadvm' command
will try to call sd_create_branch() inside sd_snapshot_goto(). But
failed with reading the wrong snapshot because the vdi's inode object is
changed by sd_create_branch().

>> The chain will look like:
>>
>> As1 --> As2
>>  |
>>  V
>>  A
>>
>> This is how sheepdog handles savevm/loadvm.
>>
>> So for 'qemu-img snapshot -a', we should create the branch in the
>> .bdrv_snapshot_goto.
>>
>> As you pointed out, we need to consider vm state even for 'snapshot -a',
>> so I need to rework the patch 2/2.
> 
> Yes, the presence of VM state is independent from whether you're using
> qemu-img or loadvm. And it actually goes both ways: qemu-img can revert
> to snapshots that have a VM state, and loadvm can be used with images
> that don't have a VM state (if you have multiple images, only one of
> them has the VM state).
> 

Seems not true of current code. If I 'loadvm' a snapshot without a
vmstate, I'll get 'qemu-system-x86_64: This is a disk-only snapshot.
Revert to it offline using qemu-img'.

But 'qemu-img snapshot -a' works as you said, it can rollback to the
snapshot regardless of vmstate.

Also this is a difference to store vmstate for sheepdog images. *Every*
snapshot image can have its own vmstate stored in sheepdog cluster. That
is, we can have multiple snapshot with its own private vmstate for sheepdog.

I think my patch did the correct thing, just rollback the disk state of
the snapshot for 'qemu-img snapshot -a'. Anyway, I found a minor issue
of 2/2 patch, so I'll resend the set.

Thanks,
Yuan

Re: [Qemu-devel] [PATCH v5] net: add support of mac-programming over macvtap in QEMU side

[Luiz Capitulino]

On Wed,  5 Jun 2013 18:42:13 +0800
Amos Kong  wrote:

> Currently macvtap based macvlan device is working in promiscuous
> mode, we want to implement mac-programming over macvtap through
> Libvirt for better performance.
> 
> Design:
> QEMU notifies Libvirt when rx-filter config is changed in guest,
> then Libvirt query the rx-filter information by a monitor command,
> and sync the change to macvtap device. Related rx-filter config
> of the nic contains main mac, rx-mode items and vlan table.
> 
> This patch adds a QMP event to notify management of rx-filter change,
> and adds a monitor command for management to query rx-filter
> information.
> 
> For reducing length of output, we just return the entries of vlan
> filter table that have active vlan.
> 
> Event_throttle API can avoid the events to flood QMP client, but it
> could cause an unexpected delay. So a flag for each nic is used to
> avoid events flooding, if management doesn't query rx-filter after
> it receives one event, new events won't be emitted to QMP monitor.
> 
> There maybe exist an uncontrollable delay if we let Libvirt do the
> real change, guests normally expect rx-filter updates immediately.
> But it's another separate issue, we can investigate it when the
> work in Libvirt side is done.

I think I completely misunderstood your testing results.

I had understood that: 1. changing the mac often & quick enough to
be a problem was a corner case and 2. you actually can overflow mngt

I hope I really got it wrong, otherwise you'll be using that
flag as a replacement for the event throttle API, which would be
a big mistake.

Can you please add your test results & analysis to this commit message?

Two small comments below.

> Signed-off-by: Amos Kong 
> ---
> v2: add argument to filter mac-table info of single nic (Stefan)
> update the document, add event notification
> v3: rename to rx-filter, add main mac, avoid events flooding (MST)
> fix error process (Stefan), fix qmp interface (Eric)
> v4: process qerror in hmp, cleanup (Luiz)
> set flag for each device, add device path in event, add
> helper for g_strdup_printf (MST)
> fix qmp document (Eric)
> v5: add path in doc, define notify flag to unsigned (Eric)
> add vlan table (Jason), drop monitor cmd
> ---
>  QMP/qmp-events.txt|  20 +
>  hw/net/virtio-net.c   | 112 
> ++
>  include/monitor/monitor.h |   1 +
>  include/net/net.h |   3 ++
>  monitor.c |   1 +
>  net/net.c |  47 +++
>  qapi-schema.json  |  89 
>  qmp-commands.hx   |  66 +++
>  8 files changed, 339 insertions(+)
> 
> diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
> index 92fe5fb..885230e 100644
> --- a/QMP/qmp-events.txt
> +++ b/QMP/qmp-events.txt
> @@ -172,6 +172,26 @@ Data:
>},
>"timestamp": { "seconds": 1265044230, "microseconds": 450486 } }
>  
> +NIC_RX_FILTER_CHANGED
> +-
> +
> +Emitted when rx-filter configuration of nic is changed by the guest.
> +Each nic has a flag to control event emit, the flag is set to false
> +when it emits one event of the nic, the flag is set to true when
> +management queries the rx-filter of the nic. This is used to avoid
> +events flooding.

Having this flag is an implementation detail. I think you should only
say that the event is emitted once until the query command is executed.

> +
> +Data:
> +
> +- "name": net client name (json-string)
> +- "path": device path (json-string)
> +
> +{ "event": "NIC_RX_FILTER_CHANGED",
> +  "data": { "name": "vnet0",
> +"path": "/machine/peripheral/vnet0/virtio-backend" },
> +  "timestamp": { "seconds": 1368697518, "microseconds": 326866 } }
> +}
> +
>  RESET
>  -
>  
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 1ea9556..ae1eab6 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -21,6 +21,8 @@
>  #include "hw/virtio/virtio-net.h"
>  #include "net/vhost_net.h"
>  #include "hw/virtio/virtio-bus.h"
> +#include "qapi/qmp/qjson.h"
> +#include "monitor/monitor.h"
>  
>  #define VIRTIO_NET_VM_VERSION11
>  
> @@ -192,6 +194,104 @@ static void virtio_net_set_link_status(NetClientState 
> *nc)
>  virtio_net_set_status(vdev, vdev->status);
>  }
>  
> +static void rxfilter_notify(NetClientState *nc)
> +{
> +QObject *event_data;
> +VirtIONet *n = qemu_get_nic_opaque(nc);
> +
> +if (nc->rxfilter_notify_enabled) {
> +event_data = qobject_from_jsonf("{ 'name': %s, 'path': %s }",
> +   n->netclient_name,
> +   object_get_canonical_path(OBJECT(n->qdev)));
> +monitor_protocol_event(QEVENT_NIC_RX_FILTER_CHANGED, event_data);
> +qobject_decref(event_data);
> +/* disable event notification to avoid events flooding */
> +nc->rxfilter_notify_enabled = 0;
> +}
> +}
> +
> +st

Re: [Qemu-devel] [PATCH v2] e600 core for MPC86xx processors

[Julio Guerra]

>>
>> However I can't judge whether all that code is right for e600 and
>> whether you may want to share some code with e500 / e5500 in some way?
>> CC'ing some Freescale folks.
>
>
> e600 is a very different core from e500/e5500.  It is a 74xx derivative.
> The only thing I can see that could be shared with e500 is the code to
> register SPRG4-7, but that's something that's already duplicated between a
> bunch of cores.
>

Yes, and as previously said, this is almost entirely a copy/paste from
the 7440 code, plus SPRGs 4..7, the high BATs and the POWERPC_MMU_32B
setting. It can be shared in a later patch.

--
Julio Guerra

Re: [Qemu-devel] [PATCH 5/5] trace-events: Fix up source file comments

[Andreas Färber]

Am 07.06.2013 12:59, schrieb Markus Armbruster:
> They're all wrong since (at least) Paolo's big source tree
> reorganization.  Need to shuffle some event declarations around to
> keep them under the correct source file comment.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  trace-events | 162 
> ---
>  1 file changed, 87 insertions(+), 75 deletions(-)
> 
> diff --git a/trace-events b/trace-events
> index e12d376..d95e903 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -25,18 +25,14 @@
>  #
>  # The  should be a sprintf()-compatible format string.
>  
> -# qemu-malloc.c
> -g_malloc(size_t size, void *ptr) "size %zu ptr %p"
> -g_realloc(void *ptr, size_t size, void *newptr) "ptr %p size %zu newptr %p"
> -g_free(void *ptr) "ptr %p"
> -
> -# osdep.c
> +# util/oslib-win32.c
> +# util/oslib-posix.c
[snip]

Thanks for taking time for this cleanup! I noticed before but haven't
found the time myself so far. ;)

If someone else feels like cleaning up, there's also quite a number of
header files which annotate prototypes with now wrong source files.

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 v2 0/6] Some -smbios work

[Laszlo Ersek]

On 06/07/13 15:00, Markus Armbruster wrote:

> v2: Address "Hawkeye" Laszlo's review

You're too kind, but it did crack me up :)

(Next time I'll miss something I'll have to hang my head in shame all
the more!)

> * 1-3/7 unchanged
> * Drop 4/7 because it's buggy, and the fixed version isn't worthwhile
> * Spelling fix in commit message of 5/7
> * Correct scanf format in 5-6/7

series
Reviewed-by: Laszlo "ever the optimist" Ersek 

Re: [Qemu-devel] [PATCH 00/39] Delay destruction of memory regions to instance_finalize

[Andreas Färber]

Hi Peter,

Am 07.06.2013 10:41, schrieb Peter Crosthwaite:
> I have a series that fixes all qom cast macros for all PCI devices tree
> wide. Can post. Qom cast macros added as needed.

Sounds promising! I just CC'ed you on my ISA series v2, which touches on
PCI_BUS() in the final patch, dropping FROM_QBUS().

> How are you regression testing this series? If you have a pc/PCI
> regression suite I could use it for my series.

I rely on `make check` to find the most obvious QOM errors.
libqos PCI support is still very new, so we don't have full device
coverage there yet.

Usually I run an openSUSE and/or SLES guest under KVM (pick your
favorite), which will exercise the relevant init, realize and reset code
paths at least.

And finally I re-review things in gitk before sending - not the nicest
tool but convenient for fixing up things and verifying quickly.

HTH,
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] 'qemu-nbd' explicit flush

[Mark Trumpold]

On 5/28/13 11:42 PM, "Stefan Hajnoczi"  wrote:

>On Tue, May 28, 2013 at 06:00:08PM +, Mark Trumpold wrote:
>> 
>> >-Original Message-
>> >From: Stefan Hajnoczi [mailto:stefa...@gmail.com]
>> >Sent: Monday, May 27, 2013 05:36 AM
>> >To: 'Mark Trumpold'
>> >Cc: 'Paolo Bonzini', qemu-devel@nongnu.org, ma...@tachyon.net
>> >Subject: Re: 'qemu-nbd' explicit flush
>> >
>> >On Sat, May 25, 2013 at 09:42:08AM -0800, Mark Trumpold wrote:
>> >> On 5/24/13 1:05 AM, "Stefan Hajnoczi"  wrote:
>> >> >On Thu, May 23, 2013 at 09:58:31PM +, Mark Trumpold wrote:
>> >> >One thing to be careful of is whether these operations are
>>asynchronous.
>> >> >The signal is asynchronous, you have no way of knowing when
>>qemu-nbd is
>> >> >finished flushing to the physical disk.
>> >>
>> >> Right, of course.  I missed the obvious.
>> >
>> >I missed something too.  Paolo may have already hinted at this when he
>> >posted a dd oflag=sync command-line option:
>> >
>> >blockdev --flushbufs is the wrong tool because ioctl(BLKFLSBUF) only
>> >writes out dirty pages to the block device.  It does *not* guarantee to
>> >send a flush request to the device.
>> >
>> >Therefore, the underlying image file may not be put into an up-to-date
>> >state by qemu-nbd.
>> >
>> >
>> >I suggest trying the following instead of blockdev --flushbufs:
>> >
>> >  python -c 'import os; os.fsync(open("/dev/loopX", "r+b"))'
>> >
>> >This should do the same as blockdev --flushbufs *plus* it sends and
>> >waits for the NBD FLUSH command.
>> >
>> >You may have to play with this command-line a little but the main idea
>> >is to open the block device and fsync it.
>> >
>> >Stefan
>> >
>> 
>> Hi Stefan,
>> 
>> One of my early experiments was adding a command line option to
>>'qemu-nbd' that did an open on 'device' (similar to the -c option), and
>>then calling 'fsync' on the 'device'.  By itself, I did not get a
>>complete flush to disk.  Was I missing something?
>> 
>> Empirically, the signal solution (blockdev --flushbufs plus
>>'bdrv_flush_all') was keeping my disk consistent.  My unit test
>>exercises the flush and snapshot pretty rigorously; that is, it never
>>passed before with 'qemu-nbd --cache=writeback ...'.  However, I did not
>>want to rely on 'sleep' for the race condition.
>> 
>> Is there any opportunity with the nbd client socket interface?  The
>>advantage for me there is not modifying 'qemu-nbd' source.
>
>I'm suggesting that you don't need to modify qemu-nbd.  If your host is
>running nbd.ko with flush support, then it should be enough to open the
>device and issue fsync(2).
>
>You can verify this using tcpdump(8) and checking that the NBD FLUSH
>command is really being sent by the host kernel.  If not, double check
>you're using the latest nbd.ko.
>
>Stefan


Stefan,

I tried the 'fsync' approach.  It apparently has no effect with my
3.3.1 Linux kernel and patch.  Changing kernels is not an option for me
at the moment, so I will revisit when we have an opportunity to upgrade
kernels, but for the moment I'll have to stick with 'cache=writethrough'.

Thank you again for your attention and help.

Best Regards,
Mark T.

[Qemu-devel] [PATCH v2 3/6] smbios: Convert to error_report()

[Markus Armbruster]

Improves diagnistics from ad hoc messages like

Invalid SMBIOS UUID string

to

qemu-system-x86_64: -smbios type=1,uuid=gaga: Invalid UUID

Signed-off-by: Markus Armbruster 
---
 arch_init.c  |  1 -
 hw/i386/smbios.c | 24 
 2 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index 5d32ecf..5d71870 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -1053,7 +1053,6 @@ void do_smbios_option(const char *optarg)
 {
 #ifdef TARGET_I386
 if (smbios_entry_add(optarg) < 0) {
-fprintf(stderr, "Wrong smbios provided\n");
 exit(1);
 }
 #endif
diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index c00bb2f..a67a328 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -13,6 +13,7 @@
  * GNU GPL, version 2 or (at your option) any later version.
  */
 
+#include "qemu/error-report.h"
 #include "sysemu/sysemu.h"
 #include "hw/i386/smbios.h"
 #include "hw/loader.h"
@@ -48,8 +49,7 @@ static int smbios_type4_count = 0;
 static void smbios_validate_table(void)
 {
 if (smbios_type4_count && smbios_type4_count != smp_cpus) {
- fprintf(stderr,
- "Number of SMBIOS Type 4 tables must match cpu count.\n");
+error_report("Number of SMBIOS Type 4 tables must match cpu count");
 exit(1);
 }
 }
@@ -82,16 +82,16 @@ static void smbios_check_collision(int type, int entry)
 if (entry == SMBIOS_TABLE_ENTRY && header->type == SMBIOS_FIELD_ENTRY) 
{
 struct smbios_field *field = (void *)header;
 if (type == field->type) {
-fprintf(stderr, "SMBIOS type %d field already defined, "
-"cannot add table\n", type);
+error_report("SMBIOS type %d field already defined, "
+ "cannot add table", type);
 exit(1);
 }
 } else if (entry == SMBIOS_FIELD_ENTRY &&
header->type == SMBIOS_TABLE_ENTRY) {
 struct smbios_structure_header *table = (void *)(header + 1);
 if (type == table->type) {
-fprintf(stderr, "SMBIOS type %d table already defined, "
-"cannot add field\n", type);
+error_report("SMBIOS type %d table already defined, "
+ "cannot add field", type);
 exit(1);
 }
 }
@@ -166,7 +166,7 @@ static void smbios_build_type_1_fields(const char *t)
  strlen(buf) + 1, buf);
 if (get_param_value(buf, sizeof(buf), "uuid", t)) {
 if (qemu_uuid_parse(buf, qemu_uuid) != 0) {
-fprintf(stderr, "Invalid SMBIOS UUID string\n");
+error_report("Invalid UUID");
 exit(1);
 }
 }
@@ -188,7 +188,7 @@ int smbios_entry_add(const char *t)
 int size = get_image_size(buf);
 
 if (size == -1 || size < sizeof(struct smbios_structure_header)) {
-fprintf(stderr, "Cannot read smbios file %s\n", buf);
+error_report("Cannot read SMBIOS file %s", buf);
 exit(1);
 }
 
@@ -204,7 +204,7 @@ int smbios_entry_add(const char *t)
 table->header.length = cpu_to_le16(sizeof(*table) + size);
 
 if (load_image(buf, table->data) != size) {
-fprintf(stderr, "Failed to load smbios file %s", buf);
+error_report("Failed to load SMBIOS file %s", buf);
 exit(1);
 }
 
@@ -230,12 +230,12 @@ int smbios_entry_add(const char *t)
 smbios_build_type_1_fields(t);
 return 0;
 default:
-fprintf(stderr, "Don't know how to build fields for SMBIOS type "
-"%ld\n", type);
+error_report("Don't know how to build fields for SMBIOS type %ld",
+ type);
 exit(1);
 }
 }
 
-fprintf(stderr, "smbios: must specify type= or file=\n");
+error_report("Must specify type= or file=");
 return -1;
 }
-- 
1.7.11.7

[Qemu-devel] [PULL 21/26] ide-test: Add FLUSH CACHE test case

[Stefan Hajnoczi]

From: Kevin Wolf 

This checks in particular that BSY is set while the flush request is in
flight.

Signed-off-by: Kevin Wolf 
Signed-off-by: Stefan Hajnoczi 
---
 tests/ide-test.c | 40 
 1 file changed, 40 insertions(+)

diff --git a/tests/ide-test.c b/tests/ide-test.c
index 1c31a2e..828e71a 100644
--- a/tests/ide-test.c
+++ b/tests/ide-test.c
@@ -77,6 +77,7 @@ enum {
 enum {
 CMD_READ_DMA= 0xc8,
 CMD_WRITE_DMA   = 0xca,
+CMD_FLUSH_CACHE = 0xe7,
 CMD_IDENTIFY= 0xec,
 
 CMDF_ABORT  = 0x100,
@@ -424,6 +425,43 @@ static void test_identify(void)
 ide_test_quit();
 }
 
+static void test_flush(void)
+{
+uint8_t data;
+
+ide_test_start(
+"-vnc none "
+"-drive file=blkdebug::%s,if=ide,cache=writeback",
+tmp_path);
+
+/* Delay the completion of the flush request until we explicitly do it */
+qmp("{'execute':'human-monitor-command', 'arguments': { "
+"'command-line': 'qemu-io ide0-hd0 \"break flush_to_os A\"'} }");
+
+/* FLUSH CACHE command on device 0*/
+outb(IDE_BASE + reg_device, 0);
+outb(IDE_BASE + reg_command, CMD_FLUSH_CACHE);
+
+/* Check status while request is in flight*/
+data = inb(IDE_BASE + reg_status);
+assert_bit_set(data, BSY | DRDY);
+assert_bit_clear(data, DF | ERR | DRQ);
+
+/* Complete the command */
+qmp("{'execute':'human-monitor-command', 'arguments': { "
+"'command-line': 'qemu-io ide0-hd0 \"resume A\"'} }");
+
+/* Check registers */
+data = inb(IDE_BASE + reg_device);
+g_assert_cmpint(data & DEV, ==, 0);
+
+data = inb(IDE_BASE + reg_status);
+assert_bit_set(data, DRDY);
+assert_bit_clear(data, BSY | DF | ERR | DRQ);
+
+ide_test_quit();
+}
+
 int main(int argc, char **argv)
 {
 const char *arch = qtest_get_arch();
@@ -454,6 +492,8 @@ int main(int argc, char **argv)
 qtest_add_func("/ide/bmdma/long_prdt", test_bmdma_long_prdt);
 qtest_add_func("/ide/bmdma/teardown", test_bmdma_teardown);
 
+qtest_add_func("/ide/flush", test_flush);
+
 ret = g_test_run();
 
 /* Cleanup */
-- 
1.8.1.4

[Qemu-devel] [PATCH v2 4/6] smbios: Clean up smbios_add_field() parameters

[Markus Armbruster]

Having size precede the associated pointer is odd.  Swap them, and fix
up the types.

Signed-off-by: Markus Armbruster 
---
 arch_init.c  |  2 +-
 hw/i386/smbios.c | 26 ++
 include/hw/i386/smbios.h |  2 +-
 3 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index 5d71870..872020e 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -1029,7 +1029,7 @@ int qemu_uuid_parse(const char *str, uint8_t *uuid)
 return -1;
 }
 #ifdef TARGET_I386
-smbios_add_field(1, offsetof(struct smbios_type_1, uuid), 16, uuid);
+smbios_add_field(1, offsetof(struct smbios_type_1, uuid), uuid, 16);
 #endif
 return 0;
 }
diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index a67a328..322f0a0 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -99,7 +99,7 @@ static void smbios_check_collision(int type, int entry)
 }
 }
 
-void smbios_add_field(int type, int offset, int len, void *data)
+void smbios_add_field(int type, int offset, const void *data, size_t len)
 {
 struct smbios_field *field;
 
@@ -130,21 +130,23 @@ static void smbios_build_type_0_fields(const char *t)
 
 if (get_param_value(buf, sizeof(buf), "vendor", t))
 smbios_add_field(0, offsetof(struct smbios_type_0, vendor_str),
- strlen(buf) + 1, buf);
+ buf, strlen(buf) + 1);
 if (get_param_value(buf, sizeof(buf), "version", t))
 smbios_add_field(0, offsetof(struct smbios_type_0, bios_version_str),
- strlen(buf) + 1, buf);
+ buf, strlen(buf) + 1);
 if (get_param_value(buf, sizeof(buf), "date", t))
 smbios_add_field(0, offsetof(struct smbios_type_0,
  bios_release_date_str),
- strlen(buf) + 1, buf);
+ buf, strlen(buf) + 1);
 if (get_param_value(buf, sizeof(buf), "release", t)) {
 int major, minor;
 sscanf(buf, "%d.%d", &major, &minor);
 smbios_add_field(0, offsetof(struct smbios_type_0,
- system_bios_major_release), 1, &major);
+ system_bios_major_release),
+ &major, 1);
 smbios_add_field(0, offsetof(struct smbios_type_0,
- system_bios_minor_release), 1, &minor);
+ system_bios_minor_release),
+ &minor, 1);
 }
 }
 
@@ -154,16 +156,16 @@ static void smbios_build_type_1_fields(const char *t)
 
 if (get_param_value(buf, sizeof(buf), "manufacturer", t))
 smbios_add_field(1, offsetof(struct smbios_type_1, manufacturer_str),
- strlen(buf) + 1, buf);
+ buf, strlen(buf) + 1);
 if (get_param_value(buf, sizeof(buf), "product", t))
 smbios_add_field(1, offsetof(struct smbios_type_1, product_name_str),
- strlen(buf) + 1, buf);
+ buf, strlen(buf) + 1);
 if (get_param_value(buf, sizeof(buf), "version", t))
 smbios_add_field(1, offsetof(struct smbios_type_1, version_str),
- strlen(buf) + 1, buf);
+ buf, strlen(buf) + 1);
 if (get_param_value(buf, sizeof(buf), "serial", t))
 smbios_add_field(1, offsetof(struct smbios_type_1, serial_number_str),
- strlen(buf) + 1, buf);
+ buf, strlen(buf) + 1);
 if (get_param_value(buf, sizeof(buf), "uuid", t)) {
 if (qemu_uuid_parse(buf, qemu_uuid) != 0) {
 error_report("Invalid UUID");
@@ -172,10 +174,10 @@ static void smbios_build_type_1_fields(const char *t)
 }
 if (get_param_value(buf, sizeof(buf), "sku", t))
 smbios_add_field(1, offsetof(struct smbios_type_1, sku_number_str),
- strlen(buf) + 1, buf);
+ buf, strlen(buf) + 1);
 if (get_param_value(buf, sizeof(buf), "family", t))
 smbios_add_field(1, offsetof(struct smbios_type_1, family_str),
- strlen(buf) + 1, buf);
+ buf, strlen(buf) + 1);
 }
 
 int smbios_entry_add(const char *t)
diff --git a/include/hw/i386/smbios.h b/include/hw/i386/smbios.h
index 94e3641..9babeaf 100644
--- a/include/hw/i386/smbios.h
+++ b/include/hw/i386/smbios.h
@@ -14,7 +14,7 @@
  */
 
 int smbios_entry_add(const char *t);
-void smbios_add_field(int type, int offset, int len, void *data);
+void smbios_add_field(int type, int offset, const void *data, size_t len);
 uint8_t *smbios_get_table(size_t *length);
 
 /*
-- 
1.7.11.7

[Qemu-devel] [PATCH v2 0/6] Some -smbios work

[Markus Armbruster]

Better error messages, a bit of code cleanup, and a big endian fix.

Not addressed: qemu_uuid_parse() sets an SMBIOS field by side effect.
Gross!

Testing:
* Verify error messages improve for
-smbios gaga
-smbios file=gaga
-smbios type=42
-smbios type=1,uuid=gaga
-smbios type=0,release=gaga
* Verify SMBIOS table remains unchanged
-smbios type=0,vendor=me,version=42,date=today,release=1.2 -smbios 
type=1,manufacturer=me,product=crap,version=6,serial=77,uuid=988bc9dd-0986-440f-ac24-cf9626c5aa88,sku=888,family=flintstones
  by sticking
  qemu_hexdump(smbios_entries, stdout, "SMBIOS", smbios_entries_len);
  into smbios_get_table()

v2: Address "Hawkeye" Laszlo's review
* 1-3/7 unchanged
* Drop 4/7 because it's buggy, and the fixed version isn't worthwhile
* Spelling fix in commit message of 5/7
* Correct scanf format in 5-6/7

Markus Armbruster (6):
  error-report.h: Supply missing include
  log.h: Supply missing includes
  smbios: Convert to error_report()
  smbios: Clean up smbios_add_field() parameters
  smbios: Fix -smbios type=0,release=... for big endian hosts
  smbios: Check R in -smbios type=0,release=R parses okay

 arch_init.c |  3 +--
 hw/i386/smbios.c| 57 -
 include/hw/i386/smbios.h|  2 +-
 include/qemu/error-report.h |  1 +
 include/qemu/log.h  |  3 +++
 5 files changed, 37 insertions(+), 29 deletions(-)

-- 
1.7.11.7

[Qemu-devel] [PULL 16/26] qemu-io: Use the qemu version for -V

[Stefan Hajnoczi]

From: Kevin Wolf 

Always printing 0.0.1 and never updating the version number wasn't very
useful. qemu-io is released with qemu, so using the same version number
makes most sense.

Signed-off-by: Kevin Wolf 
Signed-off-by: Stefan Hajnoczi 
---
 qemu-io.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/qemu-io.c b/qemu-io.c
index 514edcb..cb9def5 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -19,8 +19,6 @@
 #include "block/block_int.h"
 #include "trace/control.h"
 
-#define VERSION"0.0.1"
-
 #define CMD_NOFILE_OK   0x01
 
 char *progname;
@@ -380,7 +378,7 @@ int main(int argc, char **argv)
 }
 break;
 case 'V':
-printf("%s version %s\n", progname, VERSION);
+printf("%s version %s\n", progname, QEMU_VERSION);
 exit(0);
 case 'h':
 usage(progname);
-- 
1.8.1.4

[Qemu-devel] [PULL 06/26] qemu-io: Don't use global bs in command implementations

[Stefan Hajnoczi]

From: Kevin Wolf 

Pass in the BlockDriverState to the command handlers instead of using
the global variable. This is an important step to make the commands
usable outside of qemu-io.

Signed-off-by: Kevin Wolf 
Reviewed-by: Eric Blake 
Signed-off-by: Stefan Hajnoczi 
---
 cmd.c |   6 ++-
 cmd.h |   8 ++-
 qemu-io.c | 167 ++
 3 files changed, 101 insertions(+), 80 deletions(-)

diff --git a/cmd.c b/cmd.c
index 214c6f7..d501aab 100644
--- a/cmd.c
+++ b/cmd.c
@@ -57,7 +57,7 @@ check_command(
const cmdinfo_t *ci)
 {
if (check_func)
-   return check_func(ci);
+   return check_func(qemuio_bs, ci);
return 1;
 }
 
@@ -103,7 +103,7 @@ command(
return 0;
}
optind = 0;
-   return ct->cfunc(argc, argv);
+   return ct->cfunc(qemuio_bs, argc, argv);
 }
 
 const cmdinfo_t *
@@ -452,6 +452,7 @@ static cmdinfo_t quit_cmd;
 /* ARGSUSED */
 static int
 quit_f(
+BlockDriverState *bs,
int argc,
char**argv)
 {
@@ -490,6 +491,7 @@ help_all(void)
 
 static int
 help_f(
+BlockDriverState *bs,
int argc,
char**argv)
 {
diff --git a/cmd.h b/cmd.h
index 4dcfe88..ccf6336 100644
--- a/cmd.h
+++ b/cmd.h
@@ -17,9 +17,13 @@
 #ifndef __COMMAND_H__
 #define __COMMAND_H__
 
+#include "qemu-common.h"
+
 #define CMD_FLAG_GLOBAL((int)0x8000)   /* don't iterate "args" 
*/
 
-typedef int (*cfunc_t)(int argc, char **argv);
+extern BlockDriverState *qemuio_bs;
+
+typedef int (*cfunc_t)(BlockDriverState *bs, int argc, char **argv);
 typedef void (*helpfunc_t)(void);
 
 typedef struct cmdinfo {
@@ -41,7 +45,7 @@ extern intncmds;
 void help_init(void);
 void quit_init(void);
 
-typedef int (*checkfunc_t)(const cmdinfo_t *ci);
+typedef int (*checkfunc_t)(BlockDriverState *bs, const cmdinfo_t *ci);
 
 void add_command(const cmdinfo_t *ci);
 void add_user_command(char *optarg);
diff --git a/qemu-io.c b/qemu-io.c
index b4f56fc..39d7063 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -25,8 +25,8 @@
 #define CMD_NOFILE_OK   0x01
 
 char *progname;
-static BlockDriverState *bs;
 
+BlockDriverState *qemuio_bs;
 static int misalign;
 
 static int64_t cvtnum(const char *s)
@@ -63,7 +63,7 @@ static int parse_pattern(const char *arg)
  */
 
 #define MISALIGN_OFFSET 16
-static void *qemu_io_alloc(size_t len, int pattern)
+static void *qemu_io_alloc(BlockDriverState *bs, size_t len, int pattern)
 {
 void *buf;
 
@@ -136,7 +136,8 @@ static void print_report(const char *op, struct timeval *t, 
int64_t offset,
  * vector matching it.
  */
 static void *
-create_iovec(QEMUIOVector *qiov, char **argv, int nr_iov, int pattern)
+create_iovec(BlockDriverState *bs, QEMUIOVector *qiov, char **argv, int nr_iov,
+ int pattern)
 {
 size_t *sizes = g_new0(size_t, nr_iov);
 size_t count = 0;
@@ -172,7 +173,7 @@ create_iovec(QEMUIOVector *qiov, char **argv, int nr_iov, 
int pattern)
 
 qemu_iovec_init(qiov, nr_iov);
 
-buf = p = qemu_io_alloc(count, pattern);
+buf = p = qemu_io_alloc(bs, count, pattern);
 
 for (i = 0; i < nr_iov; i++) {
 qemu_iovec_add(qiov, p, sizes[i]);
@@ -184,7 +185,8 @@ fail:
 return buf;
 }
 
-static int do_read(char *buf, int64_t offset, int count, int *total)
+static int do_read(BlockDriverState *bs, char *buf, int64_t offset, int count,
+   int *total)
 {
 int ret;
 
@@ -196,7 +198,8 @@ static int do_read(char *buf, int64_t offset, int count, 
int *total)
 return 1;
 }
 
-static int do_write(char *buf, int64_t offset, int count, int *total)
+static int do_write(BlockDriverState *bs, char *buf, int64_t offset, int count,
+int *total)
 {
 int ret;
 
@@ -208,7 +211,8 @@ static int do_write(char *buf, int64_t offset, int count, 
int *total)
 return 1;
 }
 
-static int do_pread(char *buf, int64_t offset, int count, int *total)
+static int do_pread(BlockDriverState *bs, char *buf, int64_t offset, int count,
+int *total)
 {
 *total = bdrv_pread(bs, offset, (uint8_t *)buf, count);
 if (*total < 0) {
@@ -217,7 +221,8 @@ static int do_pread(char *buf, int64_t offset, int count, 
int *total)
 return 1;
 }
 
-static int do_pwrite(char *buf, int64_t offset, int count, int *total)
+static int do_pwrite(BlockDriverState *bs, char *buf, int64_t offset, int 
count,
+ int *total)
 {
 *total = bdrv_pwrite(bs, offset, (uint8_t *)buf, count);
 if (*total < 0) {
@@ -227,6 +232,7 @@ static int do_pwrite(char *buf, int64_t offset, int count, 
int *total)
 }
 
 typedef struct {
+BlockDriverState *bs;
 int64_t offset;
 int count;
 int *total;
@@ -238,7 +244,7 @@ static void coroutine_fn co_write_zeroes_entry(void *opaque)
 {
 CoWriteZeroes *data = opaque;
 
-data->ret = bdrv_co_write_zeroes(bs, data->offset / BDRV_SECTOR_SIZE,
+data->ret = bd

[Qemu-devel] [PATCH v2 00/12] QOM realize for ISA, part 2

[Andreas Färber]

Hello,

Here is my next batch of QOM realize patches for ISA devices.

For two "new" ISA devices QOM cast macros are introduced (gus, cs4231a);
these two and the new pvpanic ISA device are now converted, too.

Series is extended to clean up all ISABus and ISADevice uses;
it then becomes easy to get rid of FROM_QBUS() as next step.

Available from:
https://github.com/afaerber/qemu-cpu/commits/realize-isa.v2
git://github.com/afaerber/qemu-cpu.git realize-isa.v2

Regards,
Andreas

v1 -> v2:
* Most QOM'ifications were already applied.
* Rebased on file movements.
* Dropped \n from error_setg().
* gus.c and cs4231a.c are now compiled in and needed to be updated.
* pvpanic device was added and needed to be converted.
* Tidied errp argument/variable naming consistently.
* debugcon: Don't double-report error.
* parallel: Dropped \n.
* serial: Replaced fprintf()+exit().
* pc port92: Split off instance_init from realizefn.
* Appended patches renaming ISABus and ISADevice parent fields.
* Appended patch dropping FROM_QBUS() macro.

Cc: Anthony Liguori 
Cc: Blue Swirl 
Cc: Aurélien Jarno 

Cc: Paolo Bonzini 
Cc: malc 
Cc: Hu Tao 
Cc: Michael S. Tsirkin 
Cc: Peter C. Crosthwaite 

Andreas Färber (12):
  gus: QOM'ify some more
  cs4231a: QOM'ify some more
  isa: Use realizefn for ISADevice
  i8254: QOM'ify some more
  kvm/i8254: QOM'ify some more
  i8254: Convert PITCommonState to QOM realizefn
  i8259: QOM'ify some more
  kvm/i8259: QOM'ify some more
  i8259: Convert PICCommonState to use QOM realizefn
  isa: QOM'ify ISABus
  isa: QOM'ify ISADevice
  qdev: Drop FROM_QBUS() macro

 hw/audio/adlib.c  | 23 ---
 hw/audio/cs4231a.c| 38 +++--
 hw/audio/gus.c| 27 ++
 hw/audio/pcspk.c  | 19 -
 hw/audio/sb16.c   | 21 +-
 hw/block/fdc.c| 40 +++---
 hw/char/debugcon.c| 23 +--
 hw/char/parallel.c| 29 +++
 hw/char/serial-isa.c  | 34 --
 hw/char/serial-pci.c  | 17 +--
 hw/char/serial.c  | 22 +++
 hw/display/cirrus_vga.c   | 12 
 hw/display/vga-isa.c  | 17 ++-
 hw/dma/i82374.c   | 13 -
 hw/i2c/core.c |  4 +--
 hw/i386/kvm/i8254.c   | 59 ++-
 hw/i386/kvm/i8259.c   | 34 ++
 hw/i386/pc.c  | 28 ---
 hw/i386/pc_piix.c |  2 +-
 hw/ide/isa.c  | 16 +--
 hw/input/pckbd.c  | 29 +++
 hw/input/vmmouse.c|  8 ++
 hw/intc/i8259.c   | 48 ++-
 hw/intc/i8259_common.c| 34 ++
 hw/isa/i82378.c   |  4 +--
 hw/isa/isa-bus.c  | 25 -
 hw/isa/pc87312.c  | 12 
 hw/isa/piix4.c|  2 +-
 hw/isa/vt82c686.c |  2 +-
 hw/misc/applesmc.c| 10 +++
 hw/misc/debugexit.c   | 10 +++
 hw/misc/pc-testdev.c  | 11 
 hw/misc/pvpanic.c | 19 -
 hw/misc/sga.c |  7 ++---
 hw/misc/vmport.c  | 10 +++
 hw/net/ne2000-isa.c   | 15 +-
 hw/pci/pci.c  |  2 +-
 hw/ppc/prep.c |  7 +++--
 hw/sparc64/sun4u.c|  3 +-
 hw/ssi/ssi.c  |  2 +-
 hw/timer/i8254.c  | 28 ++-
 hw/timer/i8254_common.c   | 19 -
 hw/timer/m48t59.c | 22 +--
 hw/timer/mc146818rtc.c| 18 ++--
 hw/watchdog/wdt_ib700.c   |  8 ++
 include/hw/audio/pcspk.h  | 14 ++
 include/hw/char/serial.h  |  2 +-
 include/hw/i386/pc.h  | 30 +++-
 include/hw/isa/i8259_internal.h   |  2 +-
 include/hw/isa/isa.h  | 11 ++--
 include/hw/qdev-core.h|  2 --
 include/hw/timer/i8254.h  | 31 
 include/hw/timer/i8254_internal.h |  1 -
 53 files changed, 535 insertions(+), 391 deletions(-)

-- 
1.8.1.4

Re: [Qemu-devel] [PATCH v2 00/12] QOM realize for ISA, part 2

[Andreas Färber]

Am 07.06.2013 14:58, schrieb Andreas Färber:
> Cc: malc 

FWIW should be av1474, but either one bounces. ;)

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 v2 5/6] smbios: Fix -smbios type=0, release=... for big endian hosts

[Markus Armbruster]

Classic endianness bug due to careless dirty coding: assuming reading
a byte from an int variable gets the least significant byte.

Signed-off-by: Markus Armbruster 
---
 hw/i386/smbios.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index 322f0a0..6431dd4 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -127,6 +127,7 @@ void smbios_add_field(int type, int offset, const void 
*data, size_t len)
 static void smbios_build_type_0_fields(const char *t)
 {
 char buf[1024];
+unsigned char major, minor;
 
 if (get_param_value(buf, sizeof(buf), "vendor", t))
 smbios_add_field(0, offsetof(struct smbios_type_0, vendor_str),
@@ -139,8 +140,7 @@ static void smbios_build_type_0_fields(const char *t)
  bios_release_date_str),
  buf, strlen(buf) + 1);
 if (get_param_value(buf, sizeof(buf), "release", t)) {
-int major, minor;
-sscanf(buf, "%d.%d", &major, &minor);
+sscanf(buf, "%hhu.%hhu", &major, &minor);
 smbios_add_field(0, offsetof(struct smbios_type_0,
  system_bios_major_release),
  &major, 1);
-- 
1.7.11.7

[Qemu-devel] [PATCH v2 1/6] error-report.h: Supply missing include

[Markus Armbruster]

Missed in commit e5924d8.

Signed-off-by: Markus Armbruster 
---
 include/qemu/error-report.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/qemu/error-report.h b/include/qemu/error-report.h
index c902cc1..14c1719 100644
--- a/include/qemu/error-report.h
+++ b/include/qemu/error-report.h
@@ -14,6 +14,7 @@
 #define QEMU_ERROR_H
 
 #include 
+#include "qemu/compiler.h"
 
 typedef struct Location {
 /* all members are private to qemu-error.c */
-- 
1.7.11.7