Re: [Qemu-devel] [PATCH] block: allow write-threshold on device name
On Sun, Jun 7, 2015 at 9:38 AM, Eric Blake wrote:
>
> Commit e2462113 allowed the ability to fire an event if a BDS
> node exceeds a threshold during a write, but limited the option
> to only work on node names. For convenience, expand this to
> allow a device name as a way to set the threshold on the BDS
> at the active layer of the device.
>
> Signed-off-by: Eric Blake
> ---
> block/write-threshold.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/block/write-threshold.c b/block/write-threshold.c
> index a53c1f5..e3df419 100644
> --- a/block/write-threshold.c
> +++ b/block/write-threshold.c
> @@ -1,7 +1,7 @@
> /*
> * QEMU System Emulator block write threshold notification
> *
> - * Copyright Red Hat, Inc. 2014
> + * Copyright Red Hat, Inc. 2014, 2015
> *
> * Authors:
> * Francesco Romani
> @@ -110,9 +110,8 @@ void qmp_block_set_write_threshold(const char *node_name,
> BlockDriverState *bs;
> AioContext *aio_context;
>
> -bs = bdrv_find_node(node_name);
> +bs = bdrv_lookup_bs(node_name, node_name, errp);
It means we can pass device name by 'node_name' parameter？
do we need to update command doc in qapi/block-core.json?
> if (!bs) {
> -error_setg(errp, "Device '%s' not found", node_name);
> return;
> }
>
> --
> 2.4.2
>
>
[Qemu-devel] about [SeaBIOS PATCH] Try to hard-reboot on rerun of post even on emulators.
Hi Kevin, When I use old seabios in some stable linux release, some bootable devices (2 ide disks) would be lost when I try to restart guest by Ctrl+Alt+Delete during boot stage. Releated Red Hat BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1129549 I found an upstream commit [1] fixed this bug, but when I backport this patch to old seabios, guest will shutdown when I try to restart by Ctrl+Alt+Delete during boot stage. Kevin, can you help to describe that: "Unfortunately, kvm does not keep a pristine copy of the BIOS at 0x" It's a kvm (userspace, QEMU) bug? If it's a qemu-kvm bug, I should also fix this bz in old stable release. [1] === commit 244caf86f11f5f65d166d91704f64cb673167abc Author: Kevin O'Connor Date: Wed Sep 15 21:48:16 2010 -0400 Try to hard-reboot on rerun of post even on emulators. Extend the hard-reboot logic to qemu and kvm. On qemu, a reboot will not reset the memory settings for 0xc-0xf, so copy that memory area manually before rebooting. Unfortunately, kvm does not keep a pristine copy of the BIOS at 0x, so detect that case and shutdown the machine. Two backport dependences: [PATCH] Try to hard-reboot processor on rerun of post under coreboot. [PATCH] Don't do shadow copying of optionroms when CONFIG_OPTIONROMS_DEPLOYED. -- Amos.
Re: [Qemu-devel] [PATCH v4] vhost: fix log base address
On Fri, Apr 17, 2015 at 05:13:24PM +0200, Michael S. Tsirkin wrote:
> VHOST_SET_LOG_BASE got an incorrect address, causing
> migration errors and potentially even memory corruption.
>
> Reported-by: Wen Congyang
> Signed-off-by: Michael S. Tsirkin
> ---
>
> changed (uint64_t)(unsigned long) to (uintptr_t).
> Untested.
> Wen Congyang, can you pls test and confirm.
>
> hw/virtio/vhost.c | 7 +--
> 1 file changed, 5 insertions(+), 2 deletions(-)
All mentioned issues were addressed.
Reviewed-by: Amos Kong
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 8dd2f59..b83675f 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -288,7 +288,7 @@ static inline void vhost_dev_log_resize(struct vhost_dev*
> dev, uint64_t size)
> int r;
>
> log = g_malloc0(size * sizeof *log);
> -log_base = (uint64_t)(unsigned long)log;
> +log_base = (uintptr_t)log;
> r = dev->vhost_ops->vhost_call(dev, VHOST_SET_LOG_BASE, &log_base);
> assert(r >= 0);
> /* Sync only the range covered by the old log */
> @@ -1016,10 +1016,13 @@ int vhost_dev_start(struct vhost_dev *hdev,
> VirtIODevice *vdev)
> }
>
> if (hdev->log_enabled) {
> +uint64_t log_base;
> +
> hdev->log_size = vhost_get_log_size(hdev);
> hdev->log = hdev->log_size ?
> g_malloc0(hdev->log_size * sizeof *hdev->log) : NULL;
> -r = hdev->vhost_ops->vhost_call(hdev, VHOST_SET_LOG_BASE, hdev->log);
> +log_base = (uintptr_t)hdev->log;
> +r = hdev->vhost_ops->vhost_call(hdev, VHOST_SET_LOG_BASE, &log_base);
> if (r < 0) {
> r = -errno;
> goto fail_log;
> --
> MST
--
Amos.
Re: [Qemu-devel] [PATCH 2/2] e1000: unconditionally enable bus mastering
On Fri, Feb 06, 2015 at 01:46:36PM +, Stefan Hajnoczi wrote:
> On Wed, Jan 07, 2015 at 04:08:29PM +, Stefan Hajnoczi wrote:
> > On Thu, Dec 18, 2014 at 05:22:19PM +0800, Amos Kong wrote:
> > > After enabled network debug of e1000 in Win2012-64r2 guest,
> > > Bus mastering of e1000 can't be enabled by e1000 driver. It
> > > caused guest can't get IP address.
> > >
> > > # bcdedit /debug on
> > > # bcdedit /dbgsettings net hostip:192.168.122.100 port:5
> > > (We can use non-existed IP here, it's just used to pass the
> > >setup, not really use it)
> > >
> > > If we disable debug function, e1000 driver can enable bus
> > > mastering bit successfully, guest network is fine.
> > >
> > > This patch changed e1000 backend to enalbe bus mastering
> > > unconditionally as a workaround.
> > >
> > > Signed-off-by: Amos Kong
> > > ---
> > > hw/net/e1000.c | 11 +--
> > > 1 file changed, 9 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/hw/net/e1000.c b/hw/net/e1000.c
> > > index ec9224b..82829ae 100644
> > > --- a/hw/net/e1000.c
> > > +++ b/hw/net/e1000.c
> > > @@ -1544,8 +1544,15 @@ static void e1000_write_config(PCIDevice *pci_dev,
> > > uint32_t address,
> > >
> > > pci_default_write_config(pci_dev, address, val, len);
> > >
> > > -if (range_covers_byte(address, len, PCI_COMMAND) &&
> > > -(pci_dev->config[PCI_COMMAND] & PCI_COMMAND_MASTER)) {
> > > +if (range_covers_byte(address, len, PCI_COMMAND)) {
> > > +/*
> > > + * Some guest (eg: Win2012-64r2) doesn't enable bus mastering
> > > + * correctly, it caused guest network down. So we unconditionally
> > > + * enable PCI bus mastering and BM memory region for e1000 as
> > > + * a workaround.
> > > + */
> > > +pci_dev->config[PCI_COMMAND] |= PCI_COMMAND_MASTER;
> > > +memory_region_set_enabled(&pci_dev->bus_master_enable_region,
> > > true);
> > > qemu_flush_queued_packets(qemu_get_queue(s->nic));
> > > start_xmit(s);
> > > }
> >
> > This is weird.
> >
> > Are you sure there's not some guest behavior missing like the NIC option
> > ROM leaving bus mastering enabled after the BIOS/EFI has booted, causing
> > Windows debug to work on physical machines?
QEMU emulated e1000 is too old, it's not in the official support
list of window network debug.
> > Before we merge a hack like this we should understand the problem 100%.
>
> Any new insights into what is going on here?
It's not good to unconditionaly enable BM, we should enable/disable it
according to other event/status change.
> Stefan
--
Amos.
signature.asc
Description: Digital signature
Re: [Qemu-devel] [PATCH] check return value of fcntl() to detect invalid fd
On Mon, Dec 22, 2014 at 11:48:29AM +0800, Jason Wang wrote:
>
> On 12/19/2014 09:25 PM, Amos Kong wrote:
> > Passing some invalid fds in QEMU commandline, the fds don't exist.
> > QEMU will get error "TUNGETIFF ioctl() failed: Bad file descriptor",
> > and coredump in setting queues.
> >
> > This patch checked return value of first operate to fd, QEMU will
> > report error and exit without coredump. It's effected for both netdev
> > fds and vhost_net fds.
> >
> > Signed-off-by: Amos Kong
> > ---
> > net/tap.c | 16 +---
> > 1 file changed, 13 insertions(+), 3 deletions(-)
> >
> > diff --git a/net/tap.c b/net/tap.c
> > index bde6b58..039280a 100644
> > --- a/net/tap.c
> > +++ b/net/tap.c
> > @@ -688,7 +688,7 @@ int net_init_tap(const NetClientOptions *opts, const
> > char *name,
> > NetClientState *peer)
> > {
> > const NetdevTapOptions *tap;
> > -int fd, vnet_hdr = 0, i = 0, queues;
> > +int fd, vnet_hdr = 0, i = 0, queues, ret;
> > /* for the no-fd, no-helper case */
> > const char *script = NULL; /* suppress wrong "uninit'd use" gcc
> > warning */
> > const char *downscript = NULL;
> > @@ -722,7 +722,12 @@ int net_init_tap(const NetClientOptions *opts, const
> > char *name,
> > return -1;
> > }
> >
> > -fcntl(fd, F_SETFL, O_NONBLOCK);
> > +ret = fcntl(fd, F_SETFL, O_NONBLOCK);
> > +if (ret < 0) {
> > +error_report("Fail to set file status to nonblock, "
> > + "%s", strerror(-ret));
> > +return -1;
> > +}
>
> This may not work. There may be still some kinds of fd can pass this but
> still fail at TUNGETIFF or other tun ioctls.
Early catching the error is better. This only help to check if the fd
exists.
> Probably you need to fail during TUNGETIFF, which can make sure it was
> not a tap fd.
Currently if ioctl fails, we treat the IFF_VNET_HDR flag isn't set.
We can return -1 in this case, and checking return value of
tap_probe_vnet_hdr(),
and fail qemu.
qemu/net/tap-linux.c:
int tap_probe_vnet_hdr(int fd)
{
struct ifreq ifr;
if (ioctl(fd, TUNGETIFF, &ifr) != 0) {
error_report("TUNGETIFF ioctl() failed: %s", strerror(errno));
return 0;<
}
return ifr.ifr_flags & IFF_VNET_HDR;
}
I think we can fix tap_probe_vnet_hdr() and add checking return value of
fcntl().
--
Amos.
signature.asc
Description: Digital signature
[Qemu-devel] [PATCH] check return value of fcntl() to detect invalid fd
Passing some invalid fds in QEMU commandline, the fds don't exist.
QEMU will get error "TUNGETIFF ioctl() failed: Bad file descriptor",
and coredump in setting queues.
This patch checked return value of first operate to fd, QEMU will
report error and exit without coredump. It's effected for both netdev
fds and vhost_net fds.
Signed-off-by: Amos Kong
---
net/tap.c | 16 +---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/net/tap.c b/net/tap.c
index bde6b58..039280a 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -688,7 +688,7 @@ int net_init_tap(const NetClientOptions *opts, const char
*name,
NetClientState *peer)
{
const NetdevTapOptions *tap;
-int fd, vnet_hdr = 0, i = 0, queues;
+int fd, vnet_hdr = 0, i = 0, queues, ret;
/* for the no-fd, no-helper case */
const char *script = NULL; /* suppress wrong "uninit'd use" gcc warning */
const char *downscript = NULL;
@@ -722,7 +722,12 @@ int net_init_tap(const NetClientOptions *opts, const char
*name,
return -1;
}
-fcntl(fd, F_SETFL, O_NONBLOCK);
+ret = fcntl(fd, F_SETFL, O_NONBLOCK);
+if (ret < 0) {
+error_report("Fail to set file status to nonblock, "
+ "%s", strerror(-ret));
+return -1;
+}
vnet_hdr = tap_probe_vnet_hdr(fd);
@@ -761,7 +766,12 @@ int net_init_tap(const NetClientOptions *opts, const char
*name,
return -1;
}
-fcntl(fd, F_SETFL, O_NONBLOCK);
+ret = fcntl(fd, F_SETFL, O_NONBLOCK);
+if (ret < 0) {
+error_report("Fail to set file status to nonblock, "
+ "%s", strerror(-ret));
+return -1;
+}
if (i == 0) {
vnet_hdr = tap_probe_vnet_hdr(fd);
--
2.1.0
Re: [Qemu-devel] [PATCH 0/2] ignore bus master for e1000
On Thu, Dec 18, 2014 at 10:13:07AM +0008, Jason Wang wrote: > > > On Thu, Dec 18, 2014 at 5:22 PM, Amos Kong wrote: > >Win2012-64r2 guest doesn't set bus mastering correctly, > >it caused guest network down, this patch ignored it for > >e1000 nic for workarounding the guest issue. > > > >Patch 1 is an update version of: > >http://lists.gnu.org/archive/html/qemu-devel/2014-12/msg00048.html > >transmit packets are also defered in latest version. > > > >Amos Kong (1): > > e1000: unconditionally enable bus mastering > > > >Michael S. Tsirkin (1): > > e1000: defer packets until BM enabled > > > > hw/net/e1000.c | 34 +- > > 1 file changed, 33 insertions(+), 1 deletion(-) > > > >-- > >2.1.0 > > Another question unrelated to this series: > Does 82540EM still supported by Win2k12? 82540EM isn't in the support list. Some users want to use emulated e1000 to debug Windows guests. I will try other emulated nics in QEMU. > The following link shows that 82540EM were not supported in Win2k12. > https://downloadcenter.intel.com/SearchResult.aspx?lang=eng&ProdId=983 > > The oldest 825x card supported by 2k12 seems 82574. > https://downloadcenter.intel.com/Detail_Desc.aspx?DwnldID=21694&lang=eng&ProdId=3189 > If yes, workaround a buggy driver may be endless work in the future. I think so. -- Amos. signature.asc Description: Digital signature
Re: [Qemu-devel] [PATCH 2/2] e1000: unconditionally enable bus mastering
On Thu, Dec 18, 2014 at 09:57:29AM +0008, Jason Wang wrote:
>
>
> On Thu, Dec 18, 2014 at 5:22 PM, Amos Kong wrote:
> >After enabled network debug of e1000 in Win2012-64r2 guest,
> >Bus mastering of e1000 can't be enabled by e1000 driver. It
> >caused guest can't get IP address.
> >
> > # bcdedit /debug on
> > # bcdedit /dbgsettings net hostip:192.168.122.100 port:5
> > (We can use non-existed IP here, it's just used to pass the
> > setup, not really use it)
> >
> >If we disable debug function, e1000 driver can enable bus
> >mastering bit successfully, guest network is fine.
> >
> >This patch changed e1000 backend to enalbe bus mastering
> >unconditionally as a workaround.
> >
> >Signed-off-by: Amos Kong
> >---
> > hw/net/e1000.c | 11 +--
> > 1 file changed, 9 insertions(+), 2 deletions(-)
> >
> >diff --git a/hw/net/e1000.c b/hw/net/e1000.c
> >index ec9224b..82829ae 100644
> >--- a/hw/net/e1000.c
> >+++ b/hw/net/e1000.c
> >@@ -1544,8 +1544,15 @@ static void e1000_write_config(PCIDevice *pci_dev,
> >uint32_t address,
> > pci_default_write_config(pci_dev, address, val, len);
> >-if (range_covers_byte(address, len, PCI_COMMAND) &&
> >-(pci_dev->config[PCI_COMMAND] & PCI_COMMAND_MASTER)) {
> >+if (range_covers_byte(address, len, PCI_COMMAND)) {
> >+/*
> >+ * Some guest (eg: Win2012-64r2) doesn't enable bus mastering
> >+ * correctly, it caused guest network down. So we unconditionally
> >+ * enable PCI bus mastering and BM memory region for e1000 as
> >+ * a workaround.
> >+ */
> >+pci_dev->config[PCI_COMMAND] |= PCI_COMMAND_MASTER;
> >+memory_region_set_enabled(&pci_dev->bus_master_enable_region,
> >true);
>
> So BM is still set even if guest want to clear it?
BM will be cleared many times in boot stage, enabling it once doesn't
work. Curently we honestly clear the bit, then BM isn't enabled.
But it's might be too aggressive/crazy to enable BM forever.
> >
> > qemu_flush_queued_packets(qemu_get_queue(s->nic));
> > start_xmit(s);
> > }
--
Amos.
signature.asc
Description: Digital signature
[Qemu-devel] [PATCH 1/2] e1000: defer packets until BM enabled
From: "Michael S. Tsirkin"
Some guests seem to set BM for e1000 after enabling RX.
If packets arrive in the window, device is wedged.
Probably works by luck on real hardware, work around
this by making can_receive depend on BM. This patch
defer transmit packets, only start transmit when BM
is enabled.
Tested-by: Gabriel Somlo
Signed-off-by: Michael S. Tsirkin
Signed-off-by: Amos Kong
---
hw/net/e1000.c | 27 ++-
1 file changed, 26 insertions(+), 1 deletion(-)
diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index e33a4da..ec9224b 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -33,6 +33,7 @@
#include "sysemu/sysemu.h"
#include "sysemu/dma.h"
#include "qemu/iov.h"
+#include "qemu/range.h"
#include "e1000_regs.h"
@@ -790,6 +791,11 @@ start_xmit(E1000State *s)
struct e1000_tx_desc desc;
uint32_t tdh_start = s->mac_reg[TDH], cause = E1000_ICS_TXQE;
+if (!(d->config[PCI_COMMAND] & PCI_COMMAND_MASTER)) {
+DBGOUT(TX, "BM disabled\n");
+return;
+}
+
if (!(s->mac_reg[TCTL] & E1000_TCTL_EN)) {
DBGOUT(TX, "tx disabled\n");
return;
@@ -923,7 +929,9 @@ e1000_can_receive(NetClientState *nc)
E1000State *s = qemu_get_nic_opaque(nc);
return (s->mac_reg[STATUS] & E1000_STATUS_LU) &&
-(s->mac_reg[RCTL] & E1000_RCTL_EN) && e1000_has_rxbufs(s, 1);
+(s->mac_reg[RCTL] & E1000_RCTL_EN) &&
+(s->parent_obj.config[PCI_COMMAND] & PCI_COMMAND_MASTER) &&
+e1000_has_rxbufs(s, 1);
}
static uint64_t rx_desc_base(E1000State *s)
@@ -1529,6 +1537,21 @@ static NetClientInfo net_e1000_info = {
.link_status_changed = e1000_set_link_status,
};
+static void e1000_write_config(PCIDevice *pci_dev, uint32_t address,
+uint32_t val, int len)
+{
+E1000State *s = E1000(pci_dev);
+
+pci_default_write_config(pci_dev, address, val, len);
+
+if (range_covers_byte(address, len, PCI_COMMAND) &&
+(pci_dev->config[PCI_COMMAND] & PCI_COMMAND_MASTER)) {
+qemu_flush_queued_packets(qemu_get_queue(s->nic));
+start_xmit(s);
+}
+}
+
+
static int pci_e1000_init(PCIDevice *pci_dev)
{
DeviceState *dev = DEVICE(pci_dev);
@@ -1539,6 +1562,8 @@ static int pci_e1000_init(PCIDevice *pci_dev)
int i;
uint8_t *macaddr;
+pci_dev->config_write = e1000_write_config;
+
pci_conf = pci_dev->config;
/* TODO: RST# value should be 0, PCI spec 6.2.4 */
--
2.1.0
[Qemu-devel] [PATCH 0/2] ignore bus master for e1000
Win2012-64r2 guest doesn't set bus mastering correctly, it caused guest network down, this patch ignored it for e1000 nic for workarounding the guest issue. Patch 1 is an update version of: http://lists.gnu.org/archive/html/qemu-devel/2014-12/msg00048.html transmit packets are also defered in latest version. Amos Kong (1): e1000: unconditionally enable bus mastering Michael S. Tsirkin (1): e1000: defer packets until BM enabled hw/net/e1000.c | 34 +- 1 file changed, 33 insertions(+), 1 deletion(-) -- 2.1.0
[Qemu-devel] [PATCH 2/2] e1000: unconditionally enable bus mastering
After enabled network debug of e1000 in Win2012-64r2 guest,
Bus mastering of e1000 can't be enabled by e1000 driver. It
caused guest can't get IP address.
# bcdedit /debug on
# bcdedit /dbgsettings net hostip:192.168.122.100 port:5
(We can use non-existed IP here, it's just used to pass the
setup, not really use it)
If we disable debug function, e1000 driver can enable bus
mastering bit successfully, guest network is fine.
This patch changed e1000 backend to enalbe bus mastering
unconditionally as a workaround.
Signed-off-by: Amos Kong
---
hw/net/e1000.c | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index ec9224b..82829ae 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -1544,8 +1544,15 @@ static void e1000_write_config(PCIDevice *pci_dev,
uint32_t address,
pci_default_write_config(pci_dev, address, val, len);
-if (range_covers_byte(address, len, PCI_COMMAND) &&
-(pci_dev->config[PCI_COMMAND] & PCI_COMMAND_MASTER)) {
+if (range_covers_byte(address, len, PCI_COMMAND)) {
+/*
+ * Some guest (eg: Win2012-64r2) doesn't enable bus mastering
+ * correctly, it caused guest network down. So we unconditionally
+ * enable PCI bus mastering and BM memory region for e1000 as
+ * a workaround.
+ */
+pci_dev->config[PCI_COMMAND] |= PCI_COMMAND_MASTER;
+memory_region_set_enabled(&pci_dev->bus_master_enable_region, true);
qemu_flush_queued_packets(qemu_get_queue(s->nic));
start_xmit(s);
}
--
2.1.0
Re: [Qemu-devel] [PATCH 1/1] virtio-rng: fix check for period_ms validity
On Thu, Dec 11, 2014 at 01:19:27PM +0530, Amit Shah wrote:
> This was reported for this warning:
>
> hw/virtio/virtio-rng.c:150:31: warning: logical not is only applied to
> the left hand side of comparison [-Wlogical-not-parentheses]
>
> Reported-by: dcb
> Suggested-by: dcb
> Bug: https://bugs.launchpad.net/qemu/+bug/1393486
> Signed-off-by: Amit Shah
> ---
> hw/virtio/virtio-rng.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c
> index 473c044..06e7178 100644
> --- a/hw/virtio/virtio-rng.c
> +++ b/hw/virtio/virtio-rng.c
> @@ -149,7 +149,7 @@ static void virtio_rng_device_realize(DeviceState *dev,
> Error **errp)
> VirtIORNG *vrng = VIRTIO_RNG(dev);
> Error *local_err = NULL;
>
> -if (!vrng->conf.period_ms > 0) {
> +if (vrng->conf.period_ms <= 0) {
Thanks.
Reviewed-by: Amos Kong
> error_setg(errp, "'period' parameter expects a positive integer");
> return;
> }
> --
> 2.1.0
--
Amos.
signature.asc
Description: Digital signature
Re: [Qemu-devel] [PATCH 0/4] migration: fix CVE-2014-7840
On Wed, Nov 12, 2014 at 11:44:35AM +0200, Michael S. Tsirkin wrote: > This patchset fixes CVE-2014-7840: invalid > migration stream can cause arbitrary qemu memory > overwrite. > First patch includes the minimal fix for the issue. > Follow-up patches on top add extra checking to reduce the > chance this kind of bug recurs. > > Note: these are already (tentatively-pending review) > queued in my tree, so only review/ack > is necessary. > > Michael S. Tsirkin (4): Reviewed-by: Amos Kong > migration: fix parameter validation on ram load > exec: add wrapper for host pointer access > cpu: assert host pointer offset within block > cpu: verify that block->host is set > > include/exec/cpu-all.h | 7 +++ > arch_init.c| 5 +++-- > exec.c | 10 +- > 3 files changed, 15 insertions(+), 7 deletions(-) > > -- > MST > -- Amos. signature.asc Description: Digital signature
Re: [Qemu-devel] [PATCH] block/rbd: fix memory leak
On Thu, Dec 04, 2014 at 02:34:11PM +0800, arei.gong...@huawei.com wrote:
> From: Gonglei
>
> Variable local_err going out of scope
> leaks the storage it points to.
>
> Cc: Markus Armbruster
> Signed-off-by: Gonglei
> ---
> block/rbd.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/block/rbd.c b/block/rbd.c
> index 5b5a64a..f3ab2dd 100644
> --- a/block/rbd.c
> +++ b/block/rbd.c
> @@ -459,7 +459,7 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict
> *options, int flags,
> clientname = qemu_rbd_parse_clientname(conf, clientname_buf);
> r = rados_create(&s->cluster, clientname);
> if (r < 0) {
> -error_setg(&local_err, "error initializing");
> +error_setg(errp, "error initializing");
> goto failed_opts;
> }
>
> @@ -495,19 +495,19 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict
> *options, int flags,
>
> r = rados_connect(s->cluster);
> if (r < 0) {
> -error_setg(&local_err, "error connecting");
> +error_setg(errp, "error connecting");
> goto failed_shutdown;
> }
>
> r = rados_ioctx_create(s->cluster, pool, &s->io_ctx);
> if (r < 0) {
> -error_setg(&local_err, "error opening pool %s", pool);
> +error_setg(errp, "error opening pool %s", pool);
> goto failed_shutdown;
> }
>
> r = rbd_open(s->io_ctx, s->name, &s->image, s->snap);
> if (r < 0) {
> -error_setg(&local_err, "error reading header from %s", s->name);
> +error_setg(errp, "error reading header from %s", s->name);
> goto failed_open;
> }
'local_err' was defined for collecting error insider qemu_opts_absorb_qdict().
We should set error to 'errp', then the error can be passed to caller,
and free the memory in future.
Reviewed-by: Amos Kong
--
Amos.
signature.asc
Description: Digital signature
Re: [Qemu-devel] [PATCH for-2.3] configure: Replace which(1) with "has"
On Thu, Dec 04, 2014 at 02:18:16PM +0800, Fam Zheng wrote: > Using "has" is more slick because which(1) is not always there. > > Signed-off-by: Fam Zheng > --- > configure | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/configure b/configure > index 47048f0..cae588c 100755 > --- a/configure > +++ b/configure > @@ -2727,7 +2727,7 @@ fi > if test "$modules" = yes; then > shacmd_probe="sha1sum sha1 shasum" > for c in $shacmd_probe; do > -if which $c >/dev/null 2>&1; then > +if has $c; then we use self defined has function to check command in all other place in configure. LGTM. Reviewed-by: Amos Kong > shacmd="$c" > break > fi > -- > 1.9.3 > -- Amos. signature.asc Description: Digital signature
Re: [Qemu-devel] [PATCH] input: move input-send-event into experimental namespace
On Tue, Nov 25, 2014 at 09:35:26AM -0700, Eric Blake wrote: > On 11/25/2014 04:58 AM, Markus Armbruster wrote: > > Amos Kong writes: > > > >> On Tue, Nov 25, 2014 at 09:06:34AM +0100, Markus Armbruster wrote: > >>> Gerd Hoffmann writes: > >>> > >>>> Ongoing discussions on how we are going to specify the console, > >>>> so tag the command as experiemntal so we can refine things in > >>>> the 2.3 development cycle. > >>>> > >>>> Signed-off-by: Gerd Hoffmann > >>>> --- > >>>> qmp-commands.hx | 12 ++-- > >>> > >>> Don't you need to patch qapi-schema.json, too? > >> > >> Not necessary in function level. > > > > s/need to/want to/? > > Yes, if you are going to mark the HMP command experimental, please also > mark the QMP command as experimental. Gerd's patch wasn't make hmp command experimental, but QMP. Inputted command will be compared with .name field in qmp-commands.hx, not qapi-schema.json. The command name in qapi-schema.json only effects the function name. But it's good to also update qapi-schema.json. -- Amos. signature.asc Description: Digital signature
Re: [Qemu-devel] [PATCH v2 for-2.2] input: move input-send-event into experimental namespace
On Tue, Nov 25, 2014 at 02:54:17PM +0100, Markus Armbruster wrote:
> From: Gerd Hoffmann
>
> Ongoing discussions on how we are going to specify the console,
> so tag the command as experiental so we can refine things in
> the 2.3 development cycle.
>
> Signed-off-by: Gerd Hoffmann
> [Spell out "not a stable API", and x- the QAPI schema, too]
> Signed-off-by: Markus Armbruster
>
> Signed-off-by: Markus Armbruster
> ---
> I would've ripped out the command entirely, but Gerd's approach works
> for me, too. Since time for -rc3 is running out, I'm respinning his
> patch with my and Amos's review comments worked in. Hope that's okay.
>
> We should also pick Amos's "[PATCH] qmp: fix typo in input-send-event
> examples".
Reviewed-by: Amos Kong
> qapi-schema.json | 6 --
> qmp-commands.hx | 16 +---
> ui/input.c | 4 ++--
> 3 files changed, 15 insertions(+), 11 deletions(-)
>
> diff --git a/qapi-schema.json b/qapi-schema.json
> index d0926d9..9ffdcf8 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3245,7 +3245,7 @@
>'abs' : 'InputMoveEvent' } }
>
> ##
> -# @input-send-event
> +# @x-input-send-event
> #
> # Send input event(s) to guest.
> #
> @@ -3257,8 +3257,10 @@
> #
> # Since: 2.2
> #
> +# Note: this command is experimental, and not a stable API.
> +#
> ##
> -{ 'command': 'input-send-event',
> +{ 'command': 'x-input-send-event',
>'data': { '*console':'int', 'events': [ 'InputEvent' ] } }
>
> ##
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 8812401..718dd92 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -3791,13 +3791,13 @@ Example:
> EQMP
>
> {
> -.name = "input-send-event",
> +.name = "x-input-send-event",
> .args_type = "console:i?,events:q",
> -.mhandler.cmd_new = qmp_marshal_input_input_send_event,
> +.mhandler.cmd_new = qmp_marshal_input_x_input_send_event,
> },
>
> SQMP
> -@input-send-event
> +@x-input-send-event
> -
>
> Send input event to guest.
> @@ -3811,17 +3811,19 @@ The consoles are visible in the qom tree, under
> /backend/console[$index]. They have a device link and head property, so
> it is possible to map which console belongs to which device and display.
>
> +Note: this command is experimental, and not a stable API.
> +
> Example (1):
>
> Press left mouse button.
>
> --> { "execute": "input-send-event",
> +-> { "execute": "x-input-send-event",
> "arguments": { "console": 0,
> "events": [ { "type": "btn",
> "data" : { "down": true, "button": "Left" } } } }
> <- { "return": {} }
>
> --> { "execute": "input-send-event",
> +-> { "execute": "x-input-send-event",
> "arguments": { "console": 0,
> "events": [ { "type": "btn",
> "data" : { "down": false, "button": "Left" } } } }
> @@ -3831,7 +3833,7 @@ Example (2):
>
> Press ctrl-alt-del.
>
> --> { "execute": "input-send-event",
> +-> { "execute": "x-input-send-event",
> "arguments": { "console": 0, "events": [
> { "type": "key", "data" : { "down": true,
>"key": {"type": "qcode", "data": "ctrl" } } },
> @@ -3845,7 +3847,7 @@ Example (3):
>
> Move mouse pointer to absolute coordinates (2, 400).
>
> --> { "execute": "input-send-event" ,
> +-> { "execute": "x-input-send-event" ,
>"arguments": { "console": 0, "events": [
> { "type": "abs", "data" : { "axis": "X", "value" : 2 } },
> { "type": "abs", "data" : { "axis": "Y", "value" : 400 } } ]
> } }
> diff --git a/ui/input.c b/ui/input.c
> index 37ff46f..7ba99e5 100644
> --- a/ui/input.c
> +++ b/ui/input.c
> @@ -122,8 +122,8 @@ qemu_input_find_handler(uint32_t mask, QemuConsole *con)
> return NULL;
> }
>
> -void qmp_input_send_event(bool has_console, int64_t console,
> - InputEventList *events, Error **errp)
> +void qmp_x_input_send_event(bool has_console, int64_t console,
> +InputEventList *events, Error **errp)
> {
> InputEventList *e;
> QemuConsole *con;
> --
> 1.9.3
--
Amos.
signature.asc
Description: Digital signature
Re: [Qemu-devel] [PATCH] input: move input-send-event into experimental namespace
On Tue, Nov 25, 2014 at 09:06:34AM +0100, Markus Armbruster wrote: > Gerd Hoffmann writes: > > > Ongoing discussions on how we are going to specify the console, > > so tag the command as experiemntal so we can refine things in > > the 2.3 development cycle. > > > > Signed-off-by: Gerd Hoffmann > > --- > > qmp-commands.hx | 12 ++-- > > Don't you need to patch qapi-schema.json, too? Not necessary in function level. > Do we actually explain "x- means experimental" anywhere? What's the official way to make command experimental? Quote from qapi-schema.json: | # Notes: This command is experimental and may change | syntax in future releases. -- Amos. signature.asc Description: Digital signature
[Qemu-devel] [PATCH] qmp: fix typo in input-send-event examples
Lack of two closed bracket in json commands.
Signed-off-by: Amos Kong
---
qmp-commands.hx | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 8812401..bb2e380 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3818,13 +3818,13 @@ Press left mouse button.
-> { "execute": "input-send-event",
"arguments": { "console": 0,
"events": [ { "type": "btn",
-"data" : { "down": true, "button": "Left" } } } }
+"data" : { "down": true, "button": "Left" } } ] } }
<- { "return": {} }
-> { "execute": "input-send-event",
"arguments": { "console": 0,
"events": [ { "type": "btn",
-"data" : { "down": false, "button": "Left" } } } }
+"data" : { "down": false, "button": "Left" } } ] } }
<- { "return": {} }
Example (2):
--
1.9.3
Re: [Qemu-devel] [PATCH] input: move input-send-event into experimental namespace
On Tue, Nov 25, 2014 at 08:18:54AM +0100, Gerd Hoffmann wrote:
> Ongoing discussions on how we are going to specify the console,
> so tag the command as experiemntal so we can refine things in
> the 2.3 development cycle.
%s/experiemntal/experimental/
Reviewed-by: Amos Kong
> Signed-off-by: Gerd Hoffmann
> ---
> qmp-commands.hx | 12 ++--
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 8812401..d6b7362 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -3791,13 +3791,13 @@ Example:
> EQMP
>
> {
> -.name = "input-send-event",
> +.name = "x-input-send-event",
> .args_type = "console:i?,events:q",
> .mhandler.cmd_new = qmp_marshal_input_input_send_event,
> },
>
> SQMP
> -@input-send-event
> +@x-input-send-event
> -
>
> Send input event to guest.
> @@ -3815,13 +3815,13 @@ Example (1):
>
> Press left mouse button.
>
> --> { "execute": "input-send-event",
> +-> { "execute": "x-input-send-event",
> "arguments": { "console": 0,
> "events": [ { "type": "btn",
> "data" : { "down": true, "button": "Left" } } } }
(not problem of this patch)
typo, lack of ']'
> <- { "return": {} }
>
> --> { "execute": "input-send-event",
> +-> { "execute": "x-input-send-event",
> "arguments": { "console": 0,
> "events": [ { "type": "btn",
> "data" : { "down": false, "button": "Left" } } } }
some typo.
> @@ -3831,7 +3831,7 @@ Example (2):
>
> Press ctrl-alt-del.
>
> --> { "execute": "input-send-event",
> +-> { "execute": "x-input-send-event",
> "arguments": { "console": 0, "events": [
> { "type": "key", "data" : { "down": true,
>"key": {"type": "qcode", "data": "ctrl" } } },
> @@ -3845,7 +3845,7 @@ Example (3):
>
> Move mouse pointer to absolute coordinates (2, 400).
>
> --> { "execute": "input-send-event" ,
> +-> { "execute": "x-input-send-event" ,
>"arguments": { "console": 0, "events": [
> { "type": "abs", "data" : { "axis": "X", "value" : 2 } },
> { "type": "abs", "data" : { "axis": "Y", "value" : 400 } } ]
> } }
> --
> 1.8.3.1
>
--
Amos.
signature.asc
Description: Digital signature
[Qemu-devel] [PATCH for-2.2] qmp: document input-send-event detail
This patch documents what exactly input-send-event is supposed to do. Signed-off-by: Amos Kong --- qapi-schema.json | 11 +++ 1 file changed, 11 insertions(+) diff --git a/qapi-schema.json b/qapi-schema.json index d0926d9..9aa19bc 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -3249,7 +3249,18 @@ # # Send input event(s) to guest. # +# The VM must be running or suspended. +# +# Each event is sent to exactly one admissible input device. Such a +# device must exist. If multiple admissible devices can handle an +# event, which one gets it is unspecified, except as described below. +# # @console: #optional console to send event(s) to. +# If @console is missing, only devices that aren't associated +# with a console are admissible. +# If @console is specified, it must exist, and both devices +# associated with that console and devices not associated with a +# console are admissible, but the former takes precedence. # # @events: List of InputEvent union. # -- 1.9.3
Re: [Qemu-devel] [PATCH v3] error: passing a negative value to an os_errno is wrong
On Mon, Nov 10, 2014 at 02:54:49PM +0900, SeokYeon Hwang wrote: > Added 'assert(os_errno > 0)' in 'error_set_errno()'. > Fixed errno since it passes wrong value to 'error_set_errno()'. > > Signed-off-by: SeokYeon Hwang Reviewed-by: Amos Kong > --- > hw/pci/pcie.c | 2 +- > util/error.c | 1 + > 2 files changed, 2 insertions(+), 1 deletion(-) > > diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c > index 58455bd..2902f7d 100644 > --- a/hw/pci/pcie.c > +++ b/hw/pci/pcie.c > @@ -229,7 +229,7 @@ static void pcie_cap_slot_hotplug_common(PCIDevice > *hotplug_dev, > /* the slot is electromechanically locked. > * This error is propagated up to qdev and then to HMP/QMP. > */ > -error_setg_errno(errp, -EBUSY, "slot is electromechanically locked"); > +error_setg_errno(errp, EBUSY, "slot is electromechanically locked"); > } > } > > diff --git a/util/error.c b/util/error.c > index 2ace0d8..6c9d995 100644 > --- a/util/error.c > +++ b/util/error.c > @@ -62,6 +62,7 @@ void error_set_errno(Error **errp, int os_errno, ErrorClass > err_class, > return; > } > assert(*errp == NULL); > +assert(os_errno >= 0); > > err = g_malloc0(sizeof(*err)); > > -- > 2.1.0 > -- Amos. signature.asc Description: Digital signature
Re: [Qemu-devel] [PATCH v2] error: passing a negative value to an os_errno is wrong
On Fri, Nov 07, 2014 at 11:24:55AM +0900, SeokYeon Hwang wrote: > Added 'assert(os_errno > 0)' in 'error_set_errno()'. > Fixed errno since it passes wrong value to 'error_set_errno()'. > > Signed-off-by: SeokYeon Hwang > --- > hw/pci/pcie.c | 2 +- > util/error.c | 1 + > 2 files changed, 2 insertions(+), 1 deletion(-) > > diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c > index 58455bd..2902f7d 100644 > --- a/hw/pci/pcie.c > +++ b/hw/pci/pcie.c > @@ -229,7 +229,7 @@ static void pcie_cap_slot_hotplug_common(PCIDevice > *hotplug_dev, > /* the slot is electromechanically locked. > * This error is propagated up to qdev and then to HMP/QMP. > */ > -error_setg_errno(errp, -EBUSY, "slot is electromechanically locked"); > +error_setg_errno(errp, EBUSY, "slot is electromechanically locked"); > } > } > > diff --git a/util/error.c b/util/error.c > index 2ace0d8..4ce22cc 100644 > --- a/util/error.c > +++ b/util/error.c > @@ -62,6 +62,7 @@ void error_set_errno(Error **errp, int os_errno, ErrorClass > err_class, > return; > } > assert(*errp == NULL); > +assert(os_errno > 0); strerror(0) will return string 'Success', do we need to reserve zero here? assert(os_errno >= 0); > > err = g_malloc0(sizeof(*err)); > > -- > 2.1.0 > -- Amos. signature.asc Description: Digital signature
[Qemu-devel] [PATCH for-2.2 2/2] QMP/input-send-event: make console parameter optional
The 'QemuConsole' is the input source for handler, we share some
input handlers to process the input events from different QemuConsole.
Normally we only have one set of keyboard, mouse, usbtablet, etc.
The devices have different mask, it's fine to just checking mask to
insure that the handler has the ability to process the event.
I saw we try to bind console to handler in usb/dev-hid.c, but display
always isn't available at that time.
If we have multiseat setup (as Gerd said), we only have 'problem' in
this case. Actually event from different devices have the same effect
for system, it's fine to always use the first available handler
without caring about the console.
For send-key command, we just pass a NULL for console parameter in
calling qemu_input_event_send_key(NULL, ..), but 'input-send-event'
needs to care more devices.
Conclusion:
Generally assigning the special console is meanless, and we can't
directly remove the QMP parameter for compatibility.
So we can make the parameter optional. The parameter might be useful
for some special condition: we have multiple devices without binding
console and they all have the ability(mask) to process events, and
we don't want to use the first one.
Cc: Gerd Hoffmann
Cc: Markus Armbruster
Cc: Marcelo Tosatti
Signed-off-by: Amos Kong
---
qapi-schema.json | 4 ++--
qmp-commands.hx | 4 ++--
ui/input.c | 15 +--
3 files changed, 13 insertions(+), 10 deletions(-)
diff --git a/qapi-schema.json b/qapi-schema.json
index a1573d8..367fb8e 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3249,7 +3249,7 @@
#
# Send input event(s) to guest.
#
-# @console: Which console to send event(s) to.
+# @console: #optional console to send event(s) to.
#
# @events: List of InputEvent union.
#
@@ -3259,7 +3259,7 @@
#
##
{ 'command': 'input-send-event',
- 'data': { 'console':'int', 'events': [ 'InputEvent' ] } }
+ 'data': { '*console':'int', 'events': [ 'InputEvent' ] } }
##
# @NumaOptions
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 1abd619..8812401 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3792,7 +3792,7 @@ EQMP
{
.name = "input-send-event",
-.args_type = "console:i,events:q",
+.args_type = "console:i?,events:q",
.mhandler.cmd_new = qmp_marshal_input_input_send_event,
},
@@ -3804,7 +3804,7 @@ Send input event to guest.
Arguments:
-- "console": console index.
+- "console": console index. (json-int, optional)
- "events": list of input events.
The consoles are visible in the qom tree, under
diff --git a/ui/input.c b/ui/input.c
index 002831e..37ff46f 100644
--- a/ui/input.c
+++ b/ui/input.c
@@ -122,16 +122,19 @@ qemu_input_find_handler(uint32_t mask, QemuConsole *con)
return NULL;
}
-void qmp_input_send_event(int64_t console, InputEventList *events,
- Error **errp)
+void qmp_input_send_event(bool has_console, int64_t console,
+ InputEventList *events, Error **errp)
{
InputEventList *e;
QemuConsole *con;
-con = qemu_console_lookup_by_index(console);
-if (!con) {
-error_setg(errp, "console %" PRId64 " not found", console);
-return;
+con = NULL;
+if (has_console) {
+con = qemu_console_lookup_by_index(console);
+if (!con) {
+error_setg(errp, "console %" PRId64 " not found", console);
+return;
+}
}
if (!runstate_is_running() && !runstate_check(RUN_STATE_SUSPENDED)) {
--
1.9.3
[Qemu-devel] [PATCH for-2.2 1/2] QMP/input-send-event: update document of union InputEvent
Signed-off-by: Amos Kong
---
qapi-schema.json | 5 +
1 file changed, 5 insertions(+)
diff --git a/qapi-schema.json b/qapi-schema.json
index 24379ab..a1573d8 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3231,6 +3231,11 @@
#
# Input event union.
#
+# @key: Input event of Keyboard
+# @btn: Input event of pointer bottons
+# @rel: Input event of relative pointer motion
+# @abs: Input event of absolute pointer motion
+#
# Since: 2.0
##
{ 'union' : 'InputEvent',
--
1.9.3
Re: [Qemu-devel] [PATCH] ui/input: strictly check console in finding input handler
On Thu, Nov 06, 2014 at 11:01:11PM +0800, Amos Kong wrote: > On Thu, Nov 06, 2014 at 02:37:54PM +0800, Amos Kong wrote: > > On Wed, Nov 05, 2014 at 09:47:47AM +0100, Gerd Hoffmann wrote: > > > On Mi, 2014-11-05 at 00:49 +0800, Amos Kong wrote: > > > > qemu_input_find_handler() prefers a handler associated with con. > > > > But if none exists, it takes any. This patch added a parameter > > > > to strictly check console, in case we want to input event to > > > > special console. > > > > If console is assigned, it will try to find right handler by first > > loop in qemu_input_find_handler(). The second loop is used to find > > mask matched handler if console isn't assigned. > > > > If we assigned console and didn't find handler in first loop, it > > skip second loop body by 'continue', and return NULL. > > It seems my concern is wrong, we don't need this repeated parameter. > > I was wrong, if we don't assign the console for qemu_input_find_handler(), > it has chance to get an arbitrary console (mask matched). So the > original issue this patch try to fix truely exists. The 'console' in my mind was the input destination, so I thgouth we need to distinguish the console with strictly checking. The 'QemuConsole' is the input source for handler, we share some input handlers to process the input events from different QemuConsole. Normally we only have one set of keyboard, mouse, usbtablet, etc. The devices have different mask, it's fine to just checking mask to insure that the handler has the ability to process the event. I saw we try to bind console to handler in usb/dev-hid.c, but display always isn't available at that time. If we have multiseat setup (as Gerd said), we only have 'problem' in this case. Actually event from different devices have the same effect for system, it's fine to always use the first available handler without caring about the console. For send-key command, we just pass a NULL for console parameter in calling qemu_input_event_send_key(NULL, ..) Conclusion: * Generally assigning the special console is meanless, and we can't directly remove the QMP parameter for compatibility. So we can make the parameter optional. The parameter might be useful for some special condition: we have multiple devices without binding console and they all have the ability(mask) to process events, and we don't want to use the first one. Thanks. Amos signature.asc Description: Digital signature
Re: [Qemu-devel] [PATCH] ui/input: strictly check console in finding input handler
On Thu, Nov 06, 2014 at 02:37:54PM +0800, Amos Kong wrote: > On Wed, Nov 05, 2014 at 09:47:47AM +0100, Gerd Hoffmann wrote: > > On Mi, 2014-11-05 at 00:49 +0800, Amos Kong wrote: > > > qemu_input_find_handler() prefers a handler associated with con. > > > But if none exists, it takes any. This patch added a parameter > > > to strictly check console, in case we want to input event to > > > special console. > > If console is assigned, it will try to find right handler by first > loop in qemu_input_find_handler(). The second loop is used to find > mask matched handler if console isn't assigned. > > If we assigned console and didn't find handler in first loop, it > skip second loop body by 'continue', and return NULL. > It seems my concern is wrong, we don't need this repeated parameter. I was wrong, if we don't assign the console for qemu_input_find_handler(), it has chance to get an arbitrary console (mask matched). So the original issue this patch try to fix truely exists. > NACK this patch. > > Thanks. > > > > 'input-send-event' has a parameter to assign special console, > > > so we should enable strict checking in finding handler. > > > > I don't think we want do that by default. It only matters in case of a > > multiseat setup where you actually have multiple input devices of the > > same kind. Which isn't a very typical use case. > > > > Options I see are: > > > > (a) Turn console into an optional parameter, do strict checking in > > case it is present. > > (b) Add a optional 'strict' parameter. > > -- > Amos. From Markus: > > Current behavior (please correct misunderstandings): > > The guest must be running. > input-send-event parameter 'console' is mandatory. > The console identified by its value must exist. > If this console can accept the event, send it there. > Else, a console that can accept the event must exist. Send it > to > one of them. Which one exactly isn't specified. > > Behavior with (a): > > The guest must be running. > input-send-event parameter 'console' is optional. > If it's present, the console identified by its value must exist, > and > must be able to accept the event. Send it there. > Else, a console that can accept the event must exist. Send it > to > one of them. Which one exactly isn't specified. > > "Must" means "or else command fails". > > I think that's a clear improvement. It's actually what I expected > from > the command documentation, until I read the code. Thanks for your clear description, I now agree with Gerd's option (a), (a) is better than (b). So we need to change QMP to support optional console parameter and change qemu_input_find_handler() to support strict checking (as my patch). Gerd, I'd like to work on both of them, if you already work on it, please let me know, thanks. -- Amos. signature.asc Description: Digital signature
Re: [Qemu-devel] [PATCH] ui/input: strictly check console in finding input handler
On Wed, Nov 05, 2014 at 09:47:47AM +0100, Gerd Hoffmann wrote: > On Mi, 2014-11-05 at 00:49 +0800, Amos Kong wrote: > > qemu_input_find_handler() prefers a handler associated with con. > > But if none exists, it takes any. This patch added a parameter > > to strictly check console, in case we want to input event to > > special console. If console is assigned, it will try to find right handler by first loop in qemu_input_find_handler(). The second loop is used to find mask matched handler if console isn't assigned. If we assigned console and didn't find handler in first loop, it skip second loop body by 'continue', and return NULL. It seems my concern is wrong, we don't need this repeated parameter. NACK this patch. Thanks. > > 'input-send-event' has a parameter to assign special console, > > so we should enable strict checking in finding handler. > > I don't think we want do that by default. It only matters in case of a > multiseat setup where you actually have multiple input devices of the > same kind. Which isn't a very typical use case. > > Options I see are: > > (a) Turn console into an optional parameter, do strict checking in > case it is present. > (b) Add a optional 'strict' parameter. -- Amos. signature.asc Description: Digital signature
Re: [Qemu-devel] [PATCH] hw/pci: fix crash on shpc error flow
On Wed, Nov 05, 2014 at 04:29:35PM +0200, Marcel Apfelbaum wrote:
> If the pci bridge enters in error flow as part
> of init process it will only delete the shpc mmio
> subregion but not remove it from the properties list,
> resulting in segmentation fault when the bridge runs
> the exit function.
>
> Example: add a pci bridge without specifing the chassis number:
> ... -device pci-bridge,id=p1
> Result:
> (qemu) qemu-system-x86_64: -device pci-bridge,id=p1: Bridge chassis not
> specified. Each bridge is required to be assigned a unique chassis id > 0.
> qemu-system-x86_64: -device pci-bridge,id=p1: Device
> initialization failed.
> Segmentation fault (core dumped)
>
> if (child->class->unparent) {
> #0 0x558d629b in object_finalize_child_property
> (obj=0x56d2e830, name=0x56d30630 "shpc-mmio[0]",
> opaque=0x56a42fc8) at qom/object.c:1078
> #1 0x558d4b1f in object_property_del_all (obj=0x56d2e830) at
> qom/object.c:367
> #2 0x558d4ca1 in object_finalize (data=0x56d2e830) at
> qom/object.c:412
> #3 0x558d55a1 in object_unref (obj=0x56d2e830) at
> qom/object.c:720
> #4 0x5572c907 in qdev_device_add (opts=0x563544f0) at
> qdev-monitor.c:566
> #5 0x55744f16 in device_init_func (opts=0x563544f0,
> opaque=0x0) at vl.c:2213
> #6 0x559cf5f0 in qemu_opts_foreach (list=0x55e0f8e0
> , func=0x55744efa , opaque=0x0,
> abort_on_failure=1) at util/qemu-option.c:1057
> #7 0x5574a11b in main (argc=16, argv=0x7fffdde8,
> envp=0x7fffde70) at vl.c:423
>
> Unparent the shpc mmio region as part of shpc cleanup.
>
> Signed-off-by: Marcel Apfelbaum
> ---
> hw/pci/shpc.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/hw/pci/shpc.c b/hw/pci/shpc.c
> index 65b2f51..2e887d7 100644
> --- a/hw/pci/shpc.c
> +++ b/hw/pci/shpc.c
> @@ -662,6 +662,7 @@ void shpc_cleanup(PCIDevice *d, MemoryRegion *bar)
> SHPCDevice *shpc = d->shpc;
> d->cap_present &= ~QEMU_PCI_CAP_SHPC;
> memory_region_del_subregion(bar, &shpc->mmio);
> +object_unparent(OBJECT(&shpc->mmio));
> /* TODO: cleanup config space changes? */
> g_free(shpc->config);
> g_free(shpc->cmask);
> --
Reviewed-by: Amos Kong
> 1.8.3.1
>
--
Amos.
signature.asc
Description: Digital signature
Re: [Qemu-devel] [PATCHv2] virtio-serial: avoid crash when port has no name
On Wed, Nov 05, 2014 at 03:43:14PM +0100, Marc-André Lureau wrote:
> It seems "name" is not mandatory, and the following command line (based
> on one generated by current libvirt) will crash qemu at start:
>
> qemu-system-x86_64 \
> -device virtio-serial-pci \
> -device virtserialport,name=foo \
> -device virtconsole
>
> Program received signal SIGSEGV, Segmentation fault.
> __strcmp_ssse3 () at ../sysdeps/x86_64/strcmp.S:210
> 210movlpd(%rsi), %xmm2
> Missing separate debuginfos, use: debuginfo-install
> python-libs-2.7.5-13.fc20.x86_64
> (gdb) bt
> #0 __strcmp_ssse3 () at ../sysdeps/x86_64/strcmp.S:210
> #1 0x5566bdc6 in find_port_by_name (name=0x0) at
> /home/elmarco/src/qemu/hw/char/virtio-serial-bus.c:67
>
> Signed-off-by: Marc-André Lureau
> ---
> hw/char/virtio-serial-bus.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
> index 3931085..f16452e 100644
> --- a/hw/char/virtio-serial-bus.c
> +++ b/hw/char/virtio-serial-bus.c
> @@ -871,7 +871,7 @@ static void virtser_port_device_realize(DeviceState *dev,
> Error **errp)
> return;
> }
>
> -if (find_port_by_name(port->name)) {
> +if (port->name != NULL && find_port_by_name(port->name)) {
> error_setg(errp, "virtio-serial-bus: A port already exists by name
> %s",
> port->name);
> return;
> --
> 1.9.3
>
Reviewed-by: Amos Kong
--
Amos.
signature.asc
Description: Digital signature
Re: [Qemu-devel] QEMU trunk now in hardfreeze
On Thu, Nov 06, 2014 at 12:51:23PM +0800, Gonglei wrote: > On 2014/11/5 23:32, Peter Maydell wrote: > > > Hi; I've just tagged v2.2.0-rc0 in master, so we're now officially > > in hardfreeze, and only accepting bug fixes from here onward. > > Mike Roth should have the rc0 tarballs out sometime later today. > > > > I've updated http://wiki.qemu.org/Planning/2.2 to list -rc dates; > > as usual, one a week, with the hope that -rc3 will be the final one > > with no changes between that and the release proper. > > > > Please make sure you update the changelog at > > http://wiki.qemu.org/ChangeLog/2.2 with any interesting/important > > new features that got added in this release. > > > > Hi, Peter > > I think the below feature should be added in 2.2 release. > > -Boot Devices > Supporting dynamically modify boot order of guest, and assuring > taking effect after the guest rebooting. > > Request for wiki account, please. Thanks! I have created one for you. username: GongLei > Best regards, > -Gonglei -- Amos. signature.asc Description: Digital signature
Re: [Qemu-devel] [PATCH] error: fixed error_set_errno() to deal with a negative type of os_error.
On Wed, Nov 05, 2014 at 12:19:34PM +0100, Eric Blake wrote:
> On 11/05/2014 12:11 PM, Max Reitz wrote:
>
> +err->msg = g_strdup_printf("%s: %s", msg1,
> strerror(abs(os_errno)));
>
> >> I don't, we really should fix the callers.
> >
> > Of course I understand, but this patch doesn't make matters worse, as
> > long as there are not systems which have negative values for errno
>
> POSIX requires all defined errno values to be positive; negative errno
> values are unambiguous as values that will cause strerror() to have to
> generate a message about an unknown value.
>
> > (which I think we generally assume not to exist throughout qemu). That's
> > why I'm fine with it. We should fix the callers but I don't see why we
> > shouldn't apply this patch as well.
>
> This patch is a bandaid; it makes it harder to find callers that need to
> be fixed. I'd almost argue the exact opposite - add an assert(os_errno
> > 0). Then we'd loudly break on broken callers, making them easier to find.
Agree! use an assert to teach the caller ;)
> > A similar issue already came up and led to commit b276d2499, where
> > callers of error_setg_errno() assumed that it would not clobber errno,
> > so we fixed some of the callers but also applied that commit which just
> > saves errno because there's no reason not to.
>
> If we're willing to accept the convenience so that callers can be lazy,
> then I like this patch. If we want to fix bugs in the callers, then
> this patch makes it harder to find those bugs.
>
> I'm actually 60:40 in favor of this patch (I think the convenience
> outweighs an audit of fixing all callers); but if we do that, then we
> might also want to intentionally switch existing callers to pass
> negative values rather than declaring that passing a negative value is a
> bug.
--
Amos.
signature.asc
Description: Digital signature
[Qemu-devel] [PATCH] ui/input: strictly check console in finding input handler
qemu_input_find_handler() prefers a handler associated with con.
But if none exists, it takes any. This patch added a parameter
to strictly check console, in case we want to input event to
special console.
'input-send-event' has a parameter to assign special console,
so we should enable strict checking in finding handler.
Signed-off-by: Amos Kong
---
ui/input.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/ui/input.c b/ui/input.c
index 002831e..61b26a0 100644
--- a/ui/input.c
+++ b/ui/input.c
@@ -98,7 +98,7 @@ void qemu_input_handler_bind(QemuInputHandlerState *s,
}
static QemuInputHandlerState*
-qemu_input_find_handler(uint32_t mask, QemuConsole *con)
+qemu_input_find_handler(uint32_t mask, QemuConsole *con, bool strict_con)
{
QemuInputHandlerState *s;
@@ -111,6 +111,10 @@ qemu_input_find_handler(uint32_t mask, QemuConsole *con)
}
}
+if (strict_con) {
+return NULL;
+}
+
QTAILQ_FOREACH(s, &handlers, node) {
if (s->con != NULL) {
continue;
@@ -142,7 +146,7 @@ void qmp_input_send_event(int64_t console, InputEventList
*events,
for (e = events; e != NULL; e = e->next) {
InputEvent *event = e->value;
-if (!qemu_input_find_handler(1 << event->kind, con)) {
+if (!qemu_input_find_handler(1 << event->kind, con, true)) {
error_setg(errp, "Input handler not found for "
"event type %s",
InputEventKind_lookup[event->kind]);
@@ -311,7 +315,7 @@ void qemu_input_event_send(QemuConsole *src, InputEvent
*evt)
}
/* send event */
-s = qemu_input_find_handler(1 << evt->kind, src);
+s = qemu_input_find_handler(1 << evt->kind, src, NULL);
if (!s) {
return;
}
@@ -428,7 +432,7 @@ bool qemu_input_is_absolute(void)
QemuInputHandlerState *s;
s = qemu_input_find_handler(INPUT_EVENT_MASK_REL | INPUT_EVENT_MASK_ABS,
-NULL);
+NULL, NULL);
return (s != NULL) && (s->handler->mask & INPUT_EVENT_MASK_ABS);
}
--
1.9.3
Re: [Qemu-devel] [PATCH v2] ui/input: fix event emitting of repeated combined keys
On Mon, Sep 29, 2014 at 11:09:56AM +0200, Gerd Hoffmann wrote: > Hi, > > > It doesn't matter, so users might release the modifier key or not. > > we should make both works > > > > 1) > > sendkey Ctrl-Scroll > > sendkey Ctrl-Scroll > > Good to know this works. > > > 2) > > sendkey Ctrl-Scroll-Scroll > > Why? Quote from http://msdn.microsoft.com/en-us/library/windows/hardware/ff545499(v=vs.85).aspx The keyboard crash can be initiated by using the following hotkey sequence: _Hold down_ the rightmost CTRL key, and press the SCROLL LOCK key twice. > This tries to squeeze something into the sendkey interface which > it doesn't was designed for, and IMO this isn't a good idea, especially > if we have something better at hand (marcelos patch). My patch fixed this problem, but it's a hack. Marcelo's patch works in (press/release) event level, it's more clear. Thanks. > cheers, > Gerd > -- Amos.
Re: [Qemu-devel] [PATCH v2] ui/input: fix event emitting of repeated combined keys
On Fri, Sep 26, 2014 at 01:24:05PM +0200, Gerd Hoffmann wrote: > On Fr, 2014-09-26 at 18:53 +0800, Amos Kong wrote: > > On Fri, Sep 26, 2014 at 12:36:50PM +0200, Gerd Hoffmann wrote: > > > On Fr, 2014-09-26 at 18:23 +0800, Amos Kong wrote: > > > > Currently we emit press events of combined keys first, then emit > > > > release events by reverse order. But it doesn't match with physical > > > > keyboard if the keys contain continued & repeated keys. > > > > > > > > For example, (qemu) sendkey a-b-b > > > > > > Hmm, somehow I don't feel like building too much magic into this. > > > If you want send Ctrl-somekey twice just use two sendkey commands ... > > > > > > Before this patch, If 'sendkey r-o-o-t', only 'rot' can be inputted. > > To type 'root' you should use > > sendkey r > sendkey o > sendkey o > sendkey t > > Multiple keys in sendkey is meant for multiple keys pressed at the same > time, i.e. ctrl-alt-del, not for sending key sequences and typing words. > > > People want to panic windows by sending Ctrl-Scrool-Scrool > > http://msdn.microsoft.com/en-us/library/windows/hardware/ff545499(v=vs.85).aspx > > But current events order doesn't work. > > sendkey Ctrl-Scroll > sendkey Ctrl-Scroll > > > In physical keyboard. We can prese Ctrl first, then press & release > > Scroll twice, then release Ctrl. It's very common behavior. > > In most cases it doesn't matter whenever you release the modifier key or > not. The windows panic hotkey might be the exception from the rule > though. It doesn't matter, so users might release the modifier key or not. we should make both works 1) sendkey Ctrl-Scroll sendkey Ctrl-Scroll 2) sendkey Ctrl-Scroll-Scroll > > So this fix just reference the physical implement, if you want to > > input same key twice, you have to release it before second pressing. > > (here we ignore the auto-repeat feature) > > sendkey doesn't cover that use case indeed. original sendkey didn't cover it, but my change isn't added a split feature, but try to adapt native behavior by a little change. It doesn't effect original right behavior, but process repeated case in native way. > /me wonders what happened to the input-send-event patch from marcelo, > see http://patchwork.ozlabs.org/patch/360649/ > > According to patchwork I've picked it up. But it is neither upstream > nor in my local input branch. And I can't remember what happened :( > Marcelo, any clue? Maybe I should just re-queue it ... > > The input-send-event gives you fine-grained control about the exact > input event sequence and it can handle your use case without problems. > > cheers, > Gerd -- Amos.
Re: [Qemu-devel] [PATCH] input: fix send-key monitor command release event ordering
On Fri, Sep 26, 2014 at 10:08:50AM +0200, Gerd Hoffmann wrote:
> commit 2e377f1730d06deafb3e3ef6cf88792de4a6f4df changed the ordering
> of the release events as side effect. Some guests are not happy with
> that and don't recognise ctrl-alt-del any more. This patch restores
> the old last-pressed first-released behavior.
>
> Cc: Amos Kong
> Signed-off-by: Gerd Hoffmann
Acked-by: Amos Kong
Tested-by: Amos Kong
I will rebase my second patch [1] on your patch.
[1] [Qemu-devel] [PATCH] ui/input: fix event emitting of repeated combined keys
> ---
> ui/input-legacy.c | 11 +--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/ui/input-legacy.c b/ui/input-legacy.c
> index 3025f50..a698a34 100644
> --- a/ui/input-legacy.c
> +++ b/ui/input-legacy.c
> @@ -85,6 +85,8 @@ void qmp_send_key(KeyValueList *keys, bool has_hold_time,
> int64_t hold_time,
>Error **errp)
> {
> KeyValueList *p;
> +KeyValue **up = NULL;
> +int count = 0;
>
> if (!has_hold_time) {
> hold_time = 0; /* use default */
> @@ -93,11 +95,16 @@ void qmp_send_key(KeyValueList *keys, bool has_hold_time,
> int64_t hold_time,
> for (p = keys; p != NULL; p = p->next) {
> qemu_input_event_send_key(NULL, copy_key_value(p->value), true);
> qemu_input_event_send_key_delay(hold_time);
> +up = g_realloc(up, sizeof(*up) * (count+1));
> +up[count] = copy_key_value(p->value);
> +count++;
> }
> -for (p = keys; p != NULL; p = p->next) {
> -qemu_input_event_send_key(NULL, copy_key_value(p->value), false);
> +while (count) {
> +count--;
> +qemu_input_event_send_key(NULL, up[count], false);
> qemu_input_event_send_key_delay(hold_time);
> }
> +g_free(up);
> }
>
> static void legacy_kbd_event(DeviceState *dev, QemuConsole *src,
> --
> 1.8.3.1
>
--
Amos.
Re: [Qemu-devel] [PATCH v2] ui/input: fix event emitting of repeated combined keys
On Fri, Sep 26, 2014 at 12:36:50PM +0200, Gerd Hoffmann wrote: > On Fr, 2014-09-26 at 18:23 +0800, Amos Kong wrote: > > Currently we emit press events of combined keys first, then emit > > release events by reverse order. But it doesn't match with physical > > keyboard if the keys contain continued & repeated keys. > > > > For example, (qemu) sendkey a-b-b > > Hmm, somehow I don't feel like building too much magic into this. > If you want send Ctrl-somekey twice just use two sendkey commands ... Before this patch, If 'sendkey r-o-o-t', only 'rot' can be inputted. People want to panic windows by sending Ctrl-Scrool-Scrool http://msdn.microsoft.com/en-us/library/windows/hardware/ff545499(v=vs.85).aspx But current events order doesn't work. In physical keyboard. We can prese Ctrl first, then press & release Scroll twice, then release Ctrl. It's very common behavior. So this fix just reference the physical implement, if you want to input same key twice, you have to release it before second pressing. (here we ignore the auto-repeat feature) > cheers, > Gerd > -- Amos.
[Qemu-devel] [PATCH v2] ui/input: fix event emitting of repeated combined keys
Currently we emit press events of combined keys first, then emit
release events by reverse order. But it doesn't match with physical
keyboard if the keys contain continued & repeated keys.
For example, (qemu) sendkey a-b-b
Current emited events: (actually the second 'presse b' and 'release b'
can't be identified by guest, the interval is too short)
press a
press b
press b
release b
release b
release a
Correct events order of physical keyboard:
press a
press b
release b
press b
release b
release a
This patch fixed the event order if keys contain continued & repeated
keys. his patch based on:
http://lists.nongnu.org/archive/html/qemu-devel/2014-09/msg05150.html
Cc: Gerd Hoffmann
Signed-off-by: Amos Kong
---
V2: rebase this patch on Gerd's better fix of release order
---
ui/input-legacy.c | 14 +++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/ui/input-legacy.c b/ui/input-legacy.c
index a698a34..3fd3e83 100644
--- a/ui/input-legacy.c
+++ b/ui/input-legacy.c
@@ -95,9 +95,17 @@ void qmp_send_key(KeyValueList *keys, bool has_hold_time,
int64_t hold_time,
for (p = keys; p != NULL; p = p->next) {
qemu_input_event_send_key(NULL, copy_key_value(p->value), true);
qemu_input_event_send_key_delay(hold_time);
-up = g_realloc(up, sizeof(*up) * (count+1));
-up[count] = copy_key_value(p->value);
-count++;
+
+/* release event will be emitted immediately if next key is repeated */
+if (p->next && p->value->kind == p->next->value->kind &&
+p->value->qcode == p->next->value->qcode) {
+qemu_input_event_send_key(NULL, copy_key_value(p->value), false);
+qemu_input_event_send_key_delay(hold_time);
+} else {
+up = g_realloc(up, sizeof(*up) * (count+1));
+up[count] = copy_key_value(p->value);
+count++;
+}
}
while (count) {
count--;
--
1.9.3
[Qemu-devel] [PATCH] ui/input: fix event emitting of repeated combined keys
Currently we emit press events of combined keys first, then emit
release events by reverse order. But it doesn't match with physical
keyboard if the keys contain continued & repeated keys.
For example, (qemu) sendkey a-b-b
Current emited events: (actually the second 'presse b' and 'release b'
can't be identified by guest, the interval is too short)
press a
press b
press b
release b
release b
release a
Correct events order of physical keyboard:
press a
press b
release b
press b
release b
release a
This patch fixed the event order if keys contain continued & repeated
keys. This patch based on another sendkey fix:
http://lists.nongnu.org/archive/html/qemu-devel/2014-09/msg05149.html
Signed-off-by: Amos Kong
---
ui/input-legacy.c | 15 +--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/ui/input-legacy.c b/ui/input-legacy.c
index 1e1f14c..e6ea7a2 100644
--- a/ui/input-legacy.c
+++ b/ui/input-legacy.c
@@ -95,6 +95,14 @@ void qmp_send_key(KeyValueList *keys, bool has_hold_time,
int64_t hold_time,
for (p = keys; p != NULL; p = p->next) {
qemu_input_event_send_key(NULL, copy_key_value(p->value), true);
qemu_input_event_send_key_delay(hold_time);
+
+/* release event will be emitted immediately if next key is repeated */
+if (p->next && p->value->kind == p->next->value->kind &&
+p->value->qcode == p->next->value->qcode) {
+qemu_input_event_send_key(NULL, copy_key_value(p->value), false);
+qemu_input_event_send_key_delay(hold_time);
+}
+
count++;
}
while (count--) {
@@ -102,8 +110,11 @@ void qmp_send_key(KeyValueList *keys, bool has_hold_time,
int64_t hold_time,
for (p = keys; p != NULL && i < count; p = p->next) {
i++;
}
-qemu_input_event_send_key(NULL, copy_key_value(p->value), false);
-qemu_input_event_send_key_delay(hold_time);
+if (!(p->next && p->value->kind == p->next->value->kind &&
+ p->value->qcode == p->next->value->qcode)) {
+qemu_input_event_send_key(NULL, copy_key_value(p->value), false);
+qemu_input_event_send_key_delay(hold_time);
+}
}
}
--
1.9.3
[Qemu-devel] [PATCH] ui/input: correct the release order of combined keys
Commit 2e377f17 wrongly overturned the release order of combined keys,
it caused some Windows guests can't be paniced by "virsh send-key
KEY_RIGHTCTRL KEY_SCROLLLOCK KEY_SCROLLLOCK"
The press event order should be different with release order.
Signed-off-by: Amos Kong
---
ui/input-legacy.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/ui/input-legacy.c b/ui/input-legacy.c
index 3025f50..1e1f14c 100644
--- a/ui/input-legacy.c
+++ b/ui/input-legacy.c
@@ -85,16 +85,23 @@ void qmp_send_key(KeyValueList *keys, bool has_hold_time,
int64_t hold_time,
Error **errp)
{
KeyValueList *p;
+int count, i;
if (!has_hold_time) {
hold_time = 0; /* use default */
}
+count = 0;
for (p = keys; p != NULL; p = p->next) {
qemu_input_event_send_key(NULL, copy_key_value(p->value), true);
qemu_input_event_send_key_delay(hold_time);
+count++;
}
-for (p = keys; p != NULL; p = p->next) {
+while (count--) {
+i = 0;
+for (p = keys; p != NULL && i < count; p = p->next) {
+i++;
+}
qemu_input_event_send_key(NULL, copy_key_value(p->value), false);
qemu_input_event_send_key_delay(hold_time);
}
--
1.9.3
Re: [Qemu-devel] [PULL 2/7] input: use kbd delays for send_key monitor command
On Wed, Jun 04, 2014 at 09:44:06AM +0200, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann
> ---
> ui/input-legacy.c | 45 ++---
> 1 file changed, 6 insertions(+), 39 deletions(-)
>
> diff --git a/ui/input-legacy.c b/ui/input-legacy.c
> index 2a53860..3025f50 100644
> --- a/ui/input-legacy.c
> +++ b/ui/input-legacy.c
> @@ -74,27 +74,6 @@ int index_from_key(const char *key)
> return i;
> }
>
> -static KeyValue **keyvalues;
> -static int keyvalues_size;
> -static QEMUTimer *key_timer;
> -
> -static void free_keyvalues(void)
> -{
> -g_free(keyvalues);
> -keyvalues = NULL;
> -keyvalues_size = 0;
> -}
> -
> -static void release_keys(void *opaque)
> -{
> -while (keyvalues_size > 0) {
> -qemu_input_event_send_key(NULL, keyvalues[--keyvalues_size],
> - false);
> -}
> -
> -free_keyvalues();
> -}
> -
> static KeyValue *copy_key_value(KeyValue *src)
> {
> KeyValue *dst = g_new(KeyValue, 1);
> @@ -107,30 +86,18 @@ void qmp_send_key(KeyValueList *keys, bool
> has_hold_time, int64_t hold_time,
> {
> KeyValueList *p;
>
> -if (!key_timer) {
> -key_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, release_keys, NULL);
> -}
> -
> -if (keyvalues != NULL) {
> -timer_del(key_timer);
> -release_keys(NULL);
> -}
> -
> if (!has_hold_time) {
> -hold_time = 100;
> +hold_time = 0; /* use default */
> }
>
> for (p = keys; p != NULL; p = p->next) {
> qemu_input_event_send_key(NULL, copy_key_value(p->value), true);
> -
> -keyvalues = g_realloc(keyvalues, sizeof(KeyValue *) *
> - (keyvalues_size + 1));
> -keyvalues[keyvalues_size++] = copy_key_value(p->value);
> +qemu_input_event_send_key_delay(hold_time);
> +}
> +for (p = keys; p != NULL; p = p->next) {
Hi Gerd,
This patch caused a regression, it changed the releasing order
When we send a serial keys together, the right order should be:
press key1
press key2
press key3
release key3
release key2
release key1
We can check the events by shokey command in Linux.
This regression will caused "sendkey Alt-Ctrl-Delete" doesn't work on some
guests.
Thanks, Amos
> +qemu_input_event_send_key(NULL, copy_key_value(p->value), false);
> +qemu_input_event_send_key_delay(hold_time);
> }
> -
> -/* delayed key up events */
> -timer_mod(key_timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
> - muldiv64(get_ticks_per_sec(), hold_time, 1000));
> }
>
> static void legacy_kbd_event(DeviceState *dev, QemuConsole *src,
> --
> 1.8.3.1
>
--
Amos.
[Qemu-devel] [PATCH] qom: suppress conscan warning of returning null point
Conscan complains about g_malloc0() and malloc() return null. Error: NULL_RETURNS (CWE-476): qemu-kvm/qom/object.c:239: returned_null: Function "g_malloc0(gsize)" returns null. qemu-kvm/qom/object.c:239: var_assigned: Assigning: "ti->class" = null return value from "g_malloc0(gsize)". qemu-kvm/qom/object.c:249: dereference: Dereferencing a null pointer "ti->class". But if the passed size parameter is >= 1, then we can always get an effective pointer, the warning disappears. Signed-off-by: Amos Kong --- qom/object.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/qom/object.c b/qom/object.c index da0919a..0fbf2df 100644 --- a/qom/object.c +++ b/qom/object.c @@ -252,6 +252,7 @@ static void type_initialize(TypeImpl *ti) ti->class_size = type_class_get_size(ti); ti->instance_size = type_object_get_size(ti); +g_assert(ti->class_size != 0); ti->class = g_malloc0(ti->class_size); parent = type_get_parent(ti); @@ -424,6 +425,7 @@ Object *object_new_with_type(Type type) g_assert(type != NULL); type_initialize(type); +g_assert(type->instance_size != 0); obj = g_malloc(type->instance_size); object_initialize_with_type(obj, type->instance_size, type); obj->free = g_free; -- 1.9.3
Re: [Qemu-devel] [PATCH v5] virtio-pci: enable bus master for old guests
On Mon, Sep 15, 2014 at 12:59:44PM +0300, Michael S. Tsirkin wrote:
> On Mon, Sep 15, 2014 at 05:48:17PM +0800, Jason Wang wrote:
> > On 09/12/2014 02:20 AM, Michael S. Tsirkin wrote:
> > > commit cc943c36faa192cd4b32af8fe5edb31894017d35
> > > pci: Use bus master address space for delivering MSI/MSI-X messages
> > > breaks virtio-net for rhel6.[56] x86 guests because they don't
> > > enable bus mastering for virtio PCI devices. For the same reason,
> > > rhel6.[56] ppc64 guests cannot boot on a virtio-blk disk anymore.
> > >
> > > Old guests forgot to enable bus mastering, enable it automatically on
> > > DRIVER (guests use some devices before DRIVER_OK).
> > >
> > > Reported-by: Greg Kurz
> > > Signed-off-by: Michael S. Tsirkin
> > > ---
> > > Sorry, v4 was broken - forgot to commit before git format-patch.
> > > Here's the correct version.
> > > Still not tested - will do next week.
> > > Testing reports appreciated meanwhile.
> > >
> > > hw/virtio/virtio-pci.c | 10 ++
> > > 1 file changed, 10 insertions(+)
> > >
> > > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> > > index ddb5da1..a827cd4 100644
> > > --- a/hw/virtio/virtio-pci.c
> > > +++ b/hw/virtio/virtio-pci.c
> > > @@ -314,6 +314,16 @@ static void virtio_ioport_write(void *opaque,
> > > uint32_t addr, uint32_t val)
> > > msix_unuse_all_vectors(&proxy->pci_dev);
> > > }
> > >
> > > +/* Linux before 2.6.34 drives the device without enabling
> > > + the PCI device bus master bit. Enable it automatically
> > > + for the guest. This is a PCI spec violation but so is
> > > + initiating DMA with bus master bit clear. */
> > > +if (val == (VIRTIO_CONFIG_S_ACKNOWLEDGE |
> > > VIRTIO_CONFIG_S_DRIVER)) {
> > > +pci_default_write_config(&proxy->pci_dev, PCI_COMMAND,
> > > + proxy->pci_dev.config[PCI_COMMAND] |
> > > + PCI_COMMAND_MASTER, 1);
> > > +}
> > > +
> > > /* Linux before 2.6.34 sets the device as OK without enabling
> > > the PCI device bus master bit. In this case we need to disable
> > > some safety checks. */
> >
> > So with this patch, we can also remove the detecting and handling of
> > VIRTIO_PCI_FLAG_BUS_MATER_BUG?
>
> Yes - I sent a follow-up patch to do that.
I will check if this patch fixed this bug (it seems WIN8r2 doesn't set
BUS_MASTER bit correctly): https://bugzilla.redhat.com/show_bug.cgi?id=1090237
--
Amos.
Re: [Qemu-devel] [PATCH] hmp: Add info machines
On Thu, Aug 28, 2014 at 9:44 AM, zhanghailiang <
zhang.zhanghaili...@huawei.com> wrote:
> On 2014/8/27 18:00, Paolo Bonzini wrote:
>
>> Il 27/08/2014 11:24, zhanghailiang ha scritto:
>>
>>> This is the hmp counterpart of qmp query_machines
>>>
>>> Signed-off-by: zhanghailiang
>>> ---
>>> hmp-commands.hx | 2 ++
>>> hmp.c | 22 ++
>>> hmp.h | 1 +
>>> monitor.c | 7 +++
>>> 4 files changed, 32 insertions(+)
>>>
>>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>>> index d0943b1..1d04235 100644
>>> --- a/hmp-commands.hx
>>> +++ b/hmp-commands.hx
>>> @@ -1780,6 +1780,8 @@ show qdev device model list
>>> show roms
>>> @item info tpm
>>> show the TPM device
>>> +@item info machines
>>> +show supported machines information
>>> @end table
>>> ETEXI
>>>
>>> diff --git a/hmp.c b/hmp.c
>>> index 4d1838e..603f2f5 100644
>>> --- a/hmp.c
>>> +++ b/hmp.c
>>> @@ -725,6 +725,28 @@ void hmp_info_tpm(Monitor *mon, const QDict *qdict)
>>> qapi_free_TPMInfoList(info_list);
>>> }
>>>
>>> +void hmp_info_machines(Monitor *mon, const QDict *qdict)
>>> +{
>>> +MachineInfoList *mach_list = NULL, *cur_item = NULL;
>>> +
>>> +mach_list = qmp_query_machines(NULL);
>>> +for (cur_item = mach_list; cur_item; cur_item = cur_item->next) {
>>> +MachineInfo *machine;
>>> +char default_chr = ' ';
>>> +
>>> +machine = cur_item->value;
>>> +if (machine->is_default) {
>>> +default_chr = '*';
>>> +}
>>> +monitor_printf(mon, "%c %s:\n", default_chr, machine->name);
>>> +monitor_printf(mon, "max_cpus: %" PRId64 "\n",
>>> machine->cpu_max);
>>> +if (machine->has_alias) {
>>> +monitor_printf(mon, "alias: %s\n", machine->alias);
>>> +}
>>> +}
>>> +qapi_free_MachineInfoList(mach_list);
>>> +}
>>> +
>>> void hmp_quit(Monitor *mon, const QDict *qdict)
>>> {
>>> monitor_suspend(mon);
>>> diff --git a/hmp.h b/hmp.h
>>> index 4fd3c4a..374e841 100644
>>> --- a/hmp.h
>>> +++ b/hmp.h
>>> @@ -38,6 +38,7 @@ void hmp_info_balloon(Monitor *mon, const QDict
>>> *qdict);
>>> void hmp_info_pci(Monitor *mon, const QDict *qdict);
>>> void hmp_info_block_jobs(Monitor *mon, const QDict *qdict);
>>> void hmp_info_tpm(Monitor *mon, const QDict *qdict);
>>> +void hmp_info_machines(Monitor *mon, const QDict *qdict);
>>> void hmp_quit(Monitor *mon, const QDict *qdict);
>>> void hmp_stop(Monitor *mon, const QDict *qdict);
>>> void hmp_system_reset(Monitor *mon, const QDict *qdict);
>>> diff --git a/monitor.c b/monitor.c
>>> index 34cee74..adc3645 100644
>>> --- a/monitor.c
>>> +++ b/monitor.c
>>> @@ -2921,6 +2921,13 @@ static mon_cmd_t info_cmds[] = {
>>> .mhandler.cmd = hmp_info_memdev,
>>> },
>>> {
>>> +.name = "machines",
>>> +.args_type = "",
>>> +.params = "",
>>> +.help = "show supported machines information",
>>> +.mhandler.cmd = hmp_info_machines,
>>> +},
>>> +{
>>> .name = NULL,
>>> },
>>> };
>>>
>>>
>> What is this useful for? You can use "-machine help".
>>
>>
> Hmm, i just looked into the qmp commands and hmp commands,
> And found there is qmp_query_machines but no hmp_info_machines...
>
> Sorry for the noise;)
>
>
There exists some QMP commands that don't have corresponding HMP commands.
QMP is the latest and recommended interface for management tools.
HMP command is not always necessary, unless you have a good reason :-)
Re: [Qemu-devel] [PATCH] Revert "virtio_rng: replace custom backend API with UserCreatable.complete() callback"
On Tue, Aug 19, 2014 at 03:47:20PM +0100, Stefan Hajnoczi wrote: > On Sat, Aug 16, 2014 at 12:23:52AM +0800, Amos Kong wrote: > > This reverts commit 57d3e1b3f52d07d215ed96df946ee01f8d9f9526. > > > > The commit introduced a regression bug, the initialization order of > > virtio-rng > > backend was changed. > > > > # x86_64-softmmu/qemu-system-x86_64 -monitor stdio -vnc :0 \ > > -chardev socket,host=localhost,port=1024,id=chr0 \ > > -object rng-egd,chardev=chr0,id=rng0 Paolo posted a new patch [1] to fix this bug, above commandline works. > > qemu-system-x86_64: -object rng-egd,chardev=chr0,id=rng0: Device 'chr0' > > not found > > > > Chardev 'chr0' isn't initialized when we try to open rng backend, > > More detail: > The problem is that vl.c:main() calls object_create() on -object before > -chardev options are processed. Moving the object_create() call after > chardev is arbitrary and does not work if a chardev depends on an > -object. Stefan, can you give an example of the complex relying? So Paolo's patch can't solve this problem? > It would have been nice to process command-line options in left-to-right > order instead of grouping them by option type. I doubt we can change > this now since it would break the command-line. > > > Conflicts: > > hw/virtio/virtio-rng.c > > This shouldn't be here. > > I agree with reverting commit 57d3e1b3f52d07d215ed96df946ee01f8d9f9526. -- Amos. pgpKSkgMiF8ij.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH RESEND 1/2] rng-egd: Free old chr_name value before setting new one
On Thu, Aug 07, 2014 at 12:18:47PM +0200, Paolo Bonzini wrote: > Il 07/08/2014 04:24, Amos Kong ha scritto: > > Eduardo, a similar patch had been applied. > > > > commit 5e490b6a504912225dff0e520e1c6af68295d238 > > Author: Amos Kong > > Date: Thu Nov 21 16:42:50 2013 +0800 > > > > rng-egd: remove redundant free > > > > We didn't set default chr_name, the free is redundant. > > > > Signed-off-by: Amos Kong > > Message-id: 1385023371-8198-2-git-send-email-ak...@redhat.com > > Signed-off-by: Anthony Liguori > > > > What if you set chr_name twice? In this case, g_free() is necessary. So I'm fine with Eduardo's patch: [PATCH RESEND 1/2] rng-egd: Free old chr_name value > Paolo -- Amos.
Re: [Qemu-devel] [PATCH v6 0/4] test virtio-blk hotplug
On Sun, Jul 06, 2014 at 08:14:46PM +0800, Amos Kong wrote: > On Wed, Jun 25, 2014 at 09:15:12AM +0800, Amos Kong wrote: > > It's worth to add a hotplug test to qtest, but without > > cooperation of guest OS, new devices can't be initialized > > by guest, and hot-unplug doesn't work. > > > > However, the new test can cover some part of code of > > hotplug/unplug. > > > > I will write another subtest to test hotplug with pci support. > > > > V2: move qmp_exec_hmp_cmd() to libqtest.c > > excape hmp cmd (stefanha) > > use qmp_exec_hmp_cmd() in blockdev-test > > V3: use vp_list to format string, free escaped string > > V4: free escaped string by g_free() > > V5: escape cmd string in QMP command (so this patchset depends on > > http://article.gmane.org/gmane.comp.emulators.qemu/279835) > > directly use QMP cmd to add/del device and blockdev > > cleanup as suggested by Andreas > > V6: hot-unplug devices of one slot once > > escaping string fix in 1st patch depends on > > (https://lists.nongnu.org/archive/html/qemu-devel/2014-06/msg03024.html) > > Kevin, Stefan, Andreas > > Can you help to review & apply this set? ping :-) > Thanks. Amos > > > Amos Kong (4): > > qtest: introduce qmp_exec_hmp_cmd() > > qtest: use qmp_exec_hmp_cmd() in blockdev-test > > virtio-blk-test: change pci_nop() to virtblk_init() > > virtio-blk-test: add hotplug subtest > > > > tests/blockdev-test.c | 23 ++ > > tests/libqtest.c| 23 ++ > > tests/libqtest.h| 9 ++ > > tests/virtio-blk-test.c | 81 > > - > > 4 files changed, 108 insertions(+), 28 deletions(-) > > > > -- > > 1.9.3 -- Amos.
[Qemu-devel] [PATCH] Revert "virtio_rng: replace custom backend API with UserCreatable.complete() callback"
This reverts commit 57d3e1b3f52d07d215ed96df946ee01f8d9f9526.
The commit introduced a regression bug, the initialization order of virtio-rng
backend was changed.
# x86_64-softmmu/qemu-system-x86_64 -monitor stdio -vnc :0 \
-chardev socket,host=localhost,port=1024,id=chr0 \
-object rng-egd,chardev=chr0,id=rng0
qemu-system-x86_64: -object rng-egd,chardev=chr0,id=rng0: Device 'chr0' not
found
Chardev 'chr0' isn't initialized when we try to open rng backend,
Signed-off-by: Amos Kong
Conflicts:
hw/virtio/virtio-rng.c
---
backends/rng.c | 12 ++--
hw/virtio/virtio-rng.c | 15 ++-
include/sysemu/rng.h | 11 +++
3 files changed, 19 insertions(+), 19 deletions(-)
diff --git a/backends/rng.c b/backends/rng.c
index 0f2fc11..26faf44 100644
--- a/backends/rng.c
+++ b/backends/rng.c
@@ -41,9 +41,9 @@ static bool rng_backend_prop_get_opened(Object *obj, Error
**errp)
return s->opened;
}
-static void rng_backend_complete(UserCreatable *uc, Error **errp)
+void rng_backend_open(RngBackend *s, Error **errp)
{
-object_property_set_bool(OBJECT(uc), true, "opened", errp);
+object_property_set_bool(OBJECT(s), true, "opened", errp);
}
static void rng_backend_prop_set_opened(Object *obj, bool value, Error **errp)
@@ -80,20 +80,12 @@ static void rng_backend_init(Object *obj)
NULL);
}
-static void rng_backend_class_init(ObjectClass *oc, void *data)
-{
-UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
-
-ucc->complete = rng_backend_complete;
-}
-
static const TypeInfo rng_backend_info = {
.name = TYPE_RNG_BACKEND,
.parent = TYPE_OBJECT,
.instance_size = sizeof(RngBackend),
.instance_init = rng_backend_init,
.class_size = sizeof(RngBackendClass),
-.class_init = rng_backend_class_init,
.abstract = true,
.interfaces = (InterfaceInfo[]) {
{ TYPE_USER_CREATABLE },
diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c
index 03fd04a..9323cbe 100644
--- a/hw/virtio/virtio-rng.c
+++ b/hw/virtio/virtio-rng.c
@@ -15,7 +15,6 @@
#include "hw/virtio/virtio.h"
#include "hw/virtio/virtio-rng.h"
#include "sysemu/rng.h"
-#include "qom/object_interfaces.h"
static bool is_guest_ready(VirtIORNG *vrng)
{
@@ -157,14 +156,6 @@ static void virtio_rng_device_realize(DeviceState *dev,
Error **errp)
if (vrng->conf.rng == NULL) {
vrng->conf.default_backend = RNG_RANDOM(object_new(TYPE_RNG_RANDOM));
-user_creatable_complete(OBJECT(vrng->conf.default_backend),
-&local_err);
-if (local_err) {
-error_propagate(errp, local_err);
-object_unref(OBJECT(vrng->conf.default_backend));
-return;
-}
-
object_property_add_child(OBJECT(dev),
"default-backend",
OBJECT(vrng->conf.default_backend),
@@ -186,6 +177,12 @@ static void virtio_rng_device_realize(DeviceState *dev,
Error **errp)
virtio_init(vdev, "virtio-rng", VIRTIO_ID_RNG, 0);
+rng_backend_open(vrng->rng, &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
+
vrng->vq = virtio_add_queue(vdev, 8, handle_input);
vrng->quota_remaining = vrng->conf.max_bytes;
diff --git a/include/sysemu/rng.h b/include/sysemu/rng.h
index 0a27c9b..7637fac 100644
--- a/include/sysemu/rng.h
+++ b/include/sysemu/rng.h
@@ -79,4 +79,15 @@ void rng_backend_request_entropy(RngBackend *s, size_t size,
* to stop tracking any request.
*/
void rng_backend_cancel_requests(RngBackend *s);
+
+/**
+ * rng_backend_open:
+ * @s: the backend to open
+ * @errp: a pointer to return the #Error object if an error occurs.
+ *
+ * This function will open the backend if it is not already open. Calling this
+ * function on an already opened backend will not result in an error.
+ */
+void rng_backend_open(RngBackend *s, Error **errp);
+
#endif
--
1.9.3
Re: [Qemu-devel] [PATCH v1 4/4] virtio_rng: replace custom backend API with UserCreatable.complete() callback
On Thu, Jan 16, 2014 at 05:34:39PM +0100, Igor Mammedov wrote:
> in addition fix default backend leak by releasing it if its
> initialization failed.
>
> Signed-off-by: Igor Mammedov
Hi Igor,
This patch introduced a regression.
x86_64-softmmu/qemu-system-x86_64 -monitor stdio -vnc :0 \
-chardev socket,host=localhost,port=1024,id=chr0 \
-object rng-egd,chardev=chr0,id=rng0
qemu-system-x86_64: -object rng-egd,chardev=chr0,id=rng0: Device 'chr0' not
found
chardev 'chr0' isn't initialized when we try to open rng backend.
RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1128095
> ---
> backends/rng.c | 12 ++--
> hw/virtio/virtio-rng.c | 15 +--
> include/sysemu/rng.h | 11 ---
> 3 files changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/backends/rng.c b/backends/rng.c
> index 7bab806..8b8d5a4 100644
> --- a/backends/rng.c
> +++ b/backends/rng.c
> @@ -41,9 +41,9 @@ static bool rng_backend_prop_get_opened(Object *obj, Error
> **errp)
> return s->opened;
> }
>
> -void rng_backend_open(RngBackend *s, Error **errp)
> +static void rng_backend_complete(UserCreatable *uc, Error **errp)
> {
> -object_property_set_bool(OBJECT(s), true, "opened", errp);
> +object_property_set_bool(OBJECT(uc), true, "opened", errp);
> }
>
> static void rng_backend_prop_set_opened(Object *obj, bool value, Error
> **errp)
> @@ -77,12 +77,20 @@ static void rng_backend_init(Object *obj)
> NULL);
> }
>
> +static void rng_backend_class_init(ObjectClass *oc, void *data)
> +{
> +UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
> +
> +ucc->complete = rng_backend_complete;
> +}
> +
> static const TypeInfo rng_backend_info = {
> .name = TYPE_RNG_BACKEND,
> .parent = TYPE_OBJECT,
> .instance_size = sizeof(RngBackend),
> .instance_init = rng_backend_init,
> .class_size = sizeof(RngBackendClass),
> +.class_init = rng_backend_class_init,
> .abstract = true,
> .interfaces = (InterfaceInfo[]) {
> { TYPE_USER_CREATABLE },
> diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c
> index 755fdee..a16e3bc 100644
> --- a/hw/virtio/virtio-rng.c
> +++ b/hw/virtio/virtio-rng.c
> @@ -15,6 +15,7 @@
> #include "hw/virtio/virtio.h"
> #include "hw/virtio/virtio-rng.h"
> #include "sysemu/rng.h"
> +#include "qom/object_interfaces.h"
>
> static bool is_guest_ready(VirtIORNG *vrng)
> {
> @@ -148,6 +149,14 @@ static void virtio_rng_device_realize(DeviceState *dev,
> Error **errp)
> if (vrng->conf.rng == NULL) {
> vrng->conf.default_backend = RNG_RANDOM(object_new(TYPE_RNG_RANDOM));
>
> +user_creatable_complete(OBJECT(vrng->conf.default_backend),
> +&local_err);
> +if (local_err) {
> +error_propagate(errp, local_err);
> +object_unref(OBJECT(vrng->conf.default_backend));
> +return;
> +}
> +
> object_property_add_child(OBJECT(dev),
>"default-backend",
>OBJECT(vrng->conf.default_backend),
> @@ -166,12 +175,6 @@ static void virtio_rng_device_realize(DeviceState *dev,
> Error **errp)
> return;
> }
>
> -rng_backend_open(vrng->rng, &local_err);
> -if (local_err) {
> -error_propagate(errp, local_err);
> -return;
> -}
> -
> vrng->vq = virtio_add_queue(vdev, 8, handle_input);
>
> assert(vrng->conf.max_bytes <= INT64_MAX);
> diff --git a/include/sysemu/rng.h b/include/sysemu/rng.h
> index 7637fac..0a27c9b 100644
> --- a/include/sysemu/rng.h
> +++ b/include/sysemu/rng.h
> @@ -79,15 +79,4 @@ void rng_backend_request_entropy(RngBackend *s, size_t
> size,
> * to stop tracking any request.
> */
> void rng_backend_cancel_requests(RngBackend *s);
> -
> -/**
> - * rng_backend_open:
> - * @s: the backend to open
> - * @errp: a pointer to return the #Error object if an error occurs.
> - *
> - * This function will open the backend if it is not already open. Calling
> this
> - * function on an already opened backend will not result in an error.
> - */
> -void rng_backend_open(RngBackend *s, Error **errp);
> -
> #endif
> --
> 1.7.1
>
--
Amos.
Re: [Qemu-devel] [PATCH RESEND 2/2] hw/machine: Free old values of string properties
On Wed, Aug 06, 2014 at 03:18:21PM -0300, Eduardo Habkost wrote:
> Reviewed-by: Markus Armbruster
> Reviewed-by: Marcel Apfelbaum
> Signed-off-by: Eduardo Habkost
Reviewed-by: Amos Kong
> ---
> hw/core/machine.c | 8
> 1 file changed, 8 insertions(+)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 7a66c57..d145aca 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -24,6 +24,7 @@ static void machine_set_accel(Object *obj, const char
> *value, Error **errp)
> {
> MachineState *ms = MACHINE(obj);
>
> +g_free(ms->accel);
> ms->accel = g_strdup(value);
> }
>
> @@ -79,6 +80,7 @@ static void machine_set_kernel(Object *obj, const char
> *value, Error **errp)
> {
> MachineState *ms = MACHINE(obj);
>
> +g_free(ms->kernel_filename);
> ms->kernel_filename = g_strdup(value);
> }
>
> @@ -93,6 +95,7 @@ static void machine_set_initrd(Object *obj, const char
> *value, Error **errp)
> {
> MachineState *ms = MACHINE(obj);
>
> +g_free(ms->initrd_filename);
> ms->initrd_filename = g_strdup(value);
> }
>
> @@ -107,6 +110,7 @@ static void machine_set_append(Object *obj, const char
> *value, Error **errp)
> {
> MachineState *ms = MACHINE(obj);
>
> +g_free(ms->kernel_cmdline);
> ms->kernel_cmdline = g_strdup(value);
> }
>
> @@ -121,6 +125,7 @@ static void machine_set_dtb(Object *obj, const char
> *value, Error **errp)
> {
> MachineState *ms = MACHINE(obj);
>
> +g_free(ms->dtb);
> ms->dtb = g_strdup(value);
> }
>
> @@ -135,6 +140,7 @@ static void machine_set_dumpdtb(Object *obj, const char
> *value, Error **errp)
> {
> MachineState *ms = MACHINE(obj);
>
> +g_free(ms->dumpdtb);
> ms->dumpdtb = g_strdup(value);
> }
>
> @@ -176,6 +182,7 @@ static void machine_set_dt_compatible(Object *obj, const
> char *value, Error **er
> {
> MachineState *ms = MACHINE(obj);
>
> +g_free(ms->dt_compatible);
> ms->dt_compatible = g_strdup(value);
> }
>
> @@ -232,6 +239,7 @@ static void machine_set_firmware(Object *obj, const char
> *value, Error **errp)
> {
> MachineState *ms = MACHINE(obj);
>
> +g_free(ms->firmware);
> ms->firmware = g_strdup(value);
> }
>
> --
> 1.9.3
>
--
Amos.
Re: [Qemu-devel] [PATCH RESEND 1/2] rng-egd: Free old chr_name value before setting new one
On Wed, Aug 06, 2014 at 03:18:20PM -0300, Eduardo Habkost wrote:
> Reviewed-by: Markus Armbruster
> Signed-off-by: Eduardo Habkost
> ---
> backends/rng-egd.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/backends/rng-egd.c b/backends/rng-egd.c
> index 25bb3b4..2962795 100644
> --- a/backends/rng-egd.c
> +++ b/backends/rng-egd.c
> @@ -169,6 +169,7 @@ static void rng_egd_set_chardev(Object *obj, const char
> *value, Error **errp)
> if (b->opened) {
> error_set(errp, QERR_PERMISSION_DENIED);
> } else {
> +g_free(s->chr_name);
> s->chr_name = g_strdup(value);
> }
> }
Eduardo, a similar patch had been applied.
commit 5e490b6a504912225dff0e520e1c6af68295d238
Author: Amos Kong
Date: Thu Nov 21 16:42:50 2013 +0800
rng-egd: remove redundant free
We didn't set default chr_name, the free is redundant.
Signed-off-by: Amos Kong
Message-id: 1385023371-8198-2-git-send-email-ak...@redhat.com
Signed-off-by: Anthony Liguori
> --
> 1.9.3
--
Amos.
Re: [Qemu-devel] [PATCH 1/3] rng-random: NULL check not needed before g_free()
On Mon, Jun 02, 2014 at 01:38:52PM +0200, Markus Armbruster wrote:
> Eduardo Habkost writes:
>
> > g_free() is NULL-safe.
> >
> > Signed-off-by: Eduardo Habkost
> > ---
> > Cc: Anthony Liguori
> > Cc: Luiz Capitulino
> > ---
> > backends/rng-random.c | 5 +
> > 1 file changed, 1 insertion(+), 4 deletions(-)
> >
> > diff --git a/backends/rng-random.c b/backends/rng-random.c
> > index 136499d..601d9dc 100644
> > --- a/backends/rng-random.c
> > +++ b/backends/rng-random.c
> > @@ -106,10 +106,7 @@ static void rng_random_set_filename(Object *obj, const
> > char *filename,
> > return;
> > }
> >
> > -if (s->filename) {
> > -g_free(s->filename);
> > -}
> > -
> > +g_free(s->filename);
> > s->filename = g_strdup(filename);
> > }
>
> Reviewed-by: Markus Armbruster
>
>
> Note there are a quite a few more instances of this anti-pattern.
> Coccinelle can find them, semantic patch appended. Beware, it can throw
> away comments in the conditional, which is usually not what we want.
>
>
> @@
> expression E;
> @@
> - if (E != NULL) {
> - g_free(E);
> - }
> + g_free(E);
[amos@z qemu]$ grep g_free -B 2 -r * |less
then try to search "/if.*NULL"
Not all can be found by this way, but many instances are found.
--
Amos.
Re: [Qemu-devel] [PATCH v2 1/1] virtio-rng: add some trace events
On Mon, Aug 04, 2014 at 04:22:44PM +0530, Amit Shah wrote:
> Add some trace events to virtio-rng for easier debugging
>
> Signed-off-by: Amit Shah
>
> ---
> v2:
> - requested_size trace event now shows proper values
Reviewed-by: Amos Kong
> ---
> hw/virtio/virtio-rng.c | 6 ++
> trace-events | 5 +
> 2 files changed, 11 insertions(+)
>
> diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c
> index 03fd04a..e85a979 100644
> --- a/hw/virtio/virtio-rng.c
> +++ b/hw/virtio/virtio-rng.c
> @@ -16,6 +16,7 @@
> #include "hw/virtio/virtio-rng.h"
> #include "sysemu/rng.h"
> #include "qom/object_interfaces.h"
> +#include "trace.h"
>
> static bool is_guest_ready(VirtIORNG *vrng)
> {
> @@ -24,6 +25,7 @@ static bool is_guest_ready(VirtIORNG *vrng)
> && (vdev->status & VIRTIO_CONFIG_S_DRIVER_OK)) {
> return true;
> }
> +trace_virtio_rng_guest_not_ready(vrng);
> return false;
> }
>
> @@ -62,6 +64,7 @@ static void chr_read(void *opaque, const void *buf, size_t
> size)
> offset += len;
>
> virtqueue_push(vrng->vq, &elem, len);
> +trace_virtio_rng_pushed(vrng, len);
> }
> virtio_notify(vdev, vrng->vq);
> }
> @@ -81,6 +84,9 @@ static void virtio_rng_process(VirtIORNG *vrng)
> quota = MIN((uint64_t)vrng->quota_remaining, (uint64_t)UINT32_MAX);
> }
> size = get_request_size(vrng->vq, quota);
> +
> +trace_virtio_rng_request(vrng, size, quota);
> +
> size = MIN(vrng->quota_remaining, size);
> if (size) {
> rng_backend_request_entropy(vrng->rng, size, chr_read, vrng);
> diff --git a/trace-events b/trace-events
> index 11a17a8..99f39ac 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -41,6 +41,11 @@ virtio_irq(void *vq) "vq %p"
> virtio_notify(void *vdev, void *vq) "vdev %p vq %p"
> virtio_set_status(void *vdev, uint8_t val) "vdev %p val %u"
>
> +# hw/virtio/virtio-rng.c
> +virtio_rng_guest_not_ready(void *rng) "rng %p: guest not ready"
> +virtio_rng_pushed(void *rng, size_t len) "rng %p: %zd bytes pushed"
> +virtio_rng_request(void *rng, size_t size, unsigned quota) "rng %p: %zd
> bytes requested, %u bytes quota left"
> +
> # hw/char/virtio-serial-bus.c
> virtio_serial_send_control_event(unsigned int port, uint16_t event, uint16_t
> value) "port %u, event %u, value %u"
> virtio_serial_throttle_port(unsigned int port, bool throttle) "port %u,
> throttle %d"
> --
> 1.9.3
--
Amos.
Re: [Qemu-devel] [PATCH] vhost_net: cleanup start/stop condition
On Mon, Aug 04, 2014 at 02:12:39PM +0200, Michael S. Tsirkin wrote:
> Checking vhost device internal state in vhost_net looks like
> a layering violation since vhost_net does not
> set this flag: it is set and tested by vhost.c.
> There seems to be no reason to check this:
> caller in virtio net uses its own flag,
> vhost_started, to ensure vhost is started/stopped
> as appropriate.
Reasonable, we already checked n->vhost_started flag in virtio-net.c
when vhost_net_start() is called.
Reviewed-by: Amos Kong
> Signed-off-by: Michael S. Tsirkin
> ---
> hw/net/vhost_net.c | 11 +++
> 1 file changed, 3 insertions(+), 8 deletions(-)
>
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index f87c798..d9423d1 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -195,10 +195,6 @@ static int vhost_net_start_one(struct vhost_net *net,
> struct vhost_vring_file file = { };
> int r;
>
> -if (net->dev.started) {
> -return 0;
> -}
> -
> net->dev.nvqs = 2;
> net->dev.vqs = net->vqs;
> net->dev.vq_index = vq_index;
> @@ -251,15 +247,11 @@ fail_notifiers:
> return r;
> }
>
> static void vhost_net_stop_one(struct vhost_net *net,
> VirtIODevice *dev)
> {
> struct vhost_vring_file file = { .fd = -1 };
>
> -if (!net->dev.started) {
> -return;
> -}
> -
> if (net->nc->info->type == NET_CLIENT_OPTIONS_KIND_TAP) {
> for (file.index = 0; file.index < net->dev.nvqs; ++file.index) {
> const VhostOps *vhost_ops = net->dev.vhost_ops;
> --
> MST
--
Amos.
Re: [Qemu-devel] [PATCH v2] po: Add Chinese translation
On Thu, Jul 31, 2014 at 11:18:45AM +0800, Fam Zheng wrote: > Signed-off-by: Fam Zheng > > --- > v2: Fix typo for "_View". > --- > po/zh_CN.po | 86 > + > 1 file changed, 86 insertions(+) > create mode 100644 po/zh_CN.po Reviewed-by: Amos Kong > diff --git a/po/zh_CN.po b/po/zh_CN.po > new file mode 100644 > index 000..7ffba30 > --- /dev/null > +++ b/po/zh_CN.po > @@ -0,0 +1,86 @@ > +# Chinese translation for QEMU. > +# This file is put in the public domain. > +# > +# Fam Zheng , 2014 > +msgid "" > +msgstr "" > +"Project-Id-Version: QEMU 2.2\n" > +"Report-Msgid-Bugs-To: qemu-devel@nongnu.org\n" > +"POT-Creation-Date: 2014-07-31 10:03+0800\n" > +"PO-Revision-Date: 2014-07-31 10:00+0800\n" > +"Last-Translator: Fam Zheng \n" > +"Language-Team: Chinese \n" > +"Language: zh\n" > +"MIME-Version: 1.0\n" > +"Content-Type: text/plain; charset=UTF-8\n" > +"Content-Transfer-Encoding: 8bit\n" > +"Plural-Forms: nplurals=2; plural=n != 1;\n" > +"X-Generator: Lokalize 1.4\n" > + > +#: ui/gtk.c:321 > +msgid " - Press Ctrl+Alt+G to release grab" > +msgstr "- 按下 Ctrl+Alt+G 取消捕获" ^ lost one blank space > + > +#: ui/gtk.c:325 > +msgid " [Paused]" > +msgstr " [已暂停]" > + > +#: ui/gtk.c:1601 > +msgid "_Pause" > +msgstr "暂停(_P)" > + > +#: ui/gtk.c:1607 > +msgid "_Reset" > +msgstr "重置(_R)" > + > +#: ui/gtk.c:1610 > +msgid "Power _Down" > +msgstr "关闭电源(_D)" > + > +#: ui/gtk.c:1616 > +msgid "_Quit" > +msgstr "退出(_Q)" > + > +#: ui/gtk.c:1692 > +msgid "_Fullscreen" > +msgstr "全屏(_F)" > + > +#: ui/gtk.c:1702 > +msgid "Zoom _In" > +msgstr "放大(_I)" > + > +#: ui/gtk.c:1709 > +msgid "Zoom _Out" > +msgstr "缩小(_O)" > + > +#: ui/gtk.c:1716 > +msgid "Best _Fit" > +msgstr "最合适大小(_F)" > + > +#: ui/gtk.c:1723 > +msgid "Zoom To _Fit" > +msgstr "缩放以适应大小(_F)" > + > +#: ui/gtk.c:1729 > +msgid "Grab On _Hover" > +msgstr "鼠标经过时捕获(_H)" > + > +#: ui/gtk.c:1732 > +msgid "_Grab Input" > +msgstr "捕获输入(_G)" > + > +#: ui/gtk.c:1761 > +msgid "Show _Tabs" > +msgstr "显示标签页(_T)" > + > +#: ui/gtk.c:1764 > +msgid "Detach Tab" > +msgstr "分离标签页" > + > +#: ui/gtk.c:1778 > +msgid "_Machine" > +msgstr "虚拟机(_M)" > + > +#: ui/gtk.c:1783 > +msgid "_View" > +msgstr "视图(_V)" > -- > 2.0.3 -- Amos.
Re: [Qemu-devel] [PATCH v3 for-2.1 1/1] virtio-serial-bus: keep port 0 reserved for virtconsole even on unplug
On Tue, Jul 15, 2014 at 03:31:32PM +0530, Amit Shah wrote:
> We keep port 0 reserved for compat with older guests, where only
> virtio-console was expected. Even if a system is started without a
> virtio-console port, port #0 is kept aside. However, after a
> virtconsole port is unplugged, port id 0 became available, and the next
> hotplug of a virtserialport caused failure due to it not being a console
> port.
>
> Steps to reproduce:
>
> $ ./x86_64-softmmu/qemu-system-x86_64 -m 512 -cpu host -enable-kvm -device
> virtio-serial-pci -monitor stdio -vnc :1
> QEMU 2.0.91 monitor - type 'help' for more information
> (qemu) device_add virtconsole,id=p1
> (qemu) device_del p1
> (qemu) device_add virtserialport,id=p1
> Port number 0 on virtio-serial devices reserved for virtconsole devices for
> backward compatibility.
> Device 'virtserialport' could not be initialized
> (qemu) quit
>
> Reported-by: dengmin
> Signed-off-by: Amit Shah
Reviewed-by: Amos Kong
> ---
> v2:
> - move calculation of 'i' inside if stmt too (Paolo)
> - re-word comment
>
> v3:
> - fix typo in comment
> ---
> hw/char/virtio-serial-bus.c | 14 +++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
> index 07bebc0..23123b7 100644
> --- a/hw/char/virtio-serial-bus.c
> +++ b/hw/char/virtio-serial-bus.c
> @@ -797,10 +797,18 @@ static void add_port(VirtIOSerial *vser, uint32_t
> port_id)
> static void remove_port(VirtIOSerial *vser, uint32_t port_id)
> {
> VirtIOSerialPort *port;
> -unsigned int i;
>
> -i = port_id / 32;
> -vser->ports_map[i] &= ~(1U << (port_id % 32));
> +/*
> + * Don't mark port 0 removed -- we explicitly reserve it for
> + * backward compat with older guests, ensure a virtconsole device
> + * unplug retains the reservation.
> + */
> +if (port_id) {
> +unsigned int i;
> +
> +i = port_id / 32;
> +vser->ports_map[i] &= ~(1U << (port_id % 32));
> +}
>
> port = find_port_by_id(vser, port_id);
> /*
> --
> 1.9.3
--
Amos.
Re: [Qemu-devel] [RFC PATCH 1/5] bootindex: add *_boot_device_path function
On Tue, Jul 08, 2014 at 01:22:53PM +, Gonglei (Arei) wrote:
> > -Original Message-
> > From: chenliang (T)
> > Sent: Tuesday, July 08, 2014 7:03 PM
> > To: Amos Kong
> > Cc: Gonglei (Arei); qemu-devel@nongnu.org; afaer...@suse.de;
> > ag...@suse.de; stefa...@redhat.com; a...@ozlabs.ru;
> > alex.william...@redhat.com; arm...@redhat.com; ebl...@redhat.com;
> > kw...@redhat.com; peter.mayd...@linaro.org; lcapitul...@redhat.com;
> > m...@redhat.com; pbonz...@redhat.com; ler...@redhat.com;
> > kra...@redhat.com; imamm...@redhat.com; dmi...@daynix.com;
> > marce...@redhat.com; peter.crosthwa...@xilinx.com; r...@twiddle.net;
> > so...@cmu.edu; Huangweidong (C); Luonengjun; Huangpeng (Peter)
> > Subject: Re: [RFC PATCH 1/5] bootindex: add *_boot_device_path function
> >
> > On 2014/7/8 16:33, Amos Kong wrote:
> >
> > > On Mon, Jul 07, 2014 at 05:10:57PM +0800, arei.gong...@huawei.com
> > wrote:
> > >> From: Chenliang
> > >>
> > >> Add del_boot_device_path and modify_boot_device_path. Device should
> > >> be removed from boot device list by del_boot_device_path when device
> > >> hotplug. modify_boot_device_path is used to modify deviceboot order.
> > >
> > > s/hotplug/is unhotplugged/
> > >
> > > same issue in commitlog of patch 3/5
> > >
>
> Yep, thanks!
>
> > >> Signed-off-by: Chenliang
> > >> Signed-off-by: Gonglei
> > >> ---
> > >> include/sysemu/sysemu.h | 4
> > >> vl.c| 55
> > +
> > >> 2 files changed, 59 insertions(+)
> > >>
> > >> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> > >> index 285c45b..38ef1cd 100644
> > >> --- a/include/sysemu/sysemu.h
> > >> +++ b/include/sysemu/sysemu.h
> > >> @@ -204,6 +204,10 @@ void usb_info(Monitor *mon, const QDict *qdict);
> > >>
> > >> void add_boot_device_path(int32_t bootindex, DeviceState *dev,
> > >>const char *suffix);
> > >> +void del_boot_device_path(int32_t bootindex, DeviceState *dev,
> > >> + const char *suffix);
> > >> +void modify_boot_device_path(int32_t bootindex, DeviceState *dev,
> > >> + const char *suffix);
> > >> char *get_boot_devices_list(size_t *size, bool ignore_suffixes);
> > >>
> > >> DeviceState *get_boot_device(uint32_t position);
> > >> diff --git a/vl.c b/vl.c
> > >> index a1686ef..6264e11 100644
> > >> --- a/vl.c
> > >> +++ b/vl.c
> > >> @@ -1247,6 +1247,61 @@ void add_boot_device_path(int32_t bootindex,
> > DeviceState *dev,
> > >> QTAILQ_INSERT_TAIL(&fw_boot_order, node, link);
> > >> }
> > >>
> > >> +static bool is_same_fw_dev_path(DeviceState *src, DeviceState *target)
> > >> +{
> > >> +bool ret = false;
> > >> +char *devpath_src = qdev_get_fw_dev_path(src);
> > >> +char *devpath_target = qdev_get_fw_dev_path(target);
> > >> +
> > >> +if (!strcmp(devpath_src, devpath_target)) {
> > >> +ret = true;
> > >> +}
> > >> +
> > >> +g_free(devpath_src);
> > >> +g_free(devpath_target);
> > >> +return ret;
> > >> +}
> > >> +
> > >> +void del_boot_device_path(int32_t bootindex, DeviceState *dev,
> > >> + const char *suffix)
> > >> +{
> > >> +FWBootEntry *i;
> > >> +
> > >> +assert(dev != NULL);
> > >> +
> > >
> > > assert(booindex >= 0 || suffix != NULL);
> > >
>
> suffix call be NULL.
>
> > >> +QTAILQ_FOREACH(i, &fw_boot_order, link) {
> > >> +if (is_same_fw_dev_path(i->dev, dev)) {
> > >
> > >if (!suffix) {
> > >break;
> > >}
If suffix is NULL, at least we should do nothing in the loop.
> > >
> > >> +if (suffix && i->suffix && strcmp(i->suffix, suffix)) {
> > >> +continue;
> > >> +}
How about this one?
if (!suffix) {
break;
} else (i->suffix && strcmp(i->
Re: [Qemu-devel] [RFC PATCH 1/5] bootindex: add *_boot_device_path function
On Mon, Jul 07, 2014 at 05:10:57PM +0800, arei.gong...@huawei.com wrote:
> From: Chenliang
>
> Add del_boot_device_path and modify_boot_device_path. Device should
> be removed from boot device list by del_boot_device_path when device
> hotplug. modify_boot_device_path is used to modify deviceboot order.
s/hotplug/is unhotplugged/
same issue in commitlog of patch 3/5
> Signed-off-by: Chenliang
> Signed-off-by: Gonglei
> ---
> include/sysemu/sysemu.h | 4
> vl.c| 55
> +
> 2 files changed, 59 insertions(+)
>
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 285c45b..38ef1cd 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -204,6 +204,10 @@ void usb_info(Monitor *mon, const QDict *qdict);
>
> void add_boot_device_path(int32_t bootindex, DeviceState *dev,
>const char *suffix);
> +void del_boot_device_path(int32_t bootindex, DeviceState *dev,
> + const char *suffix);
> +void modify_boot_device_path(int32_t bootindex, DeviceState *dev,
> + const char *suffix);
> char *get_boot_devices_list(size_t *size, bool ignore_suffixes);
>
> DeviceState *get_boot_device(uint32_t position);
> diff --git a/vl.c b/vl.c
> index a1686ef..6264e11 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1247,6 +1247,61 @@ void add_boot_device_path(int32_t bootindex,
> DeviceState *dev,
> QTAILQ_INSERT_TAIL(&fw_boot_order, node, link);
> }
>
> +static bool is_same_fw_dev_path(DeviceState *src, DeviceState *target)
> +{
> +bool ret = false;
> +char *devpath_src = qdev_get_fw_dev_path(src);
> +char *devpath_target = qdev_get_fw_dev_path(target);
> +
> +if (!strcmp(devpath_src, devpath_target)) {
> +ret = true;
> +}
> +
> +g_free(devpath_src);
> +g_free(devpath_target);
> +return ret;
> +}
> +
> +void del_boot_device_path(int32_t bootindex, DeviceState *dev,
> + const char *suffix)
> +{
> +FWBootEntry *i;
> +
> +assert(dev != NULL);
> +
assert(booindex >= 0 || suffix != NULL);
> +QTAILQ_FOREACH(i, &fw_boot_order, link) {
> +if (is_same_fw_dev_path(i->dev, dev)) {
if (!suffix) {
break;
}
> +if (suffix && i->suffix && strcmp(i->suffix, suffix)) {
> +continue;
> +}
If suffix is NULL, then all the entries will be removed?
> +QTAILQ_REMOVE(&fw_boot_order, i, link);
> +g_free(i->suffix);
> +g_free(i);
> +break;
> +}
> +}
> +
> +if (bootindex == -1) {
if (bootindex < 0) {
> +return;
> +}
> +
> +QTAILQ_FOREACH(i, &fw_boot_order, link) {
> +if (i->bootindex == bootindex) {
> +QTAILQ_REMOVE(&fw_boot_order, i, link);
> +g_free(i->suffix);
> +g_free(i);
> +break;
> +}
> +}
> +}
> +
> +void modify_boot_device_path(int32_t bootindex, DeviceState *dev,
> + const char *suffix)
> +{
> +del_boot_device_path(bootindex, dev, suffix);
> +add_boot_device_path(bootindex, dev, suffix);
Why do you directly modify existed entry?
> +}
> +
> DeviceState *get_boot_device(uint32_t position)
> {
> uint32_t counter = 0;
> --
> 1.7.12.4
>
--
Amos.
Re: [Qemu-devel] [PATCH v6 0/4] test virtio-blk hotplug
On Wed, Jun 25, 2014 at 09:15:12AM +0800, Amos Kong wrote: > It's worth to add a hotplug test to qtest, but without > cooperation of guest OS, new devices can't be initialized > by guest, and hot-unplug doesn't work. > > However, the new test can cover some part of code of > hotplug/unplug. > > I will write another subtest to test hotplug with pci support. > > V2: move qmp_exec_hmp_cmd() to libqtest.c > excape hmp cmd (stefanha) > use qmp_exec_hmp_cmd() in blockdev-test > V3: use vp_list to format string, free escaped string > V4: free escaped string by g_free() > V5: escape cmd string in QMP command (so this patchset depends on > http://article.gmane.org/gmane.comp.emulators.qemu/279835) > directly use QMP cmd to add/del device and blockdev > cleanup as suggested by Andreas > V6: hot-unplug devices of one slot once > escaping string fix in 1st patch depends on > (https://lists.nongnu.org/archive/html/qemu-devel/2014-06/msg03024.html) Kevin, Stefan, Andreas Can you help to review & apply this set? Thanks. Amos > Amos Kong (4): > qtest: introduce qmp_exec_hmp_cmd() > qtest: use qmp_exec_hmp_cmd() in blockdev-test > virtio-blk-test: change pci_nop() to virtblk_init() > virtio-blk-test: add hotplug subtest > > tests/blockdev-test.c | 23 ++ > tests/libqtest.c| 23 ++ > tests/libqtest.h| 9 ++ > tests/virtio-blk-test.c | 81 > - > 4 files changed, 108 insertions(+), 28 deletions(-) > > -- > 1.9.3
Re: [Qemu-devel] [PATCH v4 0/4] tests: add usb host adapter qtest cases
On Mon, Jun 23, 2014 at 07:53:50PM +0800, arei.gong...@huawei.com wrote:
> From: Gonglei
>
> This patch series base on my request usb host adapter hotplug/unplug
> series:
>
> [PATCH v3 00/10] usb: usb host adapter hotplug
> http://lists.gnu.org/archive/html/qemu-devel/2014-06/msg00746.html
Looks good to me.
Reviewed-by: Amos Kong
> Changes since v3:
> - rebase on latest qemu master
>
> Changes since v2:
> - add 'const' qualifier argument in usb_hcd_hotplug
> function for building errors reported by Gerd
>
> Changes since v1:
> * By Andreas suggestion:
> - rework hci-{ohci,uhci,xhci} files for functional tests
> - add "cold-plug" test, "-device" in init funciton
> - use QMP for device_add/del because of parsing security
> - some other fixs
>
> * By Gerd suggestion:
> - add an usb-hcd-hotplug-test.c file and collect hotplug
> testing for all usb hcds
>
> Thanks for your suggestion, please review again.
>
> My "make check V=1" results:
>
> [...]
> TEST: tests/usb-hcd-ohci-test... (pid=20031)
> /x86_64/ohci/pci/init: OK
> PASS: tests/usb-hcd-ohci-test
> TEST: tests/usb-hcd-uhci-test... (pid=20034)
> /x86_64/uhci/pci/init: OK
> PASS: tests/usb-hcd-uhci-test
> TEST: tests/usb-hcd-ehci-test... (pid=20037)
> /x86_64/ehci/pci/init: OK
> /x86_64/ehci/pci/uhci-port-1:OK
> /x86_64/ehci/pci/ehci-port-1:OK
> /x86_64/ehci/pci/ehci-config:OK
> /x86_64/ehci/pci/uhci-port-2:OK
> /x86_64/ehci/pci/ehci-port-2:OK
> PASS: tests/usb-hcd-ehci-test
> TEST: tests/usb-hcd-xhci-test... (pid=20040)
> /x86_64/xhci/pci/init: OK
> PASS: tests/usb-hcd-xhci-test
> TEST: tests/usb-hcd-hotplug-test... (pid=20043)
> /x86_64/usb/hcd/pci/ohci-hotplug:OK
> /x86_64/usb/hcd/pci/uhci-hotplug:OK
> /x86_64/usb/hcd/pci/ehci-hotplug:OK
> /x86_64/usb/hcd/pci/xhci-hotplug:OK
> PASS: tests/usb-hcd-hotplug-test
> TEST: tests/qom-test... (pid=20068)
> /x86_64/qom/pc-i440fx-1.4: OK
> /x86_64/qom/pc-1.3: OK
> /x86_64/qom/pc-0.11: OK
> [...]
>
> Best regards,
> -Gonglei
>
> Gonglei (4):
> tests: add OHCI qtest
> tests: add UHCI qtest
> tests: add xHCI qtest
> tests: add usb hcds hotplugging qtest
>
> MAINTAINERS | 2 +-
> tests/Makefile | 12 +-
> tests/usb-hcd-hotplug-test.c | 88
>
> tests/usb-hcd-ohci-test.c| 35 ++
> tests/usb-hcd-uhci-test.c| 35 ++
> tests/usb-hcd-xhci-test.c| 35 ++
> 6 files changed, 205 insertions(+), 2 deletions(-)
> create mode 100644 tests/usb-hcd-hotplug-test.c
> create mode 100644 tests/usb-hcd-ohci-test.c
> create mode 100644 tests/usb-hcd-uhci-test.c
> create mode 100644 tests/usb-hcd-xhci-test.c
>
> --
> 1.7.12.4
>
--
Amos.
Re: [Qemu-devel] [PATCH v4 4/4] tests: add usb hcds hotplugging qtest
On Mon, Jun 23, 2014 at 07:53:54PM +0800, arei.gong...@huawei.com wrote:
> From: Gonglei
>
> Because of we now support usb hcds hotplugging, and
> collect all hcds hotplugging tests into one file
> for code sharing.
>
> Signed-off-by: Gonglei
> ---
> tests/Makefile | 2 +
> tests/usb-hcd-hotplug-test.c | 88
>
> 2 files changed, 90 insertions(+)
> create mode 100644 tests/usb-hcd-hotplug-test.c
>
> diff --git a/tests/Makefile b/tests/Makefile
> index 6a78ade..f0362f7 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -161,6 +161,7 @@ gcov-files-i386-y += hw/usb/dev-hid.c
> gcov-files-i386-y += hw/usb/dev-storage.c
> check-qtest-i386-y += tests/usb-hcd-xhci-test$(EXESUF)
> gcov-files-i386-y += hw/usb/hcd-xhci.c
> +check-qtest-i386-y += tests/usb-hcd-hotplug-test$(EXESUF)
> #check-qtest-i386-y += tests/vhost-user-test$(EXESUF)
> check-qtest-x86_64-y = $(check-qtest-i386-y)
> gcov-files-i386-y += i386-softmmu/hw/timer/mc146818rtc.c
> @@ -331,6 +332,7 @@ tests/usb-hcd-ohci-test$(EXESUF):
> tests/usb-hcd-ohci-test.o
> tests/usb-hcd-uhci-test$(EXESUF): tests/usb-hcd-uhci-test.o
> tests/usb-hcd-ehci-test$(EXESUF): tests/usb-hcd-ehci-test.o
> $(libqos-pc-obj-y)
> tests/usb-hcd-xhci-test$(EXESUF): tests/usb-hcd-xhci-test.o
> +tests/usb-hcd-hotplug-test$(EXESUF): tests/usb-hcd-hotplug-test.o
> tests/vhost-user-test$(EXESUF): tests/vhost-user-test.o qemu-char.o
> qemu-timer.o libqemuutil.a libqemustub.a
> tests/qemu-iotests/socket_scm_helper$(EXESUF):
> tests/qemu-iotests/socket_scm_helper.o
> tests/test-qemu-opts$(EXESUF): tests/test-qemu-opts.o libqemuutil.a
> libqemustub.a
> diff --git a/tests/usb-hcd-hotplug-test.c b/tests/usb-hcd-hotplug-test.c
> new file mode 100644
> index 000..04bea97
> --- /dev/null
> +++ b/tests/usb-hcd-hotplug-test.c
> @@ -0,0 +1,88 @@
> +/*
> + * QTest testcase for usb host adapters hotplug/unplug
> + *
> + * Copyright (c) 2014 HUAWEI TECHNOLOGIES CO.,LTD.
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include
> +#include
> +#include "libqtest.h"
> +#include "qemu/osdep.h"
> +
> +
> +static void usb_hcd_hotplug(const char *hcd, const char *id)
> +{
> +QDict *response;
> +
> +qtest_start("");
> +
> +/* hotplug an usb host adapter */
> +response = qmp("{\"execute\": \"device_add\","
> + " \"arguments\": {"
> + " \"driver\": \"%s\","
> + " \"id\": \"%s\""
> + "}}", hcd, id);
> +g_assert(response);
> +g_assert(!qdict_haskey(response, "error"));
> +QDECREF(response);
> +
> +/* hotplug an usb-tablet to the usb host adapter, bus=$id.0 */
> +response = qmp("{\"execute\": \"device_add\","
> + " \"arguments\": {"
> + " \"driver\": \"usb-tablet\","
> + " \"bus\": \"%s.0\""
> + "}}", id);
> +g_assert(response);
> +g_assert(!qdict_haskey(response, "error"));
> +QDECREF(response);
Do you need to unhot-plug usb-tablet first, before unhot-plug usb host
adapter?
> +/* delete the usb host adapter */
> +response = qmp("{\"execute\": \"device_del\","
> + " \"arguments\": {"
> + " \"id\": \"%s\""
> + "}}", id);
> +g_assert(response);
> +g_assert(!qdict_haskey(response, "error"));
> +QDECREF(response);
> +
> +qtest_end();
> +}
> +
> +static void test_ohci_hotplug(void)
> +{
> +usb_hcd_hotplug("pci-ohci", "ohci");
> +}
> +
> +static void test_uhci_hotplug(void)
> +{
> +usb_hcd_hotplug("piix3-usb-uhci", "uhci");
> +}
> +
> +static void test_ehci_hotplug(void)
> +{
> +usb_hcd_hotplug("usb-ehci", "ehci");
> +}
> +
> +static void test_xhci_hotplug(void)
> +{
> +usb_hcd_hotplug("nec-usb-xhci", "xhci");
> +}
> +
> +int main(int argc, char **argv)
> +{
> +int ret;
> +
> +g_test_init(&argc, &argv, NULL);
> +
> +qtest_add_func("/usb/hcd/pci/ohci-hotplug", test_ohci_hotplug);
> +qtest_add_func("/usb/hcd/pci/uhci-hotplug", test_uhci_hotplug);
> +qtest_add_func("/usb/hcd/pci/ehci-hotplug", test_ehci_hotplug);
> +qtest_add_func("/usb/hcd/pci/xhci-hotplug", test_xhci_hotplug);
> +
> +ret = g_test_run();
> +
> +return ret;
> +}
> --
> 1.7.12.4
>
--
Amos.
[Qemu-devel] [PATCH v6 4/4] virtio-blk-test: add hotplug subtest
This patch adds a new subtest, it hotplugs 29 * 8 = 232 virtio-blk
devices to guest, and try to hot-unplug them.
Note: the hot-unplug can't work without cooperation of guest OS.
Signed-off-by: Amos Kong
---
tests/virtio-blk-test.c | 68 +
1 file changed, 68 insertions(+)
diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index 0fdec01..7cb55cd 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -7,11 +7,78 @@
* See the COPYING file in the top-level directory.
*/
+#include
#include
#include
#include "libqtest.h"
#include "qemu/osdep.h"
+
+/* Start guest without net/block device, slots 3 to 0x1f are free */
+#define MIN_PCI_SLOT 3
+#define MAX_PCI_SLOT 0x1f
+#define MAX_PCI_FUNC 7
+
+static void test_blk_hotplug(void)
+{
+QDict *response;
+int i, j;
+char drive_id[10];
+char device_id[10];
+char addr[10];
+
+qtest_start("-net none");
+
+for (i = MIN_PCI_SLOT; i <= MAX_PCI_SLOT; i++) {
+for (j = MAX_PCI_FUNC; j >= 0; j--) {
+sprintf(drive_id, "drv-%x.%x", i, j);
+response = qmp("{ 'execute': 'blockdev-add',"
+ " 'arguments': {"
+ " 'options': {"
+ " 'driver': 'file',"
+ " 'filename': '/dev/null',"
+ " 'id': %s"
+ "} } }", drive_id);
+g_assert(response);
+g_assert(!qdict_haskey(response, "error"));
+QDECREF(response);
+
+sprintf(device_id, "dev-%x.%x", i, j);
+sprintf(addr, "0x%x.%x", i, j);
+response = qmp("{'execute': 'device_add',"
+ " 'arguments': {"
+ " 'driver': 'virtio-blk-pci',"
+ " 'drive': %s,"
+ " 'id': %s,"
+ " 'addr': %s,"
+ " 'multifunction': 'on'"
+ "} }", drive_id, device_id, addr);
+g_assert(response);
+g_assert(!qdict_haskey(response, "error"));
+QDECREF(response);
+}
+}
+
+/* hot-unplug doesn't work without cooperation of guest OS */
+for (i = MIN_PCI_SLOT; i <= MAX_PCI_SLOT; i++) {
+for (j = MAX_PCI_FUNC; j >= 0; j--) {
+qmp_exec_hmp_cmd("", "drive_del drv-%x.%x", i, j);
+if (j == 0) {
+sprintf(device_id, "dev-%x.%x", i, 0);
+response = qmp("{'execute': 'device_del',"
+ " 'arguments': {"
+ " 'id': %s"
+ "} }", device_id);
+g_assert(response);
+g_assert(!qdict_haskey(response, "error"));
+QDECREF(response);
+}
+}
+}
+
+qtest_end();
+}
+
/* Tests only initialization */
static void virtblk_init(void)
{
@@ -26,6 +93,7 @@ int main(int argc, char **argv)
g_test_init(&argc, &argv, NULL);
qtest_add_func("/virtio/blk/pci/init", virtblk_init);
+qtest_add_func("/virtio/blk/pci/hotplug", test_blk_hotplug);
ret = g_test_run();
--
1.9.3
[Qemu-devel] [PATCH v6 3/4] virtio-blk-test: change pci_nop() to virtblk_init()
I want to add a new subtest in virtio-blk-test, it will start
guest without network. The original pci_init() did nothing, but
it's good to reserve a very simple initialization testing.
Signed-off-by: Amos Kong
Reviewed-by: Stefan Hajnoczi
---
tests/virtio-blk-test.c | 13 ++---
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index d53f875..0fdec01 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -12,9 +12,12 @@
#include "libqtest.h"
#include "qemu/osdep.h"
-/* Tests only initialization so far. TODO: Replace with functional tests */
-static void pci_nop(void)
+/* Tests only initialization */
+static void virtblk_init(void)
{
+qtest_start("-drive id=drv0,if=none,file=/dev/null "
+"-device virtio-blk-pci,drive=drv0");
+qtest_end();
}
int main(int argc, char **argv)
@@ -22,13 +25,9 @@ int main(int argc, char **argv)
int ret;
g_test_init(&argc, &argv, NULL);
-qtest_add_func("/virtio/blk/pci/nop", pci_nop);
+qtest_add_func("/virtio/blk/pci/init", virtblk_init);
-qtest_start("-drive id=drv0,if=none,file=/dev/null "
-"-device virtio-blk-pci,drive=drv0");
ret = g_test_run();
-qtest_end();
-
return ret;
}
--
1.9.3
[Qemu-devel] [PATCH v6 0/4] test virtio-blk hotplug
It's worth to add a hotplug test to qtest, but without cooperation of guest OS, new devices can't be initialized by guest, and hot-unplug doesn't work. However, the new test can cover some part of code of hotplug/unplug. I will write another subtest to test hotplug with pci support. V2: move qmp_exec_hmp_cmd() to libqtest.c excape hmp cmd (stefanha) use qmp_exec_hmp_cmd() in blockdev-test V3: use vp_list to format string, free escaped string V4: free escaped string by g_free() V5: escape cmd string in QMP command (so this patchset depends on http://article.gmane.org/gmane.comp.emulators.qemu/279835) directly use QMP cmd to add/del device and blockdev cleanup as suggested by Andreas V6: hot-unplug devices of one slot once escaping string fix in 1st patch depends on (https://lists.nongnu.org/archive/html/qemu-devel/2014-06/msg03024.html) Amos Kong (4): qtest: introduce qmp_exec_hmp_cmd() qtest: use qmp_exec_hmp_cmd() in blockdev-test virtio-blk-test: change pci_nop() to virtblk_init() virtio-blk-test: add hotplug subtest tests/blockdev-test.c | 23 ++ tests/libqtest.c| 23 ++ tests/libqtest.h| 9 ++ tests/virtio-blk-test.c | 81 - 4 files changed, 108 insertions(+), 28 deletions(-) -- 1.9.3
[Qemu-devel] [PATCH v6 1/4] qtest: introduce qmp_exec_hmp_cmd()
This patch wraps a helper function to execute human command by
one QMP command (human-monitor-command). It also checks the return
string.
Signed-off-by: Amos Kong
---
tests/libqtest.c | 23 +++
tests/libqtest.h | 9 +
2 files changed, 32 insertions(+)
diff --git a/tests/libqtest.c b/tests/libqtest.c
index 98e8f4b..80e0024 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -673,3 +673,26 @@ void qmp_discard_response(const char *fmt, ...)
qtest_qmpv_discard_response(global_qtest, fmt, ap);
va_end(ap);
}
+
+void qmp_exec_hmp_cmd(const char *expected_ret, const char *fmt, ...)
+{
+va_list ap;
+char cmd[1024];
+QDict *response;
+const char *response_return;
+
+va_start(ap, fmt);
+vsprintf(cmd, fmt, ap);
+va_end(ap);
+
+response = qmp("{'execute': 'human-monitor-command',"
+ " 'arguments': {"
+ " 'command-line': %s"
+ "}}", cmd);
+
+g_assert(response);
+response_return = qdict_get_try_str(response, "return");
+g_assert(response_return);
+g_assert_cmpstr(response_return, ==, expected_ret);
+QDECREF(response);
+}
diff --git a/tests/libqtest.h b/tests/libqtest.h
index 8f323c7..d2959d3 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -375,6 +375,15 @@ QDict *qmp(const char *fmt, ...);
void qmp_discard_response(const char *fmt, ...);
/**
+ * qmp_exec_hmp_cmd:
+ * @expected_ret: expected return string
+ * @fmt...: HMP command to execute
+ *
+ * Executes HMP command by 'human-monitor-command'.
+ */
+void qmp_exec_hmp_cmd(const char *expected_ret, const char *fmt, ...);
+
+/**
* qmp_receive:
*
* Reads a QMP message from QEMU and returns the response.
--
1.9.3
[Qemu-devel] [PATCH v6 2/4] qtest: use qmp_exec_hmp_cmd() in blockdev-test
Signed-off-by: Amos Kong
Reviewed-by: Stefan Hajnoczi
Reviewed-by: Andreas Färber
---
tests/blockdev-test.c | 23 ++-
1 file changed, 2 insertions(+), 21 deletions(-)
diff --git a/tests/blockdev-test.c b/tests/blockdev-test.c
index c940e00..c9127c0 100644
--- a/tests/blockdev-test.c
+++ b/tests/blockdev-test.c
@@ -16,35 +16,16 @@
static void test_drive_add_empty(void)
{
-QDict *response;
-const char *response_return;
-
/* Start with an empty drive */
qtest_start("-drive if=none,id=drive0");
/* Delete the drive */
-response = qmp("{\"execute\": \"human-monitor-command\","
- " \"arguments\": {"
- " \"command-line\": \"drive_del drive0\""
- "}}");
-g_assert(response);
-response_return = qdict_get_try_str(response, "return");
-g_assert(response_return);
-g_assert(strcmp(response_return, "") == 0);
-QDECREF(response);
+qmp_exec_hmp_cmd("", "drive_del drive0");
/* Ensure re-adding the drive works - there should be no duplicate ID error
* because the old drive must be gone.
*/
-response = qmp("{\"execute\": \"human-monitor-command\","
- " \"arguments\": {"
- " \"command-line\": \"drive_add 0 if=none,id=drive0\""
- "}}");
-g_assert(response);
-response_return = qdict_get_try_str(response, "return");
-g_assert(response_return);
-g_assert(strcmp(response_return, "OK\r\n") == 0);
-QDECREF(response);
+qmp_exec_hmp_cmd("OK\r\n", "drive_add 0 if=none,id=drive0");
qtest_end();
}
--
1.9.3
Re: [Qemu-devel] [PATCH] libqtest: escape strings in QMP commands, fix leak
On Fri, Jun 13, 2014 at 10:15:00AM +0200, Paolo Bonzini wrote:
> libqtest is using g_strdup_printf to format QMP commands, but
> this does not work if the argument strings need to be escaped.
> Instead, use the fancy %-formatting functionality of QObject.
> The only change required in tests is that strings have to be
> formatted as %s, not '%s' or \"%s\". Luckily this usage of
> parameterized QMP commands is not that frequent.
>
> The leak is in socket_sendf. Since we are extracting the send
> loop to a new function, fix it now.
>
> Signed-off-by: Paolo Bonzini
Reviewed-by: Amos Kong
> ---
> tests/fdc-test.c| 2 +-
> tests/libqtest.c| 47 +--
> tests/qom-test.c| 6 +++---
> tests/tmp105-test.c | 4 ++--
> 4 files changed, 43 insertions(+), 16 deletions(-)
>
> diff --git a/tests/fdc-test.c b/tests/fdc-test.c
> index 37096dc..c8e1e7b 100644
> --- a/tests/fdc-test.c
> +++ b/tests/fdc-test.c
> @@ -291,7 +291,7 @@ static void test_media_insert(void)
> /* Insert media in drive. DSKCHK should not be reset until a step pulse
> * is sent. */
> qmp_discard_response("{'execute':'change', 'arguments':{"
> - " 'device':'floppy0', 'target': '%s' }}",
> + " 'device':'floppy0', 'target': %s }}",
> test_image);
> qmp_discard_response(""); /* ignore event
> (FIXME open -> open transition?!) */
> diff --git a/tests/libqtest.c b/tests/libqtest.c
> index 71468ac..98e8f4b 100644
> --- a/tests/libqtest.c
> +++ b/tests/libqtest.c
> @@ -30,8 +30,9 @@
>
> #include "qemu/compiler.h"
> #include "qemu/osdep.h"
> -#include "qapi/qmp/json-streamer.h"
> #include "qapi/qmp/json-parser.h"
> +#include "qapi/qmp/json-streamer.h"
> +#include "qapi/qmp/qjson.h"
>
> #define MAX_IRQ 256
> #define SOCKET_TIMEOUT 5
> @@ -220,19 +221,15 @@ void qtest_quit(QTestState *s)
> g_free(s);
> }
>
> -static void socket_sendf(int fd, const char *fmt, va_list ap)
> +static void socket_send(int fd, const char *buf, size_t size)
> {
> -gchar *str;
> -size_t size, offset;
> -
> -str = g_strdup_vprintf(fmt, ap);
> -size = strlen(str);
> +size_t offset;
>
> offset = 0;
> while (offset < size) {
> ssize_t len;
>
> -len = write(fd, str + offset, size - offset);
> +len = write(fd, buf + offset, size - offset);
> if (len == -1 && errno == EINTR) {
> continue;
> }
> @@ -244,6 +241,15 @@ static void socket_sendf(int fd, const char *fmt,
> va_list ap)
> }
> }
>
> +static void socket_sendf(int fd, const char *fmt, va_list ap)
> +{
> +gchar *str = g_strdup_vprintf(fmt, ap);
> +size_t size = strlen(str);
> +
> +socket_send(fd, str, size);
> +g_free(str);
> +}
> +
> static void GCC_FMT_ATTR(2, 3) qtest_sendf(QTestState *s, const char *fmt,
> ...)
> {
> va_list ap;
> @@ -378,8 +384,29 @@ QDict *qtest_qmp_receive(QTestState *s)
>
> QDict *qtest_qmpv(QTestState *s, const char *fmt, va_list ap)
> {
> -/* Send QMP request */
> -socket_sendf(s->qmp_fd, fmt, ap);
> +va_list ap_copy;
> +QObject *qobj;
> +
> +/* Going through qobject ensures we escape strings properly.
> + * This seemingly unnecessary copy is required in case va_list
> + * is an array type.
> + */
> +va_copy(ap_copy, ap);
> +qobj = qobject_from_jsonv(fmt, &ap_copy);
> +va_end(ap_copy);
> +
> +/* No need to send anything for an empty QObject. */
> +if (qobj) {
> +QString *qstr = qobject_to_json(qobj);
> +const char *str = qstring_get_str(qstr);
> +size_t size = qstring_get_length(qstr);
> +
> +/* Send QMP request */
> +socket_send(s->qmp_fd, str, size);
> +
> +QDECREF(qstr);
> +qobject_decref(qobj);
> +}
>
> /* Receive reply */
> return qtest_qmp_receive(s);
> diff --git a/tests/qom-test.c b/tests/qom-test.c
> index d8d1d8d..4246382 100644
> --- a/tests/qom-test.c
> +++ b/tests/qom-test.c
> @@ -53,7 +53,7 @@ static void test_properties(const char *path, bool recurse)
>
> g_test_message("Obtaining properties of %s", path);
> response = qmp("{ 'execute': 'qo
Re: [Qemu-devel] [PATCH v5 3/4] virtio-blk-test.c: change pci_nop() to virtblk_init()
On Wed, Jun 18, 2014 at 06:43:49PM +0200, Andreas Färber wrote:
> Am 18.06.2014 18:24, schrieb Amos Kong:
> > I want to add a new subtest in virtio-blk-test.c, it will start
> > guest without network. The original pci_init() did nothing, but
> > it's good to reserve a very simple initialization testing.
> >
> > Signed-off-by: Amos Kong
> > Reviewed-by: Stefan Hajnoczi
> > ---
> > tests/virtio-blk-test.c | 13 ++---
> > 1 file changed, 6 insertions(+), 7 deletions(-)
> >
> > diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
> > index d53f875..0fdec01 100644
> > --- a/tests/virtio-blk-test.c
> > +++ b/tests/virtio-blk-test.c
> > @@ -12,9 +12,12 @@
> > #include "libqtest.h"
> > #include "qemu/osdep.h"
> >
> > -/* Tests only initialization so far. TODO: Replace with functional tests */
>
> One thing of note here is that this TODO is neither resolved here nor
> later in this series. Stefan had originally asked me to add it.
We don't treated pci_nop() as test in the past, but in this patch we
rename and split it as a single initialization testing.
We also added another functional test by next patchset.
So we can remove the 'TODO'
> Andreas
>
> > -static void pci_nop(void)
> > +/* Tests only initialization */
> > +static void virtblk_init(void)
> > {
> > +qtest_start("-drive id=drv0,if=none,file=/dev/null "
> > +"-device virtio-blk-pci,drive=drv0");
> > +qtest_end();
> > }
> >
> > int main(int argc, char **argv)
> [snip]
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
--
Amos.
Re: [Qemu-devel] [PATCH v5 4/4] virtio-blk-test.c: add hotplug subtest
On Thu, Jun 19, 2014 at 04:03:28AM +, Gonglei (Arei) wrote:
> Hi,
>
> > -Original Message-
> > From: Amos Kong [mailto:ak...@redhat.com]
> > Sent: Thursday, June 19, 2014 11:58 AM
> > To: Stefan Hajnoczi
> > Cc: qemu-devel@nongnu.org; Gonglei (Arei); afaer...@suse.de;
> > pbonz...@redhat.com; kw...@redhat.com
> > Subject: Re: [PATCH v5 4/4] virtio-blk-test.c: add hotplug subtest
> >
> > On Thu, Jun 19, 2014 at 11:49:08AM +0800, Stefan Hajnoczi wrote:
> > > On Thu, Jun 19, 2014 at 12:24:13AM +0800, Amos Kong wrote:
> > > > +for (i = MIN_PCI_SLOT; i <= MAX_PCI_SLOT; i++) {
> > > > +for (j = MAX_PCI_FUNC; j >= 0; j--) {
> > > > +if (j == MAX_PCI_FUNC) {
> > > > +qmp_exec_hmp_cmd("", "drive_del drv-%x.%x", i, 0);
> > > > +}
> > >
> > > Why is only drv-%x.0 deleted? Previous revisions of this patch series
> > > deleted all drives.
> >
> > Hot-unplug any single function, all functions in the slot will be
> > removed. So once is enough.
> >
> IMHO, the below devices as the same as the drive about multifunction
> hot-unplug, right?
I'm wrong. drive should be hot-unplugged one by one, device will be removed by
slot. PCI device has multifunction feature, not drive.
> Best regards,
> -Gonglei
--
Amos.
Re: [Qemu-devel] [PATCH v5 4/4] virtio-blk-test.c: add hotplug subtest
On Thu, Jun 19, 2014 at 11:49:08AM +0800, Stefan Hajnoczi wrote:
> On Thu, Jun 19, 2014 at 12:24:13AM +0800, Amos Kong wrote:
> > +for (i = MIN_PCI_SLOT; i <= MAX_PCI_SLOT; i++) {
> > +for (j = MAX_PCI_FUNC; j >= 0; j--) {
> > +if (j == MAX_PCI_FUNC) {
> > +qmp_exec_hmp_cmd("", "drive_del drv-%x.%x", i, 0);
> > +}
>
> Why is only drv-%x.0 deleted? Previous revisions of this patch series
> deleted all drives.
Hot-unplug any single function, all functions in the slot will be
removed. So once is enough.
> Stefan
--
Amos.
pgpNzez1ha03U.pgp
Description: PGP signature
[Qemu-devel] [PATCH v5 2/4] qtest: use qmp_exec_hmp_cmd() in blockdev-test
Signed-off-by: Amos Kong
Reviewed-by: Stefan Hajnoczi
Reviewed-by: Andreas Färber
---
tests/blockdev-test.c | 23 ++-
1 file changed, 2 insertions(+), 21 deletions(-)
diff --git a/tests/blockdev-test.c b/tests/blockdev-test.c
index c940e00..c9127c0 100644
--- a/tests/blockdev-test.c
+++ b/tests/blockdev-test.c
@@ -16,35 +16,16 @@
static void test_drive_add_empty(void)
{
-QDict *response;
-const char *response_return;
-
/* Start with an empty drive */
qtest_start("-drive if=none,id=drive0");
/* Delete the drive */
-response = qmp("{\"execute\": \"human-monitor-command\","
- " \"arguments\": {"
- " \"command-line\": \"drive_del drive0\""
- "}}");
-g_assert(response);
-response_return = qdict_get_try_str(response, "return");
-g_assert(response_return);
-g_assert(strcmp(response_return, "") == 0);
-QDECREF(response);
+qmp_exec_hmp_cmd("", "drive_del drive0");
/* Ensure re-adding the drive works - there should be no duplicate ID error
* because the old drive must be gone.
*/
-response = qmp("{\"execute\": \"human-monitor-command\","
- " \"arguments\": {"
- " \"command-line\": \"drive_add 0 if=none,id=drive0\""
- "}}");
-g_assert(response);
-response_return = qdict_get_try_str(response, "return");
-g_assert(response_return);
-g_assert(strcmp(response_return, "OK\r\n") == 0);
-QDECREF(response);
+qmp_exec_hmp_cmd("OK\r\n", "drive_add 0 if=none,id=drive0");
qtest_end();
}
--
1.9.3
[Qemu-devel] [PATCH v5 0/4] test virtio-blk hotplug
It's worth to add a hotplug test to qtest, but without cooperation of guest OS, new devices can't be initialized by guest, and hot-unplug doesn't work. However, the new test can cover some part of code of hotplug/unplug. I will write another subtest to test hotplug with pci support. V2: move qmp_exec_hmp_cmd() to libqtest.c excape hmp cmd (stefanha) use qmp_exec_hmp_cmd() in blockdev-test V3: use vp_list to format string, free escaped string V4: free escaped string by g_free() V5: escape cmd string in QMP command (so this patchset depends on http://article.gmane.org/gmane.comp.emulators.qemu/279835) directly use QMP cmd to add/del device and blockdev cleanup as suggested by Andreas Amos Kong (4): qtest: introduce qmp_exec_hmp_cmd() qtest: use qmp_exec_hmp_cmd() in blockdev-test virtio-blk-test.c: change pci_nop() to virtblk_init() virtio-blk-test.c: add hotplug subtest tests/blockdev-test.c | 23 ++ tests/libqtest.c| 23 ++ tests/libqtest.h| 9 ++ tests/virtio-blk-test.c | 81 - 4 files changed, 108 insertions(+), 28 deletions(-) -- 1.9.3
[Qemu-devel] [PATCH v5 3/4] virtio-blk-test.c: change pci_nop() to virtblk_init()
I want to add a new subtest in virtio-blk-test.c, it will start
guest without network. The original pci_init() did nothing, but
it's good to reserve a very simple initialization testing.
Signed-off-by: Amos Kong
Reviewed-by: Stefan Hajnoczi
---
tests/virtio-blk-test.c | 13 ++---
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index d53f875..0fdec01 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -12,9 +12,12 @@
#include "libqtest.h"
#include "qemu/osdep.h"
-/* Tests only initialization so far. TODO: Replace with functional tests */
-static void pci_nop(void)
+/* Tests only initialization */
+static void virtblk_init(void)
{
+qtest_start("-drive id=drv0,if=none,file=/dev/null "
+"-device virtio-blk-pci,drive=drv0");
+qtest_end();
}
int main(int argc, char **argv)
@@ -22,13 +25,9 @@ int main(int argc, char **argv)
int ret;
g_test_init(&argc, &argv, NULL);
-qtest_add_func("/virtio/blk/pci/nop", pci_nop);
+qtest_add_func("/virtio/blk/pci/init", virtblk_init);
-qtest_start("-drive id=drv0,if=none,file=/dev/null "
-"-device virtio-blk-pci,drive=drv0");
ret = g_test_run();
-qtest_end();
-
return ret;
}
--
1.9.3
[Qemu-devel] [PATCH v5 1/4] qtest: introduce qmp_exec_hmp_cmd()
This patch wraps a helper function to execute human command by
one QMP command (human-monitor-command). It also checks the return
string.
Signed-off-by: Amos Kong
---
tests/libqtest.c | 23 +++
tests/libqtest.h | 9 +
2 files changed, 32 insertions(+)
diff --git a/tests/libqtest.c b/tests/libqtest.c
index 98e8f4b..80e0024 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -673,3 +673,26 @@ void qmp_discard_response(const char *fmt, ...)
qtest_qmpv_discard_response(global_qtest, fmt, ap);
va_end(ap);
}
+
+void qmp_exec_hmp_cmd(const char *expected_ret, const char *fmt, ...)
+{
+va_list ap;
+char cmd[1024];
+QDict *response;
+const char *response_return;
+
+va_start(ap, fmt);
+vsprintf(cmd, fmt, ap);
+va_end(ap);
+
+response = qmp("{'execute': 'human-monitor-command',"
+ " 'arguments': {"
+ " 'command-line': %s"
+ "}}", cmd);
+
+g_assert(response);
+response_return = qdict_get_try_str(response, "return");
+g_assert(response_return);
+g_assert_cmpstr(response_return, ==, expected_ret);
+QDECREF(response);
+}
diff --git a/tests/libqtest.h b/tests/libqtest.h
index 8f323c7..d2959d3 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -375,6 +375,15 @@ QDict *qmp(const char *fmt, ...);
void qmp_discard_response(const char *fmt, ...);
/**
+ * qmp_exec_hmp_cmd:
+ * @expected_ret: expected return string
+ * @fmt...: HMP command to execute
+ *
+ * Executes HMP command by 'human-monitor-command'.
+ */
+void qmp_exec_hmp_cmd(const char *expected_ret, const char *fmt, ...);
+
+/**
* qmp_receive:
*
* Reads a QMP message from QEMU and returns the response.
--
1.9.3
[Qemu-devel] [PATCH v5 4/4] virtio-blk-test.c: add hotplug subtest
This patch adds a new subtest, it hotplugs 29 * 8 = 232 virtio-blk
devices to guest, and try to hot-unplug them.
Note: the hot-unplug can't work without cooperation of guest OS.
Signed-off-by: Amos Kong
---
tests/virtio-blk-test.c | 68 +
1 file changed, 68 insertions(+)
diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index 0fdec01..77817ca 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -7,11 +7,78 @@
* See the COPYING file in the top-level directory.
*/
+#include
#include
#include
#include "libqtest.h"
#include "qemu/osdep.h"
+
+/* start with no network/block device, slots 3 to 0x1f are free */
+#define MIN_PCI_SLOT 3
+#define MAX_PCI_SLOT 0x1f
+#define MAX_PCI_FUNC 7
+
+static void test_blk_hotplug(void)
+{
+QDict *response;
+int i, j;
+char drive_id[10];
+char device_id[10];
+char addr[10];
+
+qtest_start("-net none");
+
+for (i = MIN_PCI_SLOT; i <= MAX_PCI_SLOT; i++) {
+for (j = MAX_PCI_FUNC; j >= 0; j--) {
+sprintf(drive_id, "drv-%x.%x", i, j);
+response = qmp("{ 'execute': 'blockdev-add',"
+ " 'arguments': {"
+ " 'options': {"
+ " 'driver': 'file',"
+ " 'filename': '/dev/null',"
+ " 'id': %s"
+ "} } }", drive_id);
+g_assert(response);
+g_assert(!qdict_haskey(response, "error"));
+QDECREF(response);
+
+sprintf(device_id, "dev-%x.%x", i, j);
+sprintf(addr, "0x%x.%x", i, j);
+response = qmp("{'execute': 'device_add',"
+ " 'arguments': {"
+ " 'driver': 'virtio-blk-pci',"
+ " 'drive': %s,"
+ " 'id': %s,"
+ " 'addr': %s,"
+ " 'multifunction': 'on'"
+ "} }", drive_id, device_id, addr);
+g_assert(response);
+g_assert(!qdict_haskey(response, "error"));
+QDECREF(response);
+}
+}
+
+/* hot-unplug doesn't work without cooperation of guest OS */
+for (i = MIN_PCI_SLOT; i <= MAX_PCI_SLOT; i++) {
+for (j = MAX_PCI_FUNC; j >= 0; j--) {
+if (j == MAX_PCI_FUNC) {
+qmp_exec_hmp_cmd("", "drive_del drv-%x.%x", i, 0);
+}
+sprintf(device_id, "dev-%x.%x", i, j);
+response = qmp("{'execute': 'device_del',"
+ " 'arguments': {"
+ " 'id': %s"
+ "} }", device_id);
+g_assert(response);
+g_assert(!qdict_haskey(response, "error"));
+QDECREF(response);
+}
+}
+
+qtest_end();
+}
+
/* Tests only initialization */
static void virtblk_init(void)
{
@@ -26,6 +93,7 @@ int main(int argc, char **argv)
g_test_init(&argc, &argv, NULL);
qtest_add_func("/virtio/blk/pci/init", virtblk_init);
+qtest_add_func("/virtio/blk/pci/hotplug", test_blk_hotplug);
ret = g_test_run();
--
1.9.3
Re: [Qemu-devel] [PATCH v6 2/3] qapi: add const prefix to 'char *' insider c_type()
On Tue, Jun 17, 2014 at 09:51:21PM -0600, Eric Blake wrote: > On 06/10/2014 05:25 AM, Amos Kong wrote: > > It's ugly to add const prefix for parameter type by an if statement > > outside c_type(). This patch adds a parameter to do it. > > > > Signed-off-by: Amos Kong > > Suggested-by: Markus Armbruster > > Reviewed-by: Eric Blake > > Reviewed-by: Paolo Bonzini > > Reviewed-by: Markus Armbruster > > --- > > scripts/qapi-commands.py | 4 +--- > > scripts/qapi.py | 4 +++- > > 2 files changed, 4 insertions(+), 4 deletions(-) > > Wenchao's series introduces another client that needs this treatment: > https://lists.gnu.org/archive/html/qemu-devel/2014-06/msg01225.html > > Depending on what order things get merged in, you may need followup > patches or conflict resolution. Thanks for the reminder. I just checked the patch, c_type() is only used once, and the output is used insider mcgen(). So it's safe to apply my patchset. > > diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py > > index 7d93d01..34f200a 100644 > > --- a/scripts/qapi-commands.py > > +++ b/scripts/qapi-commands.py > > @@ -29,9 +29,7 @@ def type_visitor(name): > > def generate_command_decl(name, args, ret_type): > > arglist="" > > for argname, argtype, optional, structured in parse_args(args): > > -argtype = c_type(argtype) > > -if argtype == "char *": > > -argtype = "const char *" > > +argtype = c_type(argtype, is_param=True) > > -- > Eric Blake eblake redhat com+1-919-301-3266 > Libvirt virtualization library http://libvirt.org > -- Amos. pgpO4MiJ8WPkp.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH] libqtest: escape strings in QMP commands, fix leak
- Fixed Andreas's mail address
On Fri, Jun 13, 2014 at 10:15:00AM +0200, Paolo Bonzini wrote:
> libqtest is using g_strdup_printf to format QMP commands, but
> this does not work if the argument strings need to be escaped.
> Instead, use the fancy %-formatting functionality of QObject.
> The only change required in tests is that strings have to be
> formatted as %s, not '%s' or \"%s\". Luckily this usage of
> parameterized QMP commands is not that frequent.
I got this error when I apply this patch (it works without this
patch):
{"error": {"class": "GenericError", "desc": "Parameter 'id' expects an
identifier"}}
Code:
|QDict *response;
|int i, j;
|
|/* start with no network/block device, slots 3 to 0x1f are free */
|qtest_start("-net none");
|
|for (i = 3; i <= 0x1f; i++) {
|for (j = 7; j >= 0; j--) {
|response = qmp("{ 'execute': 'blockdev-add',"
| " 'arguments': {"
| " 'options': {"
| " 'driver': 'file',"
| " 'filename': '/dev/null',"
| " 'id': 'drv-%x.%x'"
^
| "} } }", i, j);
|g_assert(response);
|g_assert(!qdict_haskey(response, "error"));
|QDECREF(response);
Then I have to fix it by :
/* start with no network/block device, slots 3 to 0x1f are free */
qtest_start("-net none");
for (i = 3; i <= 0x1f; i++) {
for (j = 7; j >= 0; j--) {
+sprintf(drive_id, "drv-%x.%x", i, j);
response = qmp("{ 'execute': 'blockdev-add',"
" 'arguments': {"
" 'options': {"
" 'driver': 'file',"
" 'filename': '/dev/null',"
- " 'id': 'drv-%x.%x'"
- "} } }", i, j);
+ " 'id': %s"
+ "} } }", drive_id);
Is it the expected result?
Thanks, Amos
> The leak is in socket_sendf. Since we are extracting the send
> loop to a new function, fix it now.
>
> Signed-off-by: Paolo Bonzini
> ---
> tests/fdc-test.c| 2 +-
> tests/libqtest.c| 47 +--
> tests/qom-test.c| 6 +++---
> tests/tmp105-test.c | 4 ++--
> 4 files changed, 43 insertions(+), 16 deletions(-)
>
> diff --git a/tests/fdc-test.c b/tests/fdc-test.c
> index 37096dc..c8e1e7b 100644
> --- a/tests/fdc-test.c
> +++ b/tests/fdc-test.c
> @@ -291,7 +291,7 @@ static void test_media_insert(void)
> /* Insert media in drive. DSKCHK should not be reset until a step pulse
> * is sent. */
> qmp_discard_response("{'execute':'change', 'arguments':{"
> - " 'device':'floppy0', 'target': '%s' }}",
> + " 'device':'floppy0', 'target': %s }}",
> test_image);
> qmp_discard_response(""); /* ignore event
> (FIXME open -> open transition?!) */
> diff --git a/tests/libqtest.c b/tests/libqtest.c
> index 71468ac..98e8f4b 100644
> --- a/tests/libqtest.c
> +++ b/tests/libqtest.c
> @@ -30,8 +30,9 @@
>
> #include "qemu/compiler.h"
> #include "qemu/osdep.h"
> -#include "qapi/qmp/json-streamer.h"
> #include "qapi/qmp/json-parser.h"
> +#include "qapi/qmp/json-streamer.h"
> +#include "qapi/qmp/qjson.h"
>
> #define MAX_IRQ 256
> #define SOCKET_TIMEOUT 5
> @@ -220,19 +221,15 @@ void qtest_quit(QTestState *s)
> g_free(s);
> }
>
> -static void socket_sendf(int fd, const char *fmt, va_list ap)
> +static void socket_send(int fd, const char *buf, size_t size)
> {
> -gchar *str;
> -size_t size, offset;
> -
> -str = g_strdup_vprintf(fmt, ap);
> -size = strlen(str);
> +size_t offset;
>
> offset = 0;
> while (offset < size) {
> ssize_t len;
>
> -len = write(fd, str + offset, size - offset);
> +len = write(fd, buf + offset, size - offset);
> if (len == -1 && errno == EINTR) {
> continue;
> }
> @@ -244,6 +241,15 @@ static void socket_sendf(int fd, const char *fmt,
> va_list ap)
> }
> }
>
> +static void socket_sendf(int fd, const char *fmt, va_list ap)
> +{
> +gchar *str = g_strdup_vprintf(fmt, ap);
> +size_t size = strlen(str);
> +
> +socket_send(fd, str, size);
> +g_free(str);
> +}
> +
> static void GCC_FMT_ATTR(2, 3) qtest_sendf(QTestState *s, const char *fmt,
> ...)
> {
> va_list ap;
> @@ -378,8 +384,29 @@ QDict *qtest_qmp_receive(QTestState *s)
>
> QDict *qtest_qmpv(QTestState *s, const char *fmt, va_list ap)
> {
> -/* Send QMP request */
> -socket_sendf(s->qmp_fd, fmt, ap);
> +va_list ap_copy;
> +QObject *qobj;
> +
> +/* Going through qobject ensures we escape strings properly.
> + * Thi
Re: [Qemu-devel] [PATCH v4 3/4] virtio-blk-test.c: add hotplug subtest
On Tue, Jun 17, 2014 at 03:25:34PM +0200, Andreas Färber wrote:
> Am 06.06.2014 16:33, schrieb Amos Kong:
> > This patch adds a new subtest, it hotplugs 29 * 8 = 232 virtio-blk
> > devices to guest, and try to hot-unplug them.
> >
> > Note: the hot-unplug can't work without cooperation of guest OS.
> >
> > Signed-off-by: Amos Kong
> > Reviewed-by: Stefan Hajnoczi
> > ---
> > tests/virtio-blk-test.c | 31 +++
> > 1 file changed, 31 insertions(+)
> >
> > diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
> > index 0fdec01..7358203 100644
> > --- a/tests/virtio-blk-test.c
> > +++ b/tests/virtio-blk-test.c
> > @@ -7,11 +7,41 @@
> > * See the COPYING file in the top-level directory.
> > */
> >
> > +#include
> > #include
> > #include
> > #include "libqtest.h"
> > #include "qemu/osdep.h"
> >
> > +static void test_blk_hotplug(void)
> > +{
> > +int i, j;
> > +
> > +/* start with no network/block device, slots 3~0x1f are free */
>
> "3-0x1f" or "3 to 0x1f"?
>
> > +qtest_start("-net none");
> > +
> > +for (i = 3; i <= 0x1f; i++) {
> > +for (j = 7; j >= 0; j--) {
> > +qmp_exec_hmp_cmd("OK\r\n",
> > + "drive_add 0
> > if=none,file=/dev/null,id=drv-%x.%x",
> > + i, j);
> > +qmp_exec_hmp_cmd("",
> > + "device_add
> > virtio-blk-pci,id=dev-%x.%x,drive=drv-%x.%x,"
> > + "addr=0x%x.%x,multifunction=on", i, j, i, j, i, j);
Hi Andreas,
> Why are you using HMP-via-QMP here and not QMP directly?
I referenced existed test code.
> > +}
> > +}
> > +
> > +/* hot-unplug doesn't work without cooperation of guest OS */
> > +for (i = 3; i <= 0x1f; i++) {
> > +for (j = 7; j >= 0; j--) {
>
> While the function is still small, using a define or static const would
> be a small improvement. :) Could be a follow-up of course.
Sorry I don't get it.
test_blk_hotplug() was already decorated by 'static'
and we can't decorate a function that returns nothing.
tests/virtio-blk-test.c:16:19: error: function definition has
qualified void return type [-Werror]
static const void test_blk_hotplug(void)
> Test looks good, thanks for your effort.
>
> Regards,
> Andreas
Thanks, Amos
> > +qmp_exec_hmp_cmd("", "drive_del drv-%x.%x", i, j);
> > +qmp_exec_hmp_cmd("", "device_del dev-%x.%x", i, j);
> > +}
> > +}
> > +
> > +qtest_end();
> > +}
> > +
> > /* Tests only initialization */
> > static void virtblk_init(void)
> > {
> > @@ -26,6 +56,7 @@ int main(int argc, char **argv)
> >
> > g_test_init(&argc, &argv, NULL);
> > qtest_add_func("/virtio/blk/pci/init", virtblk_init);
> > +qtest_add_func("/virtio/blk/pci/hotplug", test_blk_hotplug);
> >
> > ret = g_test_run();
> >
> >
>
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
--
Amos.
Re: [Qemu-devel] [PATCH v4 2/4] qtest: introduce qmp_exec_hmp_cmd()
On Tue, Jun 17, 2014 at 07:37:06PM +0200, Paolo Bonzini wrote:
> Il 06/06/2014 16:33, Amos Kong ha scritto:
> >+va_end(ap);
> >+
> >+escaped_cmd = g_strescape(cmd, NULL);
> >+response = qmp("{\"execute\": \"human-monitor-command\","
> >+ " \"arguments\": {"
> >+ " \"command-line\": \"%s\""
> >+ "}}", escaped_cmd);
> >+g_free(escaped_cmd);
>
> Instead of adding g_strescape everywhere, we should use json-parser's own
> interpolation support. See this patch:
> http://article.gmane.org/gmane.comp.emulators.qemu/279836 which also fixes a
> leak as a bonus.
>
> Also, you can use ' instead of " if you fix another long-standing bug:
> http://article.gmane.org/gmane.comp.emulators.qemu/279835
I will use ' instead of ", and escape string in QMP command as
http://article.gmane.org/gmane.comp.emulators.qemu/279836
Thanks.
> Paolo
--
Amos.
Re: [Qemu-devel] [PATCH] json-lexer: fix escaped backslash in single-quoted string
On Fri, Jun 13, 2014 at 10:13:02AM +0200, Paolo Bonzini wrote:
> This made the lexer wait for a closing *double* quote.
>
> Signed-off-by: Paolo Bonzini
Reviewed-by: Amos Kong
> ---
> qobject/json-lexer.c | 4 ++--
> tests/check-qjson.c | 7 +++
> 2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/qobject/json-lexer.c b/qobject/json-lexer.c
> index 440df60..b19623e 100644
> --- a/qobject/json-lexer.c
> +++ b/qobject/json-lexer.c
> @@ -138,8 +138,8 @@ static const uint8_t json_lexer[][256] = {
> ['n'] = IN_SQ_STRING,
> ['r'] = IN_SQ_STRING,
> ['t'] = IN_SQ_STRING,
> -['/'] = IN_DQ_STRING,
> -['\\'] = IN_DQ_STRING,
> +['/'] = IN_SQ_STRING,
> +['\\'] = IN_SQ_STRING,
> ['\''] = IN_SQ_STRING,
> ['\"'] = IN_SQ_STRING,
> ['u'] = IN_SQ_UCODE0,
> diff --git a/tests/check-qjson.c b/tests/check-qjson.c
> index 4e74548..95497a0 100644
> --- a/tests/check-qjson.c
> +++ b/tests/check-qjson.c
> @@ -45,6 +45,13 @@ static void escaped_string(void)
> { "\"single byte utf-8 \\u0020\"", "single byte utf-8 ", .skip = 1
> },
> { "\"double byte utf-8 \\u00A2\"", "double byte utf-8 \xc2\xa2" },
> { "\"triple byte utf-8 \\u20AC\"", "triple byte utf-8 \xe2\x82\xac"
> },
> +{ "'\\b'", "\b", .skip = 1 },
> +{ "'\\f'", "\f", .skip = 1 },
> +{ "'\\n'", "\n", .skip = 1 },
> +{ "'\\r'", "\r", .skip = 1 },
> +{ "'\\t'", "\t", .skip = 1 },
> +{ "'\\/'", "/", .skip = 1 },
> +{ "''", "\\", .skip = 1 },
> {}
> };
>
> --
> 1.8.3.1
>
--
Amos.
Re: [Qemu-devel] [PATCH v4 0/4] test virtio-blk hotplug
On Wed, Jun 18, 2014 at 10:58:52AM +0800, Amos Kong wrote:
> On Tue, Jun 17, 2014 at 03:37:08PM +0200, Andreas Färber wrote:
> > -BEGIN PGP SIGNED MESSAGE-
> > Hash: SHA1
> >
> > Hi Amos,
> >
> > Am 17.06.2014 14:54, schrieb Amos Kong:
> > > On Mon, Jun 09, 2014 at 03:22:51PM +0200, Stefan Hajnoczi wrote:
> > >> On Fri, Jun 06, 2014 at 10:33:49PM +0800, Amos Kong wrote:
> > >>> It's worth to add a hotplug test to qtest, but without
> > >>> cooperation of guest OS, new devices can't be initialized by
> > >>> guest, and hot-unplug doesn't work.
> > >>>
> > >>> However, the new test can cover some part of code of
> > >>> hotplug/unplug.
> > >>>
> > >>> I will write another subtest to test hotplug with pci support.
> > >>>
> > >>> V2: move qmp_exec_hmp_cmd() to libqtest.c excape hmp cmd
> > >>> (stefanha) use qmp_exec_hmp_cmd() in blockdev-test V3: use
> > >>> vp_list to format string, free escaped string V4: free escaped
> > >>> string by g_free()
> > >
> > > Hi Andreas,
> > >
> > > Can you apply this patchset to your tree?
> >
> > Sorry for the late reply, I was on vacation and am still fighting a
> > mail backlog...
>
> That's OK :)
>
> > I had a question about the new HMP helper function - I see in 4/4 that
> > drive_add was already being done via HMP, so I guess that's the
> > culprit, but still I wonder whether we can do device-add via QMP.
>
> I didn't find device_add/del definition in qapi-schema.json, so I
> used HMP instead. Actually those two QMP commands exist and work.
>
> { "execute": "blockdev-add", "arguments": { "options": { "driver":
> "file", "filename": "/dev/null", "id": "id1" } } }
>
> { "execute": "device_add", "arguments": { "driver": "virtio-blk-pci",
> "drive": "id1", "driver": "virtio-blk-pci", "id": "id2" } }
>
> I will convert virtio-blk-test to use QMP, send a V5.
We have QMP command to hot-unplug device:
{ "execute": "device_del", "arguments": { "id": "id2" } }
But there is not QMP command to hot-unplug drive/blockdev
drive_del / blockdev-del doesn't exist, so I will use
human-monitor-command to hot-unplug blockdev.
> > That said, I've reviewed the generic qtest part OK, and would assume
> > virtio-blk-test (you can drop the .c in the topic please) to go
> > through the same tree virtio-blk does, so via Stefan/Kevin.
>
> Stefan said (offline) it belongs to you, and I found the first commit of
> this file was merged by you.
>
> Thanks.
>
> > Regards,
> > Andreas
>
> --
> Amos.
--
Amos.
Re: [Qemu-devel] [PATCH v4 0/4] test virtio-blk hotplug
On Tue, Jun 17, 2014 at 03:37:08PM +0200, Andreas Färber wrote:
> -BEGIN PGP SIGNED MESSAGE-
> Hash: SHA1
>
> Hi Amos,
>
> Am 17.06.2014 14:54, schrieb Amos Kong:
> > On Mon, Jun 09, 2014 at 03:22:51PM +0200, Stefan Hajnoczi wrote:
> >> On Fri, Jun 06, 2014 at 10:33:49PM +0800, Amos Kong wrote:
> >>> It's worth to add a hotplug test to qtest, but without
> >>> cooperation of guest OS, new devices can't be initialized by
> >>> guest, and hot-unplug doesn't work.
> >>>
> >>> However, the new test can cover some part of code of
> >>> hotplug/unplug.
> >>>
> >>> I will write another subtest to test hotplug with pci support.
> >>>
> >>> V2: move qmp_exec_hmp_cmd() to libqtest.c excape hmp cmd
> >>> (stefanha) use qmp_exec_hmp_cmd() in blockdev-test V3: use
> >>> vp_list to format string, free escaped string V4: free escaped
> >>> string by g_free()
> >
> > Hi Andreas,
> >
> > Can you apply this patchset to your tree?
>
> Sorry for the late reply, I was on vacation and am still fighting a
> mail backlog...
That's OK :)
> I had a question about the new HMP helper function - I see in 4/4 that
> drive_add was already being done via HMP, so I guess that's the
> culprit, but still I wonder whether we can do device-add via QMP.
I didn't find device_add/del definition in qapi-schema.json, so I
used HMP instead. Actually those two QMP commands exist and work.
{ "execute": "blockdev-add", "arguments": { "options": { "driver":
"file", "filename": "/dev/null", "id": "id1" } } }
{ "execute": "device_add", "arguments": { "driver": "virtio-blk-pci",
"drive": "id1", "driver": "virtio-blk-pci", "id": "id2" } }
I will convert virtio-blk-test to use QMP, send a V5.
> That said, I've reviewed the generic qtest part OK, and would assume
> virtio-blk-test (you can drop the .c in the topic please) to go
> through the same tree virtio-blk does, so via Stefan/Kevin.
Stefan said (offline) it belongs to you, and I found the first commit of
this file was merged by you.
Thanks.
> Regards,
> Andreas
--
Amos.
Re: [Qemu-devel] [PATCH v4 0/4] test virtio-blk hotplug
On Mon, Jun 09, 2014 at 03:22:51PM +0200, Stefan Hajnoczi wrote: > On Fri, Jun 06, 2014 at 10:33:49PM +0800, Amos Kong wrote: > > It's worth to add a hotplug test to qtest, but without > > cooperation of guest OS, new devices can't be initialized > > by guest, and hot-unplug doesn't work. > > > > However, the new test can cover some part of code of > > hotplug/unplug. > > > > I will write another subtest to test hotplug with pci support. > > > > V2: move qmp_exec_hmp_cmd() to libqtest.c > > excape hmp cmd (stefanha) > > use qmp_exec_hmp_cmd() in blockdev-test > > V3: use vp_list to format string, free escaped string > > V4: free escaped string by g_free() Hi Andreas, Can you apply this patchset to your tree? > > Amos Kong (4): > > virtio-blk-test.c: change pci_nop() to virtblk_init() > > qtest: introduce qmp_exec_hmp_cmd() > > virtio-blk-test.c: add hotplug subtest > > qtest: use qmp_exec_hmp_cmd() in blockdev-test > > > > tests/blockdev-test.c | 23 ++- > > tests/libqtest.c| 26 ++ > > tests/libqtest.h| 9 + > > tests/virtio-blk-test.c | 44 +--- > > 4 files changed, 74 insertions(+), 28 deletions(-) > > > > -- > > 1.9.3 > > > > Reviewed-by: Stefan Hajnoczi -- Amos. pgpPmwxx3dJM6.pgp Description: PGP signature
[Qemu-devel] [PATCH v6 0/3] qapi: fix coding style in generated code
Not a serious issue, but it's helpful if we can fix it. V2: split change of scripts/qapi-visit.py to a split patch, eat space by using a special char as Markus suggested V3: update commitlog, update special string, fix of adding const replace string by pattern V4: fix pattern to cleanup special string (Paolo) V5: fix string checking bug (Luiz), update commitlog (Eric) add comments for c_type() function V6: add is_c_ptr() and fix lost endswith (Markus) Amos Kong (3): qapi: fix coding style in parameters list qapi: add const prefix to 'char *' insider c_type() qapi: Suppress unwanted space between type and identifier scripts/qapi-commands.py | 8 +++- scripts/qapi-visit.py| 20 ++-- scripts/qapi.py | 25 +++-- 3 files changed, 32 insertions(+), 21 deletions(-) -- 1.9.3
[Qemu-devel] [PATCH v6 3/3] qapi: Suppress unwanted space between type and identifier
We always generate a space between type and identifier in parameter
and variable declarations, even when idiomatic C style doesn't have
a space there. Suppress it.
Signed-off-by: Amos Kong
---
scripts/qapi-commands.py | 4 ++--
scripts/qapi.py | 23 +--
2 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 34f200a..053ba85 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -102,7 +102,7 @@ def gen_visitor_input_vars_decl(args):
bool has_%(argname)s = false;
''',
argname=c_var(argname))
-if c_type(argtype).endswith("*"):
+if is_c_ptr(argtype):
ret += mcgen('''
%(argtype)s %(argname)s = NULL;
''',
@@ -227,7 +227,7 @@ def gen_marshal_input(name, args, ret_type, middle_mode):
''')
if ret_type:
-if c_type(ret_type).endswith("*"):
+if is_c_ptr(ret_type):
retval = "%s retval = NULL;" % c_type(ret_type)
else:
retval = "%s retval;" % c_type(ret_type)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index dc690bb..0079194 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -470,11 +470,17 @@ def find_enum(name):
def is_enum(name):
return find_enum(name) != None
+eatspace = '\033EATSPACE.'
+
+# A special suffix is added in c_type() for pointer types, and it's
+# stripped in mcgen(). So please notice this when you check the return
+# value of c_type() outside mcgen().
def c_type(name, is_param=False):
if name == 'str':
if is_param:
-return 'const char *'
-return 'char *'
+return 'const char *' + eatspace
+return 'char *' + eatspace
+
elif name == 'int':
return 'int64_t'
elif (name == 'int8' or name == 'int16' or name == 'int32' or
@@ -488,15 +494,19 @@ def c_type(name, is_param=False):
elif name == 'number':
return 'double'
elif type(name) == list:
-return '%s *' % c_list_type(name[0])
+return '%s *%s' % (c_list_type(name[0]), eatspace)
elif is_enum(name):
return name
elif name == None or len(name) == 0:
return 'void'
elif name == name.upper():
-return '%sEvent *' % camel_case(name)
+return '%sEvent *%s' % (camel_case(name), eatspace)
else:
-return '%s *' % name
+return '%s *%s' % (name, eatspace)
+
+def is_c_ptr(name):
+suffix = "*" + eatspace
+return c_type(name).endswith(suffix)
def genindent(count):
ret = ""
@@ -521,7 +531,8 @@ def cgen(code, **kwds):
return '\n'.join(lines) % kwds + '\n'
def mcgen(code, **kwds):
-return cgen('\n'.join(code.split('\n')[1:-1]), **kwds)
+raw = cgen('\n'.join(code.split('\n')[1:-1]), **kwds)
+return re.sub(re.escape(eatspace) + ' *', '', raw)
def basename(filename):
return filename.split("/")[-1]
--
1.9.3
[Qemu-devel] [PATCH v6 2/3] qapi: add const prefix to 'char *' insider c_type()
It's ugly to add const prefix for parameter type by an if statement outside c_type(). This patch adds a parameter to do it. Signed-off-by: Amos Kong Suggested-by: Markus Armbruster Reviewed-by: Eric Blake Reviewed-by: Paolo Bonzini Reviewed-by: Markus Armbruster --- scripts/qapi-commands.py | 4 +--- scripts/qapi.py | 4 +++- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py index 7d93d01..34f200a 100644 --- a/scripts/qapi-commands.py +++ b/scripts/qapi-commands.py @@ -29,9 +29,7 @@ def type_visitor(name): def generate_command_decl(name, args, ret_type): arglist="" for argname, argtype, optional, structured in parse_args(args): -argtype = c_type(argtype) -if argtype == "char *": -argtype = "const char *" +argtype = c_type(argtype, is_param=True) if optional: arglist += "bool has_%s, " % c_var(argname) arglist += "%s %s, " % (argtype, c_var(argname)) diff --git a/scripts/qapi.py b/scripts/qapi.py index 86e9608..dc690bb 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -470,8 +470,10 @@ def find_enum(name): def is_enum(name): return find_enum(name) != None -def c_type(name): +def c_type(name, is_param=False): if name == 'str': +if is_param: +return 'const char *' return 'char *' elif name == 'int': return 'int64_t' -- 1.9.3
[Qemu-devel] [PATCH v6 1/3] qapi: fix coding style in parameters list
A space after * when declaring a pointer type is redundant.
Signed-off-by: Amos Kong
Reviewed-by: Eric Blake
Reviewed-by: Paolo Bonzini
Reviewed-by: Markus Armbruster
---
scripts/qapi-visit.py | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 06a79f1..c129697 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -77,7 +77,7 @@ static void visit_type_%(full_name)s_field_%(c_name)s(Visitor
*m, %(name)s **obj
ret += mcgen('''
-static void visit_type_%(full_name)s_fields(Visitor *m, %(name)s ** obj, Error
**errp)
+static void visit_type_%(full_name)s_fields(Visitor *m, %(name)s **obj, Error
**errp)
{
Error *err = NULL;
''',
@@ -186,7 +186,7 @@ def generate_visit_struct(expr):
ret += mcgen('''
-void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error
**errp)
+void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error
**errp)
{
''',
name=name)
@@ -201,7 +201,7 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const
char *name, Error **
def generate_visit_list(name, members):
return mcgen('''
-void visit_type_%(name)sList(Visitor *m, %(name)sList ** obj, const char
*name, Error **errp)
+void visit_type_%(name)sList(Visitor *m, %(name)sList **obj, const char *name,
Error **errp)
{
Error *err = NULL;
GenericList *i, **prev;
@@ -230,7 +230,7 @@ out:
def generate_visit_enum(name, members):
return mcgen('''
-void visit_type_%(name)s(Visitor *m, %(name)s * obj, const char *name, Error
**errp)
+void visit_type_%(name)s(Visitor *m, %(name)s *obj, const char *name, Error
**errp)
{
visit_type_enum(m, (int *)obj, %(name)s_lookup, "%(name)s", name, errp);
}
@@ -240,7 +240,7 @@ void visit_type_%(name)s(Visitor *m, %(name)s * obj, const
char *name, Error **e
def generate_visit_anon_union(name, members):
ret = mcgen('''
-void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error
**errp)
+void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error
**errp)
{
Error *err = NULL;
@@ -327,7 +327,7 @@ def generate_visit_union(expr):
ret += mcgen('''
-void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error
**errp)
+void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error
**errp)
{
Error *err = NULL;
@@ -399,13 +399,13 @@ def generate_declaration(name, members, genlist=True,
builtin_type=False):
if not builtin_type:
ret += mcgen('''
-void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error
**errp);
+void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error
**errp);
''',
name=name)
if genlist:
ret += mcgen('''
-void visit_type_%(name)sList(Visitor *m, %(name)sList ** obj, const char
*name, Error **errp);
+void visit_type_%(name)sList(Visitor *m, %(name)sList **obj, const char *name,
Error **errp);
''',
name=name)
@@ -415,7 +415,7 @@ def generate_enum_declaration(name, members, genlist=True):
ret = ""
if genlist:
ret += mcgen('''
-void visit_type_%(name)sList(Visitor *m, %(name)sList ** obj, const char
*name, Error **errp);
+void visit_type_%(name)sList(Visitor *m, %(name)sList **obj, const char *name,
Error **errp);
''',
name=name)
@@ -424,7 +424,7 @@ void visit_type_%(name)sList(Visitor *m, %(name)sList **
obj, const char *name,
def generate_decl_enum(name, members, genlist=True):
return mcgen('''
-void visit_type_%(name)s(Visitor *m, %(name)s * obj, const char *name, Error
**errp);
+void visit_type_%(name)s(Visitor *m, %(name)s *obj, const char *name, Error
**errp);
''',
name=name)
--
1.9.3
Re: [Qemu-devel] [PATCH v5 3/3] qapi: Suppress unwanted space between type and identifier
On Mon, Jun 02, 2014 at 05:43:58PM +0200, Markus Armbruster wrote:
> Amos Kong writes:
>
> > We always generate a space between type and identifier in parameter
> > and variable declarations, even when idiomatic C style doesn't have
> > a space there. Suppress it.
> >
> > Signed-off-by: Amos Kong
> > ---
> > scripts/qapi-commands.py | 2 +-
> > scripts/qapi.py | 19 +--
> > 2 files changed, 14 insertions(+), 7 deletions(-)
> >
> > diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
> > index 34f200a..4b49735 100644
> > --- a/scripts/qapi-commands.py
> > +++ b/scripts/qapi-commands.py
> > @@ -102,7 +102,7 @@ def gen_visitor_input_vars_decl(args):
> > bool has_%(argname)s = false;
> > ''',
> > argname=c_var(argname))
> > -if c_type(argtype).endswith("*"):
> > +if c_type(argtype).endswith("*" + eatspace):
> > ret += mcgen('''
> > %(argtype)s %(argname)s = NULL;
> > ''',
>
> Ugly. A function is_c_ptr(argtype) would be cleaner.
def is_c_ptr(argtype):
suffix = "*" + eatspace
return c_type(argtype).endswith(suffix)
> There's similar code in gen_marshal_input():
>
> if ret_type:
> if c_type(ret_type).endswith("*"):
> retval = "%s retval = NULL;" % c_type(ret_type)
> else:
> retval = "%s retval;" % c_type(ret_type)
> ret += mcgen('''
> %(retval)s
> ''',
> retval=retval)
Thanks, I searched by grep in the past, but I still lost one :(
[amos@z qemu]$ git grep c_type|grep endswith
scripts/qapi-commands.py:if c_type(argtype).endswith("*"):
scripts/qapi-commands.py:if c_type(ret_type).endswith("*"):
> Don't you have to patch that, too?
>
> > diff --git a/scripts/qapi.py b/scripts/qapi.py
> > index dc690bb..562d560 100644
> > --- a/scripts/qapi.py
> > +++ b/scripts/qapi.py
> > @@ -470,11 +470,17 @@ def find_enum(name):
> > def is_enum(name):
> > return find_enum(name) != None
> >
> > +eatspace = '\033EATSPACE.'
> > +
> > +# A special suffix is added in c_type() for pointer types, and it's
> > +# stripped in mcgen(). So please notice this when you check the return
> > +# value of c_type() outside mcgen().
> > def c_type(name, is_param=False):
> > if name == 'str':
> > if is_param:
> > -return 'const char *'
> > -return 'char *'
> > +return 'const char *' + eatspace
> > +return 'char *' + eatspace
> > +
> > elif name == 'int':
> > return 'int64_t'
> > elif (name == 'int8' or name == 'int16' or name == 'int32' or
> > @@ -488,15 +494,15 @@ def c_type(name, is_param=False):
> > elif name == 'number':
> > return 'double'
> > elif type(name) == list:
> > -return '%s *' % c_list_type(name[0])
> > +return '%s *%s' % (c_list_type(name[0]), eatspace)
> > elif is_enum(name):
> > return name
> > elif name == None or len(name) == 0:
> > return 'void'
> > elif name == name.upper():
> > -return '%sEvent *' % camel_case(name)
> > +return '%sEvent *%s' % (camel_case(name), eatspace)
> > else:
> > -return '%s *' % name
> > +return '%s *%s' % (name, eatspace)
> >
> > def genindent(count):
> > ret = ""
> > @@ -521,7 +527,8 @@ def cgen(code, **kwds):
> > return '\n'.join(lines) % kwds + '\n'
> >
> > def mcgen(code, **kwds):
> > -return cgen('\n'.join(code.split('\n')[1:-1]), **kwds)
> > +raw = cgen('\n'.join(code.split('\n')[1:-1]), **kwds)
> > +return re.sub(re.escape(eatspace) + ' *', '', raw)
> >
> > def basename(filename):
> > return filename.split("/")[-1]
>
> Rest looks good.
--
Amos.
Re: [Qemu-devel] [PATCH 2/2] virtio-blk-test.c: add hotplug subtest
On Sun, Jun 08, 2014 at 05:57:11PM +0300, Michael S. Tsirkin wrote:
> On Thu, Jun 05, 2014 at 01:12:46PM +0800, Amos Kong wrote:
> > On Wed, Jun 04, 2014 at 05:35:06PM +0200, Stefan Hajnoczi wrote:
> > > On Tue, May 27, 2014 at 03:14:41PM +0800, Amos Kong wrote:
> > > > This patch adds a new subtest, it hotplugs 29 * 8 = 232 virtio-blk
> > > > devices to guest, and try to hot-unplug them.
> > > >
> > > > Note: the hot-unplug can't work without cooperation of guest OS.
> > > >
> > > > Signed-off-by: Amos Kong
> > > > ---
> > > > tests/virtio-blk-test.c | 55
> > > > +
> > > > 1 file changed, 55 insertions(+)
> > > >
> > > > diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
> > > > index 0fdec01..54d1272 100644
> > > > --- a/tests/virtio-blk-test.c
> > > > +++ b/tests/virtio-blk-test.c
> > > > @@ -7,11 +7,65 @@
> > > > * See the COPYING file in the top-level directory.
> > > > */
> > > >
> > > > +#include
> > > > #include
> > > > #include
> > > > #include "libqtest.h"
> > > > #include "qemu/osdep.h"
> > > >
> > > > +static void exec_hmp_cmd(const char *cmd, const char *expected_ret)
> > >
> > > This is a generic function, should it be moved to libqtest.c?
> >
> > OK
> >
> > >
> > > > +{
> > > > +QDict *response;
> > > > +const char *response_return;
> > > > +
> > > > +response = qmp("{\"execute\": \"human-monitor-command\","
> > > > + " \"arguments\": {"
> > > > + " \"command-line\": \"%s\""
> > > > + "}}", cmd);
> >
> > > My only worry if we make this function generic is that cmd isn't escaped
> > > so callers need to be careful.
> > > Maybe something like g_strescape() should be used:
> > >
> > > https://developer.gnome.org/glib/2.37/glib-String-Utility-Functions.html#g-strescape
> >
> >
> > > > +g_assert(response);
> > > > +response_return = qdict_get_try_str(response, "return");
> > > > +g_assert(response_return);
> > > > +g_assert(strcmp(response_return, expected_ret) == 0);
> > >
> > > Please use g_assert_cmpstr() so the error message will be pretty-printed
> > > with the values of response_return and expected_ret:
> > > https://developer.gnome.org/glib/stable/glib-Testing.html#g-assert-cmpstr
> >
> > OK
> >
> > > > +QDECREF(response);
> > > > +}
> > >
> > > > +static void test_blk_hotplug(void)
> > > > +{
> > > > +char addr[6];
> > > > +char cmd[100];
> > > > +int i, j;
> > > > +
> > > > +/* Start with no network/block device, slots 3~0x1f are free */
> > > > +qtest_start("-net none");
> > > > +
> > > > +for (i = 3; i <= 0x1f; i++) {
> > > > +for (j = 7; j >= 0; j--) {
> > > > +sprintf(addr, "%x.%x", i, j);
> > > > +sprintf(cmd, "drive_add 0x%s
> > > > if=none,file=/dev/null,id=drv-%s",
> > > > +addr, addr);
> > >
> > > Does the address matter with if=none? I think you can just do
> > > "drive_add 0 if=none,...".
> >
> > drive_add [[:]:]
> >
> > multiple function is enabled, slot parameter is needed.
> >
> > > > +exec_hmp_cmd(cmd, "OK\r\n");
> > > > +
> > > > +sprintf(cmd, "device_add
> > > > virtio-blk-pci,id=dev-%s,drive=drv-%s,"
> > > > + "addr=0x%s,multifunction=on", addr, addr,
> > > > addr);
> > > > +exec_hmp_cmd(cmd, "");
> > > > +}
> > > > +}
> > > > +
> > > > +/* hot-unplug doesn't work without cooperation of guest OS */
> > >
> > > We could implement hotplug/hotunplug in libqos, PCI support already
> > > exists: tests/libqos/pci.h.
> > >
> > > But I guess this test is useful too, it checks the case where the guest
> > > is not cooperating?
> >
> > OK, we can update and merge this test first, I will investigate and
> > write a new subtest with PCI support.
> >
> > --
> > Amos.
>
>
> Which tree should this go in through?
The V4 had been reviewed by Stefan.
http://lists.gnu.org/archive/html/qemu-devel/2014-06/msg02063.html
The file (tests/virtio-blk-test.c) was added and applied by Andreas.
Andreas, do you plan to apply the V4?
--
Amos.
[Qemu-devel] [PATCH v4 1/4] virtio-blk-test.c: change pci_nop() to virtblk_init()
I want to add a new subtest in virtio-blk-test.c, it will start
guest without network. The original pci_init() did nothing, but
it's good to reserve a very simple initialization testing.
Signed-off-by: Amos Kong
Reviewed-by: Stefan Hajnoczi
---
tests/virtio-blk-test.c | 13 ++---
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index d53f875..0fdec01 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -12,9 +12,12 @@
#include "libqtest.h"
#include "qemu/osdep.h"
-/* Tests only initialization so far. TODO: Replace with functional tests */
-static void pci_nop(void)
+/* Tests only initialization */
+static void virtblk_init(void)
{
+qtest_start("-drive id=drv0,if=none,file=/dev/null "
+"-device virtio-blk-pci,drive=drv0");
+qtest_end();
}
int main(int argc, char **argv)
@@ -22,13 +25,9 @@ int main(int argc, char **argv)
int ret;
g_test_init(&argc, &argv, NULL);
-qtest_add_func("/virtio/blk/pci/nop", pci_nop);
+qtest_add_func("/virtio/blk/pci/init", virtblk_init);
-qtest_start("-drive id=drv0,if=none,file=/dev/null "
-"-device virtio-blk-pci,drive=drv0");
ret = g_test_run();
-qtest_end();
-
return ret;
}
--
1.9.3
[Qemu-devel] [PATCH v4 0/4] test virtio-blk hotplug
It's worth to add a hotplug test to qtest, but without cooperation of guest OS, new devices can't be initialized by guest, and hot-unplug doesn't work. However, the new test can cover some part of code of hotplug/unplug. I will write another subtest to test hotplug with pci support. V2: move qmp_exec_hmp_cmd() to libqtest.c excape hmp cmd (stefanha) use qmp_exec_hmp_cmd() in blockdev-test V3: use vp_list to format string, free escaped string V4: free escaped string by g_free() Amos Kong (4): virtio-blk-test.c: change pci_nop() to virtblk_init() qtest: introduce qmp_exec_hmp_cmd() virtio-blk-test.c: add hotplug subtest qtest: use qmp_exec_hmp_cmd() in blockdev-test tests/blockdev-test.c | 23 ++- tests/libqtest.c| 26 ++ tests/libqtest.h| 9 + tests/virtio-blk-test.c | 44 +--- 4 files changed, 74 insertions(+), 28 deletions(-) -- 1.9.3
[Qemu-devel] [PATCH v4 3/4] virtio-blk-test.c: add hotplug subtest
This patch adds a new subtest, it hotplugs 29 * 8 = 232 virtio-blk
devices to guest, and try to hot-unplug them.
Note: the hot-unplug can't work without cooperation of guest OS.
Signed-off-by: Amos Kong
Reviewed-by: Stefan Hajnoczi
---
tests/virtio-blk-test.c | 31 +++
1 file changed, 31 insertions(+)
diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index 0fdec01..7358203 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -7,11 +7,41 @@
* See the COPYING file in the top-level directory.
*/
+#include
#include
#include
#include "libqtest.h"
#include "qemu/osdep.h"
+static void test_blk_hotplug(void)
+{
+int i, j;
+
+/* start with no network/block device, slots 3~0x1f are free */
+qtest_start("-net none");
+
+for (i = 3; i <= 0x1f; i++) {
+for (j = 7; j >= 0; j--) {
+qmp_exec_hmp_cmd("OK\r\n",
+ "drive_add 0 if=none,file=/dev/null,id=drv-%x.%x",
+ i, j);
+qmp_exec_hmp_cmd("",
+ "device_add
virtio-blk-pci,id=dev-%x.%x,drive=drv-%x.%x,"
+ "addr=0x%x.%x,multifunction=on", i, j, i, j, i, j);
+}
+}
+
+/* hot-unplug doesn't work without cooperation of guest OS */
+for (i = 3; i <= 0x1f; i++) {
+for (j = 7; j >= 0; j--) {
+qmp_exec_hmp_cmd("", "drive_del drv-%x.%x", i, j);
+qmp_exec_hmp_cmd("", "device_del dev-%x.%x", i, j);
+}
+}
+
+qtest_end();
+}
+
/* Tests only initialization */
static void virtblk_init(void)
{
@@ -26,6 +56,7 @@ int main(int argc, char **argv)
g_test_init(&argc, &argv, NULL);
qtest_add_func("/virtio/blk/pci/init", virtblk_init);
+qtest_add_func("/virtio/blk/pci/hotplug", test_blk_hotplug);
ret = g_test_run();
--
1.9.3
[Qemu-devel] [PATCH v4 2/4] qtest: introduce qmp_exec_hmp_cmd()
This patch wraps a helper function to execute human command by
one QMP command (human-monitor-command). It also checks the return
string.
Signed-off-by: Amos Kong
---
tests/libqtest.c | 26 ++
tests/libqtest.h | 9 +
2 files changed, 35 insertions(+)
diff --git a/tests/libqtest.c b/tests/libqtest.c
index 71468ac..90babc9 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -646,3 +646,29 @@ void qmp_discard_response(const char *fmt, ...)
qtest_qmpv_discard_response(global_qtest, fmt, ap);
va_end(ap);
}
+
+void qmp_exec_hmp_cmd(const char *expected_ret, const char *fmt, ...)
+{
+va_list ap;
+char cmd[1024];
+char *escaped_cmd;
+QDict *response;
+const char *response_return;
+
+va_start(ap, fmt);
+vsprintf(cmd, fmt, ap);
+va_end(ap);
+
+escaped_cmd = g_strescape(cmd, NULL);
+response = qmp("{\"execute\": \"human-monitor-command\","
+ " \"arguments\": {"
+ " \"command-line\": \"%s\""
+ "}}", escaped_cmd);
+g_free(escaped_cmd);
+
+g_assert(response);
+response_return = qdict_get_try_str(response, "return");
+g_assert(response_return);
+g_assert_cmpstr(response_return, ==, expected_ret);
+QDECREF(response);
+}
diff --git a/tests/libqtest.h b/tests/libqtest.h
index 8f323c7..d2959d3 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -375,6 +375,15 @@ QDict *qmp(const char *fmt, ...);
void qmp_discard_response(const char *fmt, ...);
/**
+ * qmp_exec_hmp_cmd:
+ * @expected_ret: expected return string
+ * @fmt...: HMP command to execute
+ *
+ * Executes HMP command by 'human-monitor-command'.
+ */
+void qmp_exec_hmp_cmd(const char *expected_ret, const char *fmt, ...);
+
+/**
* qmp_receive:
*
* Reads a QMP message from QEMU and returns the response.
--
1.9.3
[Qemu-devel] [PATCH v4 4/4] qtest: use qmp_exec_hmp_cmd() in blockdev-test
Signed-off-by: Amos Kong
Reviewed-by: Stefan Hajnoczi
---
tests/blockdev-test.c | 23 ++-
1 file changed, 2 insertions(+), 21 deletions(-)
diff --git a/tests/blockdev-test.c b/tests/blockdev-test.c
index c940e00..c9127c0 100644
--- a/tests/blockdev-test.c
+++ b/tests/blockdev-test.c
@@ -16,35 +16,16 @@
static void test_drive_add_empty(void)
{
-QDict *response;
-const char *response_return;
-
/* Start with an empty drive */
qtest_start("-drive if=none,id=drive0");
/* Delete the drive */
-response = qmp("{\"execute\": \"human-monitor-command\","
- " \"arguments\": {"
- " \"command-line\": \"drive_del drive0\""
- "}}");
-g_assert(response);
-response_return = qdict_get_try_str(response, "return");
-g_assert(response_return);
-g_assert(strcmp(response_return, "") == 0);
-QDECREF(response);
+qmp_exec_hmp_cmd("", "drive_del drive0");
/* Ensure re-adding the drive works - there should be no duplicate ID error
* because the old drive must be gone.
*/
-response = qmp("{\"execute\": \"human-monitor-command\","
- " \"arguments\": {"
- " \"command-line\": \"drive_add 0 if=none,id=drive0\""
- "}}");
-g_assert(response);
-response_return = qdict_get_try_str(response, "return");
-g_assert(response_return);
-g_assert(strcmp(response_return, "OK\r\n") == 0);
-QDECREF(response);
+qmp_exec_hmp_cmd("OK\r\n", "drive_add 0 if=none,id=drive0");
qtest_end();
}
--
1.9.3
Re: [Qemu-devel] [PATCH v3 3/4] virtio-blk-test.c: add hotplug subtest
On Fri, Jun 06, 2014 at 09:31:57AM +, Gonglei (Arei) wrote:
> > -Original Message-
> > From: Amos Kong [mailto:ak...@redhat.com]
> > Sent: Friday, June 06, 2014 4:24 PM
> > To: Gonglei (Arei)
> > Cc: qemu-devel@nongnu.org; stefa...@gmail.com; afaer...@suse.de
> > Subject: Re: [PATCH v3 3/4] virtio-blk-test.c: add hotplug subtest
> >
> > On Fri, Jun 06, 2014 at 08:05:15AM +, Gonglei (Arei) wrote:
> > > > -Original Message-
> > > > From: Amos Kong [mailto:ak...@redhat.com]
> > > > Sent: Friday, June 06, 2014 1:29 PM
> > > > To: Gonglei (Arei)
> > > > Cc: qemu-devel@nongnu.org; stefa...@gmail.com; afaer...@suse.de
> > > > Subject: Re: [PATCH v3 3/4] virtio-blk-test.c: add hotplug subtest
> > > >
> > > > On Fri, Jun 06, 2014 at 01:28:59AM +, Gonglei (Arei) wrote:
> > > > > Hi, Amos
> > > > >
> > > > > > -Original Message-
> > > > > > From: Amos Kong [mailto:ak...@redhat.com]
> > > > > > Sent: Friday, June 06, 2014 9:12 AM
> > > > > > To: qemu-devel@nongnu.org
> > > > > > Cc: stefa...@gmail.com; afaer...@suse.de; Gonglei (Arei)
> > > > > > Subject: [PATCH v3 3/4] virtio-blk-test.c: add hotplug subtest
> > > > > >
> > > > > > This patch adds a new subtest, it hotplugs 29 * 8 = 232 virtio-blk
> > > > > > devices to guest, and try to hot-unplug them.
> > > > > >
> > > > > > Note: the hot-unplug can't work without cooperation of guest OS.
> > > >
> > > >
> > > > > > +static void test_blk_hotplug(void)
> > > > > > +{
> > > > > > +int i, j;
> > > > > > +
> > > > > > +/* start with no network/block device, slots 3~0x1f are free */
> > > > > > +qtest_start("-net none");
> > > > > > +
> > > > > > +for (i = 3; i <= 0x1f; i++) {
> > > > > > +for (j = 7; j >= 0; j--) {
> > > > > > +qmp_exec_hmp_cmd("OK\r\n",
> > > > > > + "drive_add 0
> > > > > > if=none,file=/dev/null,id=drv-%x.%x",
> > > > > > + i, j);
> > > > > > +qmp_exec_hmp_cmd("",
> > > > > > + "device_add
> > > > > > virtio-blk-pci,id=dev-%x.%x,drive=drv-%x.%x,"
> > > > > > + "addr=0x%x.%x,multifunction=on", i, j, i, j,
> > i,
> > > > j);
> > > > > > +}
> > > > > > +}
> > > > >
> > > > > Do the pci multifunction devices support hotplug/unplug yet?
> > > >
> > > > Yes.
> > > >
> > > > You can hotplug single functions one by one, guest will identify
> > > > all the functions when function 0 is added.
> > > > Hotunplug function 0, all functions in same slot will be removed.
> > > >
> > > Thanks.
> > >
> > > IMHO, if we hot-plug function 0 first, the other functions will be not
> > > support
> > to
> > > hot-plug.
> >
> > Actually other functions can also be identified by guest, I had a
> > commit to add this support.
> >
> > | commit f382a086f3129edc152b8044b69ccc6682e637bb
> > | Author: Amos Kong
> > | Date: Fri Nov 25 15:03:07 2011 +0800
> > |
> > | PCI: Can continually add funcs after adding func0
> >
> >
> Good, Thanks!
>
> BTW, the limits of ACPI DSDT table have been resolved yet?
Which limit?
There was a hot-unplug bug of multiple function, I tried to
fix it by change ACPI DSDT table in seabios, but finally we
fixed this in kernel.
|commit ce29ca3ea40744f24c2b5d88431e8ac566d257cc
|Author: Amos Kong
|Date: Wed May 23 10:20:35 2012 -0600
|
|PCI: acpiphp: remove all functions in slot, even without ACPI _EJx
> > > Similarly, it will not be allowed if we only hot-unplug function 1~7,
> > > but function 0, right?
> >
> > Yes. function 0 is always necessary.
> >
> > > Best regards,
> > > -Gonglei
> >
> > --
> > Amos.
--
Amos.
Re: [Qemu-devel] [PATCH v3 3/4] virtio-blk-test.c: add hotplug subtest
On Fri, Jun 06, 2014 at 08:05:15AM +, Gonglei (Arei) wrote:
> > -Original Message-
> > From: Amos Kong [mailto:ak...@redhat.com]
> > Sent: Friday, June 06, 2014 1:29 PM
> > To: Gonglei (Arei)
> > Cc: qemu-devel@nongnu.org; stefa...@gmail.com; afaer...@suse.de
> > Subject: Re: [PATCH v3 3/4] virtio-blk-test.c: add hotplug subtest
> >
> > On Fri, Jun 06, 2014 at 01:28:59AM +, Gonglei (Arei) wrote:
> > > Hi, Amos
> > >
> > > > -Original Message-
> > > > From: Amos Kong [mailto:ak...@redhat.com]
> > > > Sent: Friday, June 06, 2014 9:12 AM
> > > > To: qemu-devel@nongnu.org
> > > > Cc: stefa...@gmail.com; afaer...@suse.de; Gonglei (Arei)
> > > > Subject: [PATCH v3 3/4] virtio-blk-test.c: add hotplug subtest
> > > >
> > > > This patch adds a new subtest, it hotplugs 29 * 8 = 232 virtio-blk
> > > > devices to guest, and try to hot-unplug them.
> > > >
> > > > Note: the hot-unplug can't work without cooperation of guest OS.
> >
> >
> > > > +static void test_blk_hotplug(void)
> > > > +{
> > > > +int i, j;
> > > > +
> > > > +/* start with no network/block device, slots 3~0x1f are free */
> > > > +qtest_start("-net none");
> > > > +
> > > > +for (i = 3; i <= 0x1f; i++) {
> > > > +for (j = 7; j >= 0; j--) {
> > > > +qmp_exec_hmp_cmd("OK\r\n",
> > > > + "drive_add 0
> > > > if=none,file=/dev/null,id=drv-%x.%x",
> > > > + i, j);
> > > > +qmp_exec_hmp_cmd("",
> > > > + "device_add
> > > > virtio-blk-pci,id=dev-%x.%x,drive=drv-%x.%x,"
> > > > + "addr=0x%x.%x,multifunction=on", i, j, i, j, i,
> > j);
> > > > +}
> > > > +}
> > >
> > > Do the pci multifunction devices support hotplug/unplug yet?
> >
> > Yes.
> >
> > You can hotplug single functions one by one, guest will identify
> > all the functions when function 0 is added.
> > Hotunplug function 0, all functions in same slot will be removed.
> >
> Thanks.
>
> IMHO, if we hot-plug function 0 first, the other functions will be not
> support to
> hot-plug.
Actually other functions can also be identified by guest, I had a
commit to add this support.
| commit f382a086f3129edc152b8044b69ccc6682e637bb
| Author: Amos Kong
| Date: Fri Nov 25 15:03:07 2011 +0800
|
| PCI: Can continually add funcs after adding func0
> Similarly, it will not be allowed if we only hot-unplug function 1~7,
> but function 0, right?
Yes. function 0 is always necessary.
> Best regards,
> -Gonglei
--
Amos.
Re: [Qemu-devel] active block commit bug?
On Thu, Jun 05, 2014 at 09:06:42AM +0200, Markus Armbruster wrote: > Eric Blake writes: > > > On 06/04/2014 08:09 PM, Fam Zheng wrote: > > > >>> Sounds like we have an off-by-one condition if empty files behave > >>> differently from other files. We ought to fix that bug (not that your > >>> normal guest will ever have a 0-length backing file, but this was what I > >>> was trying to use for libvirt's probing of whether active commit is > >>> supported) > >>> > >> > >> Yes, agreed, this special case is only going to make management confused. I > >> will send a patch to fix this. > > > > Thanks. > > > >> > >> Eric, is this a good way to probe the active commit? I was expecting full > >> instrospection of QMP could do it, but I don't know about the status of > >> that > >> piece of work. Amos, any ideas? > > > > Introspection already missed qemu 2.0 when active commit was added; and > > we're close enough to soft freeze for 2.1 that I'm guessing it will miss > > 2.1 as well :( > > Almost certainly. It has non-trivial design issues. To have a chance > to make it into 2.x, it needs to be posted for review early in the 2.x > cycle. I already started to respin the new version, let's see the new version next week. > [...] -- Amos.
Re: [Qemu-devel] [PATCH v3 3/4] virtio-blk-test.c: add hotplug subtest
On Fri, Jun 06, 2014 at 01:28:59AM +, Gonglei (Arei) wrote:
> Hi, Amos
>
> > -Original Message-
> > From: Amos Kong [mailto:ak...@redhat.com]
> > Sent: Friday, June 06, 2014 9:12 AM
> > To: qemu-devel@nongnu.org
> > Cc: stefa...@gmail.com; afaer...@suse.de; Gonglei (Arei)
> > Subject: [PATCH v3 3/4] virtio-blk-test.c: add hotplug subtest
> >
> > This patch adds a new subtest, it hotplugs 29 * 8 = 232 virtio-blk
> > devices to guest, and try to hot-unplug them.
> >
> > Note: the hot-unplug can't work without cooperation of guest OS.
> > +static void test_blk_hotplug(void)
> > +{
> > +int i, j;
> > +
> > +/* start with no network/block device, slots 3~0x1f are free */
> > +qtest_start("-net none");
> > +
> > +for (i = 3; i <= 0x1f; i++) {
> > +for (j = 7; j >= 0; j--) {
> > +qmp_exec_hmp_cmd("OK\r\n",
> > + "drive_add 0
> > if=none,file=/dev/null,id=drv-%x.%x",
> > + i, j);
> > +qmp_exec_hmp_cmd("",
> > + "device_add
> > virtio-blk-pci,id=dev-%x.%x,drive=drv-%x.%x,"
> > + "addr=0x%x.%x,multifunction=on", i, j, i, j, i, j);
> > +}
> > +}
>
> Do the pci multifunction devices support hotplug/unplug yet?
Yes.
You can hotplug single functions one by one, guest will identify
all the functions when function 0 is added.
Hotunplug function 0, all functions in same slot will be removed.
> Best regards,
> -Gonglei
--
Amos.
[Qemu-devel] [PATCH v3 2/4] qtest: introduce qmp_exec_hmp_cmd()
This patch wraps a helper function to execute human command by
one QMP command (human-monitor-command). It also checks the return
string.
Signed-off-by: Amos Kong
---
tests/libqtest.c | 26 ++
tests/libqtest.h | 9 +
2 files changed, 35 insertions(+)
diff --git a/tests/libqtest.c b/tests/libqtest.c
index 71468ac..c14615c 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -646,3 +646,29 @@ void qmp_discard_response(const char *fmt, ...)
qtest_qmpv_discard_response(global_qtest, fmt, ap);
va_end(ap);
}
+
+void qmp_exec_hmp_cmd(const char *expected_ret, const char *fmt, ...)
+{
+va_list ap;
+char cmd[1024];
+char *escaped_cmd;
+QDict *response;
+const char *response_return;
+
+va_start(ap, fmt);
+vsprintf(cmd, fmt, ap);
+va_end(ap);
+
+escaped_cmd = g_strescape(cmd, NULL);
+response = qmp("{\"execute\": \"human-monitor-command\","
+ " \"arguments\": {"
+ " \"command-line\": \"%s\""
+ "}}", escaped_cmd);
+free(escaped_cmd);
+
+g_assert(response);
+response_return = qdict_get_try_str(response, "return");
+g_assert(response_return);
+g_assert_cmpstr(response_return, ==, expected_ret);
+QDECREF(response);
+}
diff --git a/tests/libqtest.h b/tests/libqtest.h
index 8f323c7..d2959d3 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -375,6 +375,15 @@ QDict *qmp(const char *fmt, ...);
void qmp_discard_response(const char *fmt, ...);
/**
+ * qmp_exec_hmp_cmd:
+ * @expected_ret: expected return string
+ * @fmt...: HMP command to execute
+ *
+ * Executes HMP command by 'human-monitor-command'.
+ */
+void qmp_exec_hmp_cmd(const char *expected_ret, const char *fmt, ...);
+
+/**
* qmp_receive:
*
* Reads a QMP message from QEMU and returns the response.
--
1.9.3
[Qemu-devel] [PATCH v3 4/4] qtest: use qmp_exec_hmp_cmd() in blockdev-test
Signed-off-by: Amos Kong
Reviewed-by: Stefan Hajnoczi
---
tests/blockdev-test.c | 23 ++-
1 file changed, 2 insertions(+), 21 deletions(-)
diff --git a/tests/blockdev-test.c b/tests/blockdev-test.c
index c940e00..c9127c0 100644
--- a/tests/blockdev-test.c
+++ b/tests/blockdev-test.c
@@ -16,35 +16,16 @@
static void test_drive_add_empty(void)
{
-QDict *response;
-const char *response_return;
-
/* Start with an empty drive */
qtest_start("-drive if=none,id=drive0");
/* Delete the drive */
-response = qmp("{\"execute\": \"human-monitor-command\","
- " \"arguments\": {"
- " \"command-line\": \"drive_del drive0\""
- "}}");
-g_assert(response);
-response_return = qdict_get_try_str(response, "return");
-g_assert(response_return);
-g_assert(strcmp(response_return, "") == 0);
-QDECREF(response);
+qmp_exec_hmp_cmd("", "drive_del drive0");
/* Ensure re-adding the drive works - there should be no duplicate ID error
* because the old drive must be gone.
*/
-response = qmp("{\"execute\": \"human-monitor-command\","
- " \"arguments\": {"
- " \"command-line\": \"drive_add 0 if=none,id=drive0\""
- "}}");
-g_assert(response);
-response_return = qdict_get_try_str(response, "return");
-g_assert(response_return);
-g_assert(strcmp(response_return, "OK\r\n") == 0);
-QDECREF(response);
+qmp_exec_hmp_cmd("OK\r\n", "drive_add 0 if=none,id=drive0");
qtest_end();
}
--
1.9.3
