Re: [Qemu-devel] [RFC PATCH 00/34] Multi Architecture System Emulation
On Sun, May 10, 2015 at 11:29 PM, Peter Crosthwaite wrote: > Hi All, > > This is target-multi, a system-mode build that can support multiple > cpu-types. Patches 1-3 are the main infrastructure. The hard part > is the per-target changes needed to get each arch into an includable > state. > > Two architectures are initially converted. Microblaze and ARM. Step > by step conversion in done for each. A microblaze is added to > Xilinx Zynq platform as a test case. This will be elaborted more in > future spins. This use case is valid, as Microblazes can be added (any > number of them!) in Zynq FPGA programmable logic configuration. > > The hardest part is what to do about bootloading. Currently each arch > has it's own architecture specific bootloading which may assume a > single architecture. I have applied some hacks to at least get this > RFC testable using a -kernel -firmware split but going forward being > able to associate an elf/image with a cpu explictitly needs to be > solved. > > For the implementation of this series, the trickiest part is cpu.h > inclusion management. There are now more than one cpu.h's and different > parts of the tree need a different include scheme. target-multi defines > it's own cpu.h which is bare minimum defs as needed by core code only. > target-foo/cpu.h are mostly the same but refactored to reuse common > code (with target-multi/cpu-head.h). Inclusion scheme goes something like > this (for the multi-arch build): > > 1: All obj-y modules include target-multi/cpu.h > 2: Core code includes no other cpu.h's > 3: target-foo/ implementation code includes target-foo/cpu.h > 4: System level code (e.g. mach models) can use multiple target-foo/cpu.h's > > Point 4 means that cpu.h's needs to be refactored to be able to include one > after the other. The interrupts for ARM and MB needed to be renamed to avoid > namespace collision. A few other defs needed multiple include guards, and > a few defs which where only for user mode are compiled out or relocated. No > attempt at support for multi-arch linux-user mode (if that even makes sense?). > > The env as handle by common code now needs to architecture-agnostic. The > MB and ARM envs are refactored to have CPU_COMMON as the first field(s) > allowing QOM-style pointer casts to/from a generic env which contains only > CPU_COMMON. Might need to lock down some struct packing for that but it > works for me so far. > > The helper function namespace is going to be tricky. I haven't tackled the > problem just yet, but looking for ideas on how we can avoid prefacing all > helpers with arch prefixes to avoid link-time collisions because multiple > arches use the same helper names. > Ok so have half a plan here now for the helper link-time namespace problem. The top level HELPER() macro can be patched to glue() in an architecture name set by cpu.h. The every use of HELPER(foo) will be namespace collision free. The catch? Some arches mass-use literal helper_foo for defs and calls. Fortunately ARM is well behaved and only has about 4 usages so thats an easy convert. Microblaze has more. It will be a reasonably simple (but tedious) scripted changed to convert uses and definitions of helper_foo to HELPER(foo) but it saves on the verbosity of having to go in and preface everything (all of callsites, definitions and helper.h defs themselves). Regards, Peter > A lowest common denomintor approach is taken on architecture specifics. E.g. > TARGET_LONG is 64-bit, and the address space sizes and NUM_MMU_MODES is set > to the maximum of all the supported arches. > > The remaining globally defined interfaces between core code and CPUs are > QOMified per-cpu (P2) > > Microblaze translation needs a change pattern to allow conversion to 64-bit > TARGET_LONG. Uses of TCGv need to be removed and explicited to 32-bit. > > This RFC will serve as a reference as I send bits and piece to the respective > maintainers (many major subsystems are patched). > > No support for KVM, im not sure if a mix of TCG and KVM is supported even for > a single arch? (which would be prerequisite to MA KVM). > > Depends (not heavily) on my on-list disas QOMification. Test instructions > available on request. I have tested ARM & MB elfs handshaking through shared > memory and both printfing to the same UART (verifying system level > connectivity). -d in_asm works with the mix of disas arches comming out. > > Regards, > Peter > > Peter Crosthwaite (34): > cpu-all: Prototype cpu_exec and cpu_signal_handler > tcg+qom: QOMify core CPU defintions > target-multi: Add > mb: Change target long to 64b > mb: cpu: Delete MMAP_SHIFT definition > mb: rename EXCP macros > mb: Remove ELF_MACHINE from cpu.h > mb: cpu.h: Move cpu-all include > mb: delete dummy prototypes > HACK: microblaze: rename clz helper > mb: cpu: Remove MMUx macros > mb: cpu: Move CPU_COMMON to front of env > mb: cpu: Change phys and virt address ranges. > mb: Use qomified tcg defintions > hw: mb: Exp
Re: [Qemu-devel] [PATCH v2] block: Let bdrv_drain_all() to call aio_poll() for each AioContext
Am 14.05.2015 um 18:03 schrieb Alexander Yarygin:
> After the commit 9b536adc ("block: acquire AioContext in
> bdrv_drain_all()") the aio_poll() function got called for every
> BlockDriverState, in assumption that every device may have its own
> AioContext. The bdrv_drain_all() function is called in each
> virtio_reset() call, which in turn is called for every virtio-blk
> device on initialization, so we got aio_poll() called
> 'length(device_list)^2' times.
>
> If we have thousands of disks attached, there are a lot of
> BlockDriverStates but only a few AioContexts, leading to tons of
> unnecessary aio_poll() calls. For example, startup times with 1000 disks
> takes over 13 minutes.
>
> This patch changes the bdrv_drain_all() function allowing it find shared
> AioContexts and to call aio_poll() only for unique ones. This results in
> much better startup times, e.g. 1000 disks do come up within 5 seconds.
>
> Cc: Christian Borntraeger
> Cc: Cornelia Huck
> Cc: Kevin Wolf
> Cc: Paolo Bonzini
> Cc: Stefan Hajnoczi
> Signed-off-by: Alexander Yarygin
Applying on top of 2.3 I can verify the speedup.
Tested-by: Christian Borntraeger
PS: There is another independent issue now in the kernel when exiting QEMU
caused by Linux kernel commit 6098b45b32e6baeacc04790773ced9340601d511
Author: Gu Zheng
AuthorDate: Wed Sep 3 17:45:44 2014 +0800
Commit: Benjamin LaHaise
CommitDate: Thu Sep 4 16:54:47 2014 -0400
aio: block exit_aio() until all context requests are completed
A QEMU with 1000 devices will sleep a long time in exit_aio with now obvious
sign of activity as zombie process. I will take care of that
Christian
> ---
> block.c | 40 +---
> 1 file changed, 25 insertions(+), 15 deletions(-)
>
> diff --git a/block.c b/block.c
> index f2f8ae7..bdfb1ce 100644
> --- a/block.c
> +++ b/block.c
> @@ -1987,17 +1987,6 @@ static bool bdrv_requests_pending(BlockDriverState *bs)
> return false;
> }
>
> -static bool bdrv_drain_one(BlockDriverState *bs)
> -{
> -bool bs_busy;
> -
> -bdrv_flush_io_queue(bs);
> -bdrv_start_throttled_reqs(bs);
> -bs_busy = bdrv_requests_pending(bs);
> -bs_busy |= aio_poll(bdrv_get_aio_context(bs), bs_busy);
> -return bs_busy;
> -}
> -
> /*
> * Wait for pending requests to complete on a single BlockDriverState subtree
> *
> @@ -2010,8 +1999,13 @@ static bool bdrv_drain_one(BlockDriverState *bs)
> */
> void bdrv_drain(BlockDriverState *bs)
> {
> -while (bdrv_drain_one(bs)) {
> +bool busy = true;
> +
> +while (busy) {
> /* Keep iterating */
> +bdrv_flush_io_queue(bs);
> +busy = bdrv_requests_pending(bs);
> +busy |= aio_poll(bdrv_get_aio_context(bs), busy);
> }
> }
>
> @@ -2030,20 +2024,35 @@ void bdrv_drain(BlockDriverState *bs)
> void bdrv_drain_all(void)
> {
> /* Always run first iteration so any pending completion BHs run */
> -bool busy = true;
> +bool busy = true, pending = false;
> BlockDriverState *bs;
> +GSList *aio_ctxs = NULL, *ctx;
> +AioContext *aio_context;
>
> while (busy) {
> busy = false;
>
> QTAILQ_FOREACH(bs, &bdrv_states, device_list) {
> -AioContext *aio_context = bdrv_get_aio_context(bs);
> +aio_context = bdrv_get_aio_context(bs);
> +
> +aio_context_acquire(aio_context);
> +bdrv_flush_io_queue(bs);
> +busy |= bdrv_requests_pending(bs);
> +aio_context_release(aio_context);
> +if (!aio_ctxs || !g_slist_find(aio_ctxs, aio_context)) {
> +aio_ctxs = g_slist_prepend(aio_ctxs, aio_context);
> +}
> +}
> +pending = busy;
>
> +for (ctx = aio_ctxs; ctx != NULL; ctx = ctx->next) {
> +aio_context = ctx->data;
> aio_context_acquire(aio_context);
> -busy |= bdrv_drain_one(bs);
> +busy |= aio_poll(aio_context, pending);
> aio_context_release(aio_context);
> }
> }
> +g_slist_free(aio_ctxs);
> }
>
> /* make a BlockDriverState anonymous by removing from bdrv_state and
> @@ -6087,6 +6096,7 @@ void bdrv_flush_io_queue(BlockDriverState *bs)
> } else if (bs->file) {
> bdrv_flush_io_queue(bs->file);
> }
> +bdrv_start_throttled_reqs(bs);
> }
>
> static bool append_open_options(QDict *d, BlockDriverState *bs)
>
Re: [Qemu-devel] [PATCH v2] block: Let bdrv_drain_all() to call aio_poll() for each AioContext
Am 15.05.2015 um 08:59 schrieb Christian Borntraeger: > PS: There is another independent issue now in the kernel when exiting QEMU > caused by Linux kernel commit 6098b45b32e6baeacc04790773ced9340601d511 > Author: Gu Zheng > AuthorDate: Wed Sep 3 17:45:44 2014 +0800 > Commit: Benjamin LaHaise > CommitDate: Thu Sep 4 16:54:47 2014 -0400 > > aio: block exit_aio() until all context requests are completed > > A QEMU with 1000 devices will sleep a long time in exit_aio with now obvious > sign of activity as zombie process. I will take care of that > > Christian To make it clear. This kernel wait time happens with and without Alexanders patch.
Re: [Qemu-devel] [PATCH v3 12/12] virtio-scsi-dataplane: Add "device IO" op blocker listener
On Fri, 05/15 08:50, Paolo Bonzini wrote:
>
>
> On 15/05/2015 08:04, Fam Zheng wrote:
> > @@ -111,6 +111,10 @@ static void
> > virtio_scsi_iothread_handle_ctrl(EventNotifier *notifier)
> > VirtIOSCSI *s = VIRTIO_SCSI(vring->parent);
> > VirtIOSCSIReq *req;
> >
> > +if (s->pause_counter) {
> > +virtio_scsi_stop_ioeventfd(s);
> > +return;
> > +}
> > event_notifier_test_and_clear(notifier);
> > while ((req = virtio_scsi_pop_req_vring(s, vring))) {
> > virtio_scsi_handle_ctrl_req(s, req);
> > @@ -124,6 +128,10 @@ static void
> > virtio_scsi_iothread_handle_event(EventNotifier *notifier)
> > VirtIOSCSI *s = vring->parent;
> > VirtIODevice *vdev = VIRTIO_DEVICE(s);
> >
> > +if (s->pause_counter) {
> > +virtio_scsi_stop_ioeventfd(s);
> > +return;
> > +}
> > event_notifier_test_and_clear(notifier);
> >
> > if (!(vdev->status & VIRTIO_CONFIG_S_DRIVER_OK)) {
>
> Why are these needed?
Not strictly. I think it's something like an "assert(!s->pause_counter)" but
said gently.
Fam
Re: [Qemu-devel] [PATCH v3 01/12] block: Add op blocker type "device IO"
On Fri, 05/15 14:22, Wen Congyang wrote: > On 05/15/2015 02:04 PM, Fam Zheng wrote: > > It blocks device IO. > > I am reading mirror codes recently, and have a question: > When block job mirror is finished, the source and target is synced. But we > call bdrv_swap() later(in bh context). Can the guest write something to > the source before the bh is scheduled? If the answer is yes, I think > we should use this to block the guest's disk I/O. I think you're right. After mirror_run returns, anything can happen on this aio context, including guest writing. Fam
Re: [Qemu-devel] [PATCH RFC 1/1] virtio: migrate config_vector
Am 14.05.2015 um 19:00 schrieb Dr. David Alan Gilbert: > * Christian Borntraeger (borntrae...@de.ibm.com) wrote: >> Am 14.05.2015 um 11:36 schrieb Michael S. Tsirkin: >>> On Thu, May 14, 2015 at 11:22:13AM +0200, Christian Borntraeger wrote: Am 13.05.2015 um 23:47 schrieb Michael S. Tsirkin: > On Wed, May 13, 2015 at 08:57:00PM +0200, Christian Borntraeger wrote: >> Am 13.05.2015 um 18:14 schrieb Michael S. Tsirkin: - AFAICS, there's no easy way to add transport-specific subsections - and simply adding config_vector in ccw would break compatibility >>> >>> subsections break compatibility too. The only way around that is to set >>> a flag to skip migrating config_vector for old machine types. >> >> My main concern is about undetected compatibility issues. A subsection >> will >> tell the user that something went wrong. What happens if we just add a >> new >> qemu_put_byte in the stream. Will the savevm core always detect that we >> have >> too many or not enough bytes? If yes, adding new stuff in the stream will >> always be detected in some way as error we can go with just adding >> qemu_put_be16/qemu_get_be16 in >> virtio_ccw_save_config/virtio_ccw_load_config. >> Old/new QEMUs will then not be compatible - but thats probably ok as >> long as it >> errors out. >> >> My understanding was that we do not have a guarentee that this will be >> detected all the time and having random junk in some variables is a >> debugging >> nightmare. Is that correct? >> >> >> Christian > > It's not too bad - normally there's a bunch of strings that > helps you find out what's going on. > But if you really care about debuggability of migration streams, help move > forward dgilbert's RFC that switched to a self-delimiting format. > Just piling up random hacks in virtio seems like a wrong approach. > Thats not my question. PLEASE try to understand my question. I want a hard stop if migration changes in incompatible ways. If adding a qemu_put_byte in virtio_ccw gets detected we can just fix virtio_ccw AS YOU SUGGESTED. I just want to know if I can rely on that or not. Christian >>> >>> I answered exactly this question but let me try to spell the answer >>> out a bit more. >>> >>> There are three answers: >>> 1. Yes, it's sure to get detected because everything gets shifted >>> and then you get an unexpected string instead of next device name. >> >> Ok got it. So simply adding a qemu_put/get_byte will always fail the >> migration and we >> can just fixup virtio-ccw.c at the cost of being not migrateable between >> versions before/after that change. > > Gahhh! No! Adding an extra byte into the stream causes random horrible > failures > that get very very confusing. Yes, it will probably fail, but how it will > fail and what error you get is just guess work. > (And note, it's strictly a 'probably fail' - if you happen to land with > a zero byte where you expect the start of the next section the migration > code will think it's the end of the migration stream and blissfully start > the CPUs). As Conny is away today, I will drive the dicussion a bit further :-) So we really want a feature that detects this change and prevents migration. I think its totally fine to not be able to migrate between todays QEMUs and a fixed version for s390 as there no supported environment today I am aware of. What would be the preferred way to go? a: Connies approach of a subsection that is only migrated if necessary (config vector != 0x) b: change virtio-ccw.c with put/get_be16 and make a new version of the s390-ccw machine? The old version will set a property to not migrate the config vector. (like Michaels 2nd suggestion) c: ? Christian
Re: [Qemu-devel] [PATCH RFC 1/1] virtio: migrate config_vector
On Fri, May 15, 2015 at 09:08:07AM +0200, Christian Borntraeger wrote: > Am 14.05.2015 um 19:00 schrieb Dr. David Alan Gilbert: > > * Christian Borntraeger (borntrae...@de.ibm.com) wrote: > >> Am 14.05.2015 um 11:36 schrieb Michael S. Tsirkin: > >>> On Thu, May 14, 2015 at 11:22:13AM +0200, Christian Borntraeger wrote: > Am 13.05.2015 um 23:47 schrieb Michael S. Tsirkin: > > On Wed, May 13, 2015 at 08:57:00PM +0200, Christian Borntraeger wrote: > >> Am 13.05.2015 um 18:14 schrieb Michael S. Tsirkin: > - AFAICS, there's no easy way to add transport-specific subsections - > and simply adding config_vector in ccw would break compatibility > >>> > >>> subsections break compatibility too. The only way around that is to > >>> set > >>> a flag to skip migrating config_vector for old machine types. > >> > >> My main concern is about undetected compatibility issues. A subsection > >> will > >> tell the user that something went wrong. What happens if we just add a > >> new > >> qemu_put_byte in the stream. Will the savevm core always detect that > >> we have > >> too many or not enough bytes? If yes, adding new stuff in the stream > >> will > >> always be detected in some way as error we can go with just adding > >> qemu_put_be16/qemu_get_be16 in > >> virtio_ccw_save_config/virtio_ccw_load_config. > >> Old/new QEMUs will then not be compatible - but thats probably ok as > >> long as it > >> errors out. > >> > >> My understanding was that we do not have a guarentee that this will be > >> detected all the time and having random junk in some variables is a > >> debugging > >> nightmare. Is that correct? > >> > >> > >> Christian > > > > It's not too bad - normally there's a bunch of strings that > > helps you find out what's going on. > > But if you really care about debuggability of migration streams, help > > move > > forward dgilbert's RFC that switched to a self-delimiting format. > > Just piling up random hacks in virtio seems like a wrong approach. > > > > Thats not my question. PLEASE try to understand my question. > I want a hard stop if migration changes in incompatible ways. > If adding a qemu_put_byte in virtio_ccw gets detected we can just fix > virtio_ccw AS YOU SUGGESTED. I just want to know if I can rely on that > or not. > > Christian > >>> > >>> I answered exactly this question but let me try to spell the answer > >>> out a bit more. > >>> > >>> There are three answers: > >>> 1. Yes, it's sure to get detected because everything gets shifted > >>> and then you get an unexpected string instead of next device name. > >> > >> Ok got it. So simply adding a qemu_put/get_byte will always fail the > >> migration and we > >> can just fixup virtio-ccw.c at the cost of being not migrateable between > >> versions before/after that change. > > > > Gahhh! No! Adding an extra byte into the stream causes random horrible > > failures > > that get very very confusing. Yes, it will probably fail, but how it will > > fail and what error you get is just guess work. > > (And note, it's strictly a 'probably fail' - if you happen to land with > > a zero byte where you expect the start of the next section the migration > > code will think it's the end of the migration stream and blissfully start > > the CPUs). > > As Conny is away today, I will drive the dicussion a bit further :-) > So we really want a feature that detects this change and prevents migration. > I think its totally fine to not be able to migrate between todays QEMUs and > a fixed version for s390 as there no supported environment today I am aware > of. What would be the preferred way to go? > a: Connies approach of a subsection that is only migrated if necessary > (config vector != 0x) > b: change virtio-ccw.c with put/get_be16 and make a new version of the > s390-ccw machine? The old version will set a property to not migrate the > config > vector. (like Michaels 2nd suggestion) > c: ? c. Set old machine as non-migrateable. > Christian I prefer b or c.
Re: [Qemu-devel] [PATCH v3] mirror: correct buf_size
On 15/05/2015 08:53, Wen Congyang wrote:
> On 05/15/2015 02:45 PM, Paolo Bonzini wrote:
>>
>>
>> On 15/05/2015 03:37, Wen Congyang wrote:
>>> If bus_size is less than 0, the command fails.
>>> If buf_size % granularity is not 0, mirror_free_init() will
>>> do dangerous things.
>>>
>>> Signed-off-by: Wen Congyang
>>> ---
>>> block/mirror.c | 6 +-
>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/block/mirror.c b/block/mirror.c
>>> index 58f391a..7732f8b 100644
>>> --- a/block/mirror.c
>>> +++ b/block/mirror.c
>>> @@ -671,6 +671,10 @@ static void mirror_start_job(BlockDriverState *bs,
>>> BlockDriverState *target,
>>> return;
>>> }
>>>
>>> +if (buf_size < 0) {
>>
>> This should be <=.
>
> But commit_active_start() passes 0...
This was changed to granularity by MAX, while it would be 0 after
ROUND_UP. The right value would be DEFAULT_MIRROR_BUF_SIZE.
block_commit should have an optional buf_size, but for now I guess you
can change a 0 to DEFAULT_MIRROR_BUF_SIZE in mirror_start_job, and let
qmp_drive_mirror do this:
if (!has_buf_size) {
-buf_size = DEFAULT_MIRROR_BUF_SIZE;
+buf_size = 0;
}
Paolo
> Thanks
> Wen Congyang
>
>>
>> Jeff, can you adjust when committing?
>>
>> Paolo
>>
>>> +error_setg(errp, "Invalid parameter '%s'", "buf-size");
>>> +return;
>>> +}
>>>
>>> s = block_job_create(driver, bs, speed, cb, opaque, errp);
>>> if (!s) {
>>> @@ -684,7 +688,7 @@ static void mirror_start_job(BlockDriverState *bs,
>>> BlockDriverState *target,
>>> s->is_none_mode = is_none_mode;
>>> s->base = base;
>>> s->granularity = granularity;
>>> -s->buf_size = MAX(buf_size, granularity);
>>> +s->buf_size = ROUND_UP(buf_size, granularity);
>>>
>>> s->dirty_bitmap = bdrv_create_dirty_bitmap(bs, granularity, NULL,
>>> errp);
>>> if (!s->dirty_bitmap) {
>>>
>> .
>>
>
>
>
Re: [Qemu-devel] [PATCH v3 12/12] virtio-scsi-dataplane: Add "device IO" op blocker listener
On 15/05/2015 09:03, Fam Zheng wrote:
> On Fri, 05/15 08:50, Paolo Bonzini wrote:
>>
>>
>> On 15/05/2015 08:04, Fam Zheng wrote:
>>> @@ -111,6 +111,10 @@ static void
>>> virtio_scsi_iothread_handle_ctrl(EventNotifier *notifier)
>>> VirtIOSCSI *s = VIRTIO_SCSI(vring->parent);
>>> VirtIOSCSIReq *req;
>>>
>>> +if (s->pause_counter) {
>>> +virtio_scsi_stop_ioeventfd(s);
>>> +return;
>>> +}
>>> event_notifier_test_and_clear(notifier);
>>> while ((req = virtio_scsi_pop_req_vring(s, vring))) {
>>> virtio_scsi_handle_ctrl_req(s, req);
>>> @@ -124,6 +128,10 @@ static void
>>> virtio_scsi_iothread_handle_event(EventNotifier *notifier)
>>> VirtIOSCSI *s = vring->parent;
>>> VirtIODevice *vdev = VIRTIO_DEVICE(s);
>>>
>>> +if (s->pause_counter) {
>>> +virtio_scsi_stop_ioeventfd(s);
>>> +return;
>>> +}
>>> event_notifier_test_and_clear(notifier);
>>>
>>> if (!(vdev->status & VIRTIO_CONFIG_S_DRIVER_OK)) {
>>
>> Why are these needed?
>
> Not strictly. I think it's something like an "assert(!s->pause_counter)" but
> said gently.
Why be gentle? :)
Paolo
Re: [Qemu-devel] Kernel Panic on Yum update
On 15.05.2015 08:51, Paolo Bonzini wrote: On 15/05/2015 08:34, Gerhard Wiesinger wrote: Helllo, I'm using latest qemu-kvm-2.3.0-3.fc21.x86_64 from libvirt repository (updated afterwards). Running yum regularly crashes the VM like below. VM is stripped down to minimum memory requirements (256MB) for owncloud. See below. Looks like a problem in virtio-net or a kernel bug with low memory situations. You're simply running out of memory. Yum can be a memory hog. Yes, yum takes memory. But there is ~2.2 GB virt memory available. That should be enough. Therefore I think it is a kernel problem. As in previous crashes on the mailing list there is a lot of swap available (2GB) which isn't touched in ANY way. Under normal conditions without yum: free totalusedfree shared buff/cache available Mem: 243036 132540 12716 15520 97780 74964 Swap: 2064380 0 2064380 And running it after reboot works fine (with all the services up). Ciao, Gerhard
[Qemu-devel] [PATCH v5] mirror: correct buf_size
If bus_size is less than 0, the command fails.
If buf_size is 0, use DEFAULT_MIRROR_BUF_SIZE.
If buf_size % granularity is not 0, mirror_free_init() will
do dangerous things.
Signed-off-by: Wen Congyang
Reviewed-by: Fam Zheng
---
block/mirror.c | 11 ++-
blockdev.c | 4 +---
2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/block/mirror.c b/block/mirror.c
index 58f391a..25645ef 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -19,6 +19,7 @@
#define SLICE_TIME1ULL /* ns */
#define MAX_IN_FLIGHT 16
+#define DEFAULT_MIRROR_BUF_SIZE (10 << 20)
/* The mirroring buffer is a list of granularity-sized chunks.
* Free chunks are organized in a list.
@@ -671,6 +672,14 @@ static void mirror_start_job(BlockDriverState *bs,
BlockDriverState *target,
return;
}
+if (buf_size < 0) {
+error_setg(errp, "Invalid parameter 'buf-size'");
+return;
+}
+
+if (buf_size == 0) {
+buf_size = DEFAULT_MIRROR_BUF_SIZE;
+}
s = block_job_create(driver, bs, speed, cb, opaque, errp);
if (!s) {
@@ -684,7 +693,7 @@ static void mirror_start_job(BlockDriverState *bs,
BlockDriverState *target,
s->is_none_mode = is_none_mode;
s->base = base;
s->granularity = granularity;
-s->buf_size = MAX(buf_size, granularity);
+s->buf_size = ROUND_UP(buf_size, granularity);
s->dirty_bitmap = bdrv_create_dirty_bitmap(bs, granularity, NULL, errp);
if (!s->dirty_bitmap) {
diff --git a/blockdev.c b/blockdev.c
index 5eaf77e..95a0c6a 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2619,8 +2619,6 @@ out:
aio_context_release(aio_context);
}
-#define DEFAULT_MIRROR_BUF_SIZE (10 << 20)
-
void qmp_drive_mirror(const char *device, const char *target,
bool has_format, const char *format,
bool has_node_name, const char *node_name,
@@ -2661,7 +2659,7 @@ void qmp_drive_mirror(const char *device, const char
*target,
granularity = 0;
}
if (!has_buf_size) {
-buf_size = DEFAULT_MIRROR_BUF_SIZE;
+buf_size = 0;
}
if (granularity != 0 && (granularity < 512 || granularity > 1048576 * 64))
{
--
2.1.0
Re: [Qemu-devel] [PATCH] Do not emulate a floppy drive when -nodefaults
John Snow writes: > On 05/14/2015 10:07 AM, Michael S. Tsirkin wrote: >> On Thu, May 14, 2015 at 02:02:04PM +0200, Markus Armbruster wrote: >>> Correct. >>> >>> Here's how I think it should be done: >>> >>> * Create a machine option to control the FDC >>> >>> This is a machine-specific option. It should only exist for machine >>> types that have an optional FDC. >>> >>> Default must be "on" for old machine types. Default may be "off" for >>> new machine types. >>> >>> It should certainly be off for pc-q35-2.4 and newer. Real Q35 boards >>> commonly don't have an FDC (depends on the Super I/O chip used). >>> >>> We may want to keep it off for pc-i440fx-2.4 and newer. I doubt >>> there's a real i440FX without an FDC, but our virtual i440FX is quite >>> unlike a real one in other ways already. >> >> I think making it off by default is a bad idea, it will break >> command-line users. >> >> > > If we can add a flag to disable it, I still think I wouldn't mind that, > if it could be worked out to not be hacky and gross. > >>> * Create the FDC only if the option is "on". >>> >>> * Optional: make -drive if=floppy,... auto-enable it >> >> Every time we do such auto hacks, we regret this later. >> Just do what we are told, fail if=floppy if disabled. >> > > I agree very much. Just because the current drive/device syntax is > almost totally hosed doesn't mean we should put more wood on the fire. > >>> I wouldn't bother doing the same for -global isa-fdc.driveA=... and >>> such. >>> >>> Stefano, if you're willing to tackle this, go right ahead! > > > I'm definitely against a "--seriously-nothing" flag because the line for > what is embedded or not is fuzzy. Paolo raises some good points against > where you draw the line for what we decide to allow users to > include/exclude that is otherwise considered part of the board. --nodefaults must continue to disable all optional parts of the board. What exactly is optional is for the board / machine type to define. It can't be changed once the machine type is released. When in doubt, make it optional. Especially when the device has user-configurable properties, because optional devices are much nicer to configure than onboard devices. For an onboard device, you have to mess with -global, e.g. -global isa-fdc.driveA=fda Since -global applies to *all* devices of a kind, this has unwanted side effects when you have more than one. For instance: $ qemu-system-x86 -nodefaults -S -display none -drive if=none,id=fda -global isa-fdc.driveA=fda -device isa-fdc,iobase=370 qemu-system-x86: -device isa-fdc,iobase=370: Warning: global isa-fdc.driveA=fda ignored (Property 'isa-fdc.driveA' can't take value 'fda', it's in use) If it was optional, you'd do a perfectly regular -device isa-fdc,id=fdc0,driveA=floppyA which doesn't mess up any subsequent -device isa-fdc. Since Q35 is just starting to become migratable, the time to painlessly change its optional parts is *now*. For i440FX, we'll have to sacrifice some to the compatibility idols. > Still, given the hype train, if there is an API we could introduce that > is likely not to make our code gross (or make us belly-ache about how > dumb we were in 5 years) that disables the FDC, I don't think I would > mind terribly. I'll leave that to minds more opinionated than mine to > hash out, though. > > Maybe the best option here really is to carefully separate optional from > non-optional components (FDC vs. Floppy Drive, Floppy Disk code) and > just give the core FDC code a good scrubbing. In my not particularly humble opinion, time spent on FDC code is time stolen from more useful matters.
[Qemu-devel] PING: [PATCH v3 0/3] virtio-mmio: introduce eventfd support
Hello! I have published v3 3 days ago, and got no single reply. Was it lost? I've (hopefully) done everything you asked for, and sent it using git send-email from the command line. So should work. Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia
Re: [Qemu-devel] [PATCH v3 12/12] virtio-scsi-dataplane: Add "device IO" op blocker listener
On Fri, 05/15 09:26, Paolo Bonzini wrote:
>
>
> On 15/05/2015 09:03, Fam Zheng wrote:
> > On Fri, 05/15 08:50, Paolo Bonzini wrote:
> >>
> >>
> >> On 15/05/2015 08:04, Fam Zheng wrote:
> >>> @@ -111,6 +111,10 @@ static void
> >>> virtio_scsi_iothread_handle_ctrl(EventNotifier *notifier)
> >>> VirtIOSCSI *s = VIRTIO_SCSI(vring->parent);
> >>> VirtIOSCSIReq *req;
> >>>
> >>> +if (s->pause_counter) {
> >>> +virtio_scsi_stop_ioeventfd(s);
> >>> +return;
> >>> +}
> >>> event_notifier_test_and_clear(notifier);
> >>> while ((req = virtio_scsi_pop_req_vring(s, vring))) {
> >>> virtio_scsi_handle_ctrl_req(s, req);
> >>> @@ -124,6 +128,10 @@ static void
> >>> virtio_scsi_iothread_handle_event(EventNotifier *notifier)
> >>> VirtIOSCSI *s = vring->parent;
> >>> VirtIODevice *vdev = VIRTIO_DEVICE(s);
> >>>
> >>> +if (s->pause_counter) {
> >>> +virtio_scsi_stop_ioeventfd(s);
> >>> +return;
> >>> +}
> >>> event_notifier_test_and_clear(notifier);
> >>>
> >>> if (!(vdev->status & VIRTIO_CONFIG_S_DRIVER_OK)) {
> >>
> >> Why are these needed?
> >
> > Not strictly. I think it's something like an "assert(!s->pause_counter)" but
> > said gently.
>
> Why be gentle? :)
I guess because I want to be a gentleman.. :)
But since being gentle to these two won't make me a fortune to become a
gentlemen, let's just assert!
Fam
Re: [Qemu-devel] [PATCH 0/3] scripts/qemu-gdb: Add event tracing support
Since you're touching qemu-gdb.py anyway, could you stick in a brief comment explaining how to put it to use?
Re: [Qemu-devel] [PATCH] pci: Fix compiler warning (MinGW-w64 gcc 4.9)
Stefan Weil writes:
> i686-w64-mingw32-gcc 4.9.1 from Debian Jessie complains:
>
> hw/pci/pci.c:938:29: warning:
> array subscript is above array bounds [-Warray-bounds]
>
> Using g_assert instead of assert fixes this warning.
>
> Signed-off-by: Stefan Weil
> ---
> hw/pci/pci.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 48f19a3..34f71dc 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -927,8 +927,8 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num,
> uint64_t wmask;
> pcibus_t size = memory_region_size(memory);
>
> -assert(region_num >= 0);
> -assert(region_num < PCI_NUM_REGIONS);
> +g_assert(region_num >= 0);
> +g_assert(region_num < PCI_NUM_REGIONS);
> if (size & (size-1)) {
> fprintf(stderr, "ERROR: PCI region size must be pow2 "
> "type=0x%x, size=0x%"FMT_PCIBUS"\n", type, size);
This is obviously a bug in that version of MinGW. Have you reported it?
Do we really want to work around it?
Re: [Qemu-devel] [Qemu-block] [PATCH 1/2] block: Detect multiplication overflow in bdrv_getlength
On Fri 15 May 2015 03:39:10 AM CEST, Fam Zheng wrote: > int64_t ret = bdrv_nb_sectors(bs); > > +ret = (int64_t)(ret * BDRV_SECTOR_SIZE) < 0 ? -EFBIG : ret; > return ret < 0 ? ret : ret * BDRV_SECTOR_SIZE; Maybe in this case you're safe, but in general there's no guarantee that if there's an overflow the result will be negative. You can do something like this instead: ret = ret > INT64_MAX / BDRV_SECTOR_SIZE ? -EFBIG : ret; Of course this is only valid if BDRV_SECTOR_SIZE != 0 ;) Berto
Re: [Qemu-devel] [Qemu-block] [PATCH 2/2] qemu-iotests: qemu-img info on afl VMDK image with a huge capacity
On Fri 15 May 2015 03:39:11 AM CEST, Fam Zheng wrote: > The image is contributed by Richard W.M. Jones. > > Cc: Richard W.M. Jones > Signed-off-by: Fam Zheng Reviewed-by: Alberto Garcia Berto
Re: [Qemu-devel] Kernel Panic on Yum update
On 15/05/2015 09:37, Gerhard Wiesinger wrote: >> > > Yes, yum takes memory. But there is ~2.2 GB virt memory available. That > should be enough. Therefore I think it is a kernel problem. As in > previous crashes on the mailing list there is a lot of swap available > (2GB) which isn't touched in ANY way. > > Under normal conditions without yum: > free > totalusedfree shared buff/cache > available > Mem: 243036 132540 12716 15520 97780 74964 > Swap: 2064380 0 2064380 Not all memory is the same. Some memory cannot be swapped, and some memory can be swapped to disk without a swap file (e.g. executables). Failing an order 0 allocation is weird indeed, but this is a GFP_ATOMIC allocation so it's a bit less weird. Paolo
Re: [Qemu-devel] [PATCH 1/2] block: Detect multiplication overflow in bdrv_getlength
Fam Zheng writes:
> Bogus image may have a large total_sectors that will overflow the
> multiplication. For cleanness, fix the return code so the error message
> will be meaningful.
>
> Reported-by: Richard W.M. Jones
> Signed-off-by: Fam Zheng
> ---
> block.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/block.c b/block.c
> index 7904098..5ee3fdf 100644
> --- a/block.c
> +++ b/block.c
> @@ -2330,6 +2330,7 @@ int64_t bdrv_getlength(BlockDriverState *bs)
> {
> int64_t ret = bdrv_nb_sectors(bs);
>
> +ret = (int64_t)(ret * BDRV_SECTOR_SIZE) < 0 ? -EFBIG : ret;
> return ret < 0 ? ret : ret * BDRV_SECTOR_SIZE;
> }
Signed integer overflow is undefined behavior. Your code works just
fine on any remotely sane machine, *except* when the optimizer decides
to use its undefined behavior license to mess with you.
A more prudent way to test for overflow would be something like
ret > INT64_MAX / BDRV_SECTOR_SIZE
Re: [Qemu-devel] [PATCH v2] block: Let bdrv_drain_all() to call aio_poll() for each AioContext
Am 15.05.2015 um 08:59 schrieb Christian Borntraeger:
> Am 14.05.2015 um 18:03 schrieb Alexander Yarygin:
>> After the commit 9b536adc ("block: acquire AioContext in
>> bdrv_drain_all()") the aio_poll() function got called for every
>> BlockDriverState, in assumption that every device may have its own
>> AioContext. The bdrv_drain_all() function is called in each
>> virtio_reset() call, which in turn is called for every virtio-blk
>> device on initialization, so we got aio_poll() called
>> 'length(device_list)^2' times.
>>
>> If we have thousands of disks attached, there are a lot of
>> BlockDriverStates but only a few AioContexts, leading to tons of
>> unnecessary aio_poll() calls. For example, startup times with 1000 disks
>> takes over 13 minutes.
>>
>> This patch changes the bdrv_drain_all() function allowing it find shared
>> AioContexts and to call aio_poll() only for unique ones. This results in
>> much better startup times, e.g. 1000 disks do come up within 5 seconds.
>>
>> Cc: Christian Borntraeger
>> Cc: Cornelia Huck
>> Cc: Kevin Wolf
>> Cc: Paolo Bonzini
>> Cc: Stefan Hajnoczi
>> Signed-off-by: Alexander Yarygin
>
> Applying on top of 2.3 I can verify the speedup.
> Tested-by: Christian Borntraeger
Hmmm. When I enable iothreads for all of these devices I get hangs. So
lets defer my Tested-by until I understand that :-(
Re: [Qemu-devel] [PATCH] Do not emulate a floppy drive when -nodefaults
On 15/05/2015 09:50, Markus Armbruster wrote: > --nodefaults must continue to disable all optional parts of the board. > > What exactly is optional is for the board / machine type to define. It > can't be changed once the machine type is released. > > When in doubt, make it optional. I agree entirely. This unfortunately applies only to future boards. > Since Q35 is just starting to become migratable, the time to painlessly > change its optional parts is *now*. I can buy this. Let's just not divert attention from more important stuff. I may be more inclined to accept patches, if we add something to our security policy about disliking cute backronyms that attract bad journalism like magnets. And I'm only half joking; more like only 1% joking. Paolo
[Qemu-devel] [PULL 00/26] qapi: Fix qapi mangling of downstream names, and more
The following changes since commit 1eeace9c237a729d11c7acd7c0338ab4562af637: Merge remote-tracking branch 'remotes/agraf/tags/signed-s390-for-upstream' into staging (2015-05-13 16:06:07 +0100) are available in the git repository at: git://repo.or.cz/qemu/armbru.git qapi-next for you to fetch changes up to 4180978c9205c50acd2d6c385def9b3e81911696: qapi: Inline gen_command_decl_prologue(), gen_command_def_prologue() (2015-05-14 18:41:33 +0200) Eduardo Habkost (1): qmp: Add qom_path field to query-cpus command Eric Blake (9): qapi: Rename identical c_fun()/c_var() into c_name() qapi: Tidy c_type() logic qapi: Make c_type() consistently convert qapi names qapi: Support downstream enums qapi: Support downstream structs qapi: Support downstream simple unions qapi: Support downstream flat unions qapi: Support downstream alternates qapi: Support downstream events and commands Markus Armbruster (15): qapi: Fix C identifiers generated for names containing '.' qapi: Rename _generate_enum_string() to camel_to_upper() qapi: Rename generate_enum_full_value() to c_enum_const() qapi: Simplify c_enum_const() qapi: Use c_enum_const() in generate_alternate_qtypes() qapi: Move camel_to_upper(), c_enum_const() to closely related code tests: Add missing dependencies on $(qapi-py) qapi: qapi-event.py option -b does nothing, drop it qapi: qapi-commands.py option --type is unused, drop it qapi: Factor parse_command_line() out of the generators qapi: Fix generators to report command line errors decently qapi: Turn generators' mandatory option -i into an argument qapi: Factor open_output(), close_output() out of generators qapi: Drop pointless flush() before close() qapi: Inline gen_command_decl_prologue(), gen_command_def_prologue() Michael S. Tsirkin (1): doc: fix qmp event type Makefile| 14 +- cpus.c | 1 + docs/qapi-code-gen.txt | 10 +- docs/qmp/qmp-events.txt | 4 +- qapi-schema.json| 8 +- qmp-commands.hx | 7 +- scripts/qapi-commands.py| 236 +++--- scripts/qapi-event.py | 135 + scripts/qapi-types.py | 161 +++-- scripts/qapi-visit.py | 187 +--- scripts/qapi.py | 248 ++-- tests/Makefile | 16 +-- tests/qapi-schema/qapi-schema-test.json | 20 +++ tests/qapi-schema/qapi-schema-test.out | 21 ++- tests/test-qmp-commands.c | 15 ++ 15 files changed, 492 insertions(+), 591 deletions(-) -- 1.9.3
[Qemu-devel] [PULL 08/26] qapi: Use c_enum_const() in generate_alternate_qtypes()
Missed in commit b0b5819.
Signed-off-by: Markus Armbruster
Signed-off-by: Eric Blake
---
scripts/qapi-types.py | 6 ++
scripts/qapi.py | 11 ---
2 files changed, 2 insertions(+), 15 deletions(-)
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 6ca48c1..9eb08a6 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -180,11 +180,9 @@ const int %(name)s_qtypes[QTYPE_MAX] = {
assert qtype, "Invalid alternate member"
ret += mcgen('''
-[ %(qtype)s ] = %(abbrev)s_KIND_%(enum)s,
+[ %(qtype)s ] = %(enum_const)s,
''',
-qtype = qtype,
-abbrev = de_camel_case(name).upper(),
-enum = c_name(de_camel_case(key),False).upper())
+qtype = qtype, enum_const = c_enum_const(name + 'Kind', key))
ret += mcgen('''
};
diff --git a/scripts/qapi.py b/scripts/qapi.py
index b917cad..3757a91 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -729,17 +729,6 @@ def parse_args(typeinfo):
# value of an optional argument.
yield (argname, argentry, optional)
-def de_camel_case(name):
-new_name = ''
-for ch in name:
-if ch.isupper() and new_name:
-new_name += '_'
-if ch == '-':
-new_name += '_'
-else:
-new_name += ch.lower()
-return new_name
-
def camel_case(name):
new_name = ''
first = True
--
1.9.3
[Qemu-devel] [PULL 01/26] qmp: Add qom_path field to query-cpus command
From: Eduardo Habkost
This will allow clients to query additional information directly using
qom-get on the CPU objects.
Reviewed-by: David Gibson
Reviewed-by: Andreas Färber
Signed-off-by: Eduardo Habkost
Reviewed-by: Eric Blake
Signed-off-by: Markus Armbruster
---
cpus.c | 1 +
qapi-schema.json | 8 ++--
qmp-commands.hx | 7 +--
3 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/cpus.c b/cpus.c
index 62d157a..de6469f 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1435,6 +1435,7 @@ CpuInfoList *qmp_query_cpus(Error **errp)
info->value->CPU = cpu->cpu_index;
info->value->current = (cpu == first_cpu);
info->value->halted = cpu->halted;
+info->value->qom_path = object_get_canonical_path(OBJECT(cpu));
info->value->thread_id = cpu->thread_id;
#if defined(TARGET_I386)
info->value->has_pc = true;
diff --git a/qapi-schema.json b/qapi-schema.json
index 9c92482..f97ffa1 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -679,6 +679,8 @@
# @halted: true if the virtual CPU is in the halt state. Halt usually refers
# to a processor specific low power mode.
#
+# @qom_path: path to the CPU object in the QOM tree (since 2.4)
+#
# @pc: #optional If the target is i386 or x86_64, this is the 64-bit
instruction
#pointer.
#If the target is Sparc, this is the PC component of the
@@ -699,8 +701,10 @@
#data is sent to the client, the guest may no longer be halted.
##
{ 'struct': 'CpuInfo',
- 'data': {'CPU': 'int', 'current': 'bool', 'halted': 'bool', '*pc': 'int',
- '*nip': 'int', '*npc': 'int', '*PC': 'int', 'thread_id': 'int'} }
+ 'data': {'CPU': 'int', 'current': 'bool', 'halted': 'bool',
+ 'qom_path': 'str',
+ '*pc': 'int', '*nip': 'int', '*npc': 'int', '*PC': 'int',
+ 'thread_id': 'int'} }
##
# @query-cpus:
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 7506774..14e109e 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2569,6 +2569,7 @@ Return a json-array. Each CPU is represented by a
json-object, which contains:
- "CPU": CPU index (json-int)
- "current": true if this is the current CPU, false otherwise (json-bool)
- "halted": true if the cpu is halted, false otherwise (json-bool)
+- "qom_path": path to the CPU object in the QOM tree (json-str)
- Current program counter. The key's name depends on the architecture:
"pc": i386/x86_64 (json-int)
"nip": PPC (json-int)
@@ -2585,14 +2586,16 @@ Example:
"CPU":0,
"current":true,
"halted":false,
-"pc":3227107138
+"qom_path":"/machine/unattached/device[0]",
+"pc":3227107138,
"thread_id":3134
},
{
"CPU":1,
"current":false,
"halted":true,
-"pc":7108165
+"qom_path":"/machine/unattached/device[2]",
+"pc":7108165,
"thread_id":3135
}
]
--
1.9.3
[Qemu-devel] [PULL 02/26] doc: fix qmp event type
From: "Michael S. Tsirkin"
Event name for hot unplug errors was wrong.
Make doc match code.
Cc: Zhu Guihua
Reported-by: Eric Blake
Signed-off-by: Michael S. Tsirkin
Reviewed-by: Eric Blake
Signed-off-by: Markus Armbruster
---
docs/qmp/qmp-events.txt | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/docs/qmp/qmp-events.txt b/docs/qmp/qmp-events.txt
index 6dc2cca..4c13d48 100644
--- a/docs/qmp/qmp-events.txt
+++ b/docs/qmp/qmp-events.txt
@@ -232,7 +232,7 @@ Example:
{ "event": "GUEST_PANICKED",
"data": { "action": "pause" } }
-MEM_HOT_UNPLUG_ERROR
+MEM_UNPLUG_ERROR
Emitted when memory hot unplug error occurs.
@@ -243,7 +243,7 @@ Data:
Example:
-{ "event": "MEM_HOT_UNPLUG_ERROR"
+{ "event": "MEM_UNPLUG_ERROR"
"data": { "device": "dimm1",
"msg": "acpi: device unplug for unsupported device"
},
--
1.9.3
[Qemu-devel] [PULL 07/26] qapi: Simplify c_enum_const()
Signed-off-by: Markus Armbruster
Signed-off-by: Eric Blake
---
scripts/qapi.py | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 1258f76..b917cad 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -961,6 +961,4 @@ def camel_to_upper(value):
return new_name.lstrip('_').upper()
def c_enum_const(type_name, const_name):
-abbrev_string = camel_to_upper(type_name)
-value_string = camel_to_upper(const_name)
-return "%s_%s" % (abbrev_string, value_string)
+return camel_to_upper(type_name + '_' + const_name)
--
1.9.3
[Qemu-devel] [PULL 05/26] qapi: Rename _generate_enum_string() to camel_to_upper()
Signed-off-by: Markus Armbruster
Signed-off-by: Eric Blake
---
scripts/qapi.py | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index a4701ca..7330f7c 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -538,7 +538,7 @@ def check_union(expr, expr_info):
# Otherwise, check for conflicts in the generated enum
else:
-c_key = _generate_enum_string(key)
+c_key = camel_to_upper(key)
if c_key in values:
raise QAPIExprError(expr_info,
"Union '%s' member '%s' clashes with '%s'"
@@ -556,7 +556,7 @@ def check_alternate(expr, expr_info):
check_name(expr_info, "Member of alternate '%s'" % name, key)
# Check for conflicts in the generated enum
-c_key = _generate_enum_string(key)
+c_key = camel_to_upper(key)
if c_key in values:
raise QAPIExprError(expr_info,
"Alternate '%s' member '%s' clashes with '%s'"
@@ -587,7 +587,7 @@ def check_enum(expr, expr_info):
for member in members:
check_name(expr_info, "Member of enum '%s'" %name, member,
enum_member=True)
-key = _generate_enum_string(member)
+key = camel_to_upper(member)
if key in values:
raise QAPIExprError(expr_info,
"Enum '%s' member '%s' clashes with '%s'"
@@ -941,7 +941,7 @@ def guardend(name):
# ENUMName -> ENUM_NAME, EnumName1 -> ENUM_NAME1
# ENUM_NAME -> ENUM_NAME, ENUM_NAME1 -> ENUM_NAME1, ENUM_Name2 -> ENUM_NAME2
# ENUM24_Name -> ENUM24_NAME
-def _generate_enum_string(value):
+def camel_to_upper(value):
c_fun_str = c_name(value, False)
if value.isupper():
return c_fun_str
@@ -961,6 +961,6 @@ def _generate_enum_string(value):
return new_name.lstrip('_').upper()
def generate_enum_full_value(enum_name, enum_value):
-abbrev_string = _generate_enum_string(enum_name)
-value_string = _generate_enum_string(enum_value)
+abbrev_string = camel_to_upper(enum_name)
+value_string = camel_to_upper(enum_value)
return "%s_%s" % (abbrev_string, value_string)
--
1.9.3
[Qemu-devel] [PULL 26/26] qapi: Inline gen_command_decl_prologue(), gen_command_def_prologue()
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
---
scripts/qapi-commands.py | 58
1 file changed, 24 insertions(+), 34 deletions(-)
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index c3e420e..1c1d3aa 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -309,36 +309,6 @@ qapi_init(qmp_init_marshal);
registry=registry.rstrip())
return ret
-def gen_command_decl_prologue(prefix=""):
-ret = mcgen('''
-#include "%(prefix)sqapi-types.h"
-#include "qapi/qmp/qdict.h"
-#include "qapi/error.h"
-
-''',
- prefix=prefix)
-return ret
-
-def gen_command_def_prologue(prefix="", proxy=False):
-ret = mcgen('''
-#include "qemu-common.h"
-#include "qemu/module.h"
-#include "qapi/qmp/qerror.h"
-#include "qapi/qmp/types.h"
-#include "qapi/qmp/dispatch.h"
-#include "qapi/visitor.h"
-#include "qapi/qmp-output-visitor.h"
-#include "qapi/qmp-input-visitor.h"
-#include "qapi/dealloc-visitor.h"
-#include "%(prefix)sqapi-types.h"
-#include "%(prefix)sqapi-visit.h"
-
-''',
-prefix=prefix)
-if not proxy:
-ret += '#include "%sqmp-commands.h"' % prefix
-return ret + "\n\n"
-
middle_mode = False
(input_file, output_dir, do_c, do_h, prefix, opts) = \
@@ -385,10 +355,30 @@ h_comment = '''
'qmp-marshal.c', 'qmp-commands.h',
c_comment, h_comment)
-ret = gen_command_decl_prologue(prefix=prefix)
-fdecl.write(ret)
-ret = gen_command_def_prologue(prefix=prefix)
-fdef.write(ret)
+fdef.write(mcgen('''
+#include "qemu-common.h"
+#include "qemu/module.h"
+#include "qapi/qmp/qerror.h"
+#include "qapi/qmp/types.h"
+#include "qapi/qmp/dispatch.h"
+#include "qapi/visitor.h"
+#include "qapi/qmp-output-visitor.h"
+#include "qapi/qmp-input-visitor.h"
+#include "qapi/dealloc-visitor.h"
+#include "%(prefix)sqapi-types.h"
+#include "%(prefix)sqapi-visit.h"
+#include "%(prefix)sqmp-commands.h"
+
+''',
+prefix=prefix))
+
+fdecl.write(mcgen('''
+#include "%(prefix)sqapi-types.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/error.h"
+
+''',
+ prefix=prefix))
for cmd in commands:
arglist = []
--
1.9.3
[Qemu-devel] [PULL 20/26] qapi: qapi-commands.py option --type is unused, drop it
Anything but --type sync (which is the default) suppresses output
entirely, which makes no sense.
Dates back to the initial commit c17d990. Commit message says
"Currently only generators for synchronous qapi/qmp functions are
supported", so maybe output other than "synchronous qapi/qmp" was
planned at the time, to be selected with --type.
Should other kinds of output ever materialize, we can put the option
back.
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
---
scripts/qapi-commands.py | 66 +++-
1 file changed, 31 insertions(+), 35 deletions(-)
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 0a1d636..c94a19b 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -381,14 +381,13 @@ try:
opts, args = getopt.gnu_getopt(sys.argv[1:], "chp:i:o:m",
["source", "header", "prefix=",
"input-file=", "output-dir=",
-"type=", "middle"])
+"middle"])
except getopt.GetoptError, err:
print str(err)
sys.exit(1)
output_dir = ""
prefix = ""
-dispatch_type = "sync"
c_file = 'qmp-marshal.c'
h_file = 'qmp-commands.h'
middle_mode = False
@@ -403,8 +402,6 @@ for o, a in opts:
input_file = a
elif o in ("-o", "--output-dir"):
output_dir = a + "/"
-elif o in ("-t", "--type"):
-dispatch_type = a
elif o in ("-m", "--middle"):
middle_mode = True
elif o in ("-c", "--source"):
@@ -436,40 +433,39 @@ exprs = parse_schema(input_file)
commands = filter(lambda expr: expr.has_key('command'), exprs)
commands = filter(lambda expr: not expr.has_key('gen'), commands)
-if dispatch_type == "sync":
-fdecl = maybe_open(do_h, h_file, 'w')
-fdef = maybe_open(do_c, c_file, 'w')
-ret = gen_command_decl_prologue(header=basename(h_file),
guard=guardname(h_file), prefix=prefix)
+fdecl = maybe_open(do_h, h_file, 'w')
+fdef = maybe_open(do_c, c_file, 'w')
+ret = gen_command_decl_prologue(header=basename(h_file),
guard=guardname(h_file), prefix=prefix)
+fdecl.write(ret)
+ret = gen_command_def_prologue(prefix=prefix)
+fdef.write(ret)
+
+for cmd in commands:
+arglist = []
+ret_type = None
+if cmd.has_key('data'):
+arglist = cmd['data']
+if cmd.has_key('returns'):
+ret_type = cmd['returns']
+ret = generate_command_decl(cmd['command'], arglist, ret_type) + "\n"
fdecl.write(ret)
-ret = gen_command_def_prologue(prefix=prefix)
-fdef.write(ret)
-
-for cmd in commands:
-arglist = []
-ret_type = None
-if cmd.has_key('data'):
-arglist = cmd['data']
-if cmd.has_key('returns'):
-ret_type = cmd['returns']
-ret = generate_command_decl(cmd['command'], arglist, ret_type) + "\n"
-fdecl.write(ret)
-if ret_type:
-ret = gen_marshal_output(cmd['command'], arglist, ret_type,
middle_mode) + "\n"
-fdef.write(ret)
+if ret_type:
+ret = gen_marshal_output(cmd['command'], arglist, ret_type,
middle_mode) + "\n"
+fdef.write(ret)
-if middle_mode:
-fdecl.write('%s;\n' % gen_marshal_input_decl(cmd['command'],
arglist, ret_type, middle_mode))
+if middle_mode:
+fdecl.write('%s;\n' % gen_marshal_input_decl(cmd['command'], arglist,
ret_type, middle_mode))
-ret = gen_marshal_input(cmd['command'], arglist, ret_type,
middle_mode) + "\n"
-fdef.write(ret)
+ret = gen_marshal_input(cmd['command'], arglist, ret_type, middle_mode) +
"\n"
+fdef.write(ret)
-fdecl.write("\n#endif\n");
+fdecl.write("\n#endif\n");
-if not middle_mode:
-ret = gen_registry(commands)
-fdef.write(ret)
+if not middle_mode:
+ret = gen_registry(commands)
+fdef.write(ret)
-fdef.flush()
-fdef.close()
-fdecl.flush()
-fdecl.close()
+fdef.flush()
+fdef.close()
+fdecl.flush()
+fdecl.close()
--
1.9.3
[Qemu-devel] [PULL 04/26] qapi: Rename identical c_fun()/c_var() into c_name()
From: Eric Blake
Now that the two functions are identical, we only need one of them,
and we might as well give it a more descriptive name. Basically,
the function serves as the translation from a QAPI name into a
(portion of a) C identifier, without regards to whether it is a
variable or function name.
Signed-off-by: Eric Blake
Signed-off-by: Markus Armbruster
---
scripts/qapi-commands.py | 35 ++-
scripts/qapi-event.py| 10 +-
scripts/qapi-types.py| 8
scripts/qapi-visit.py| 10 +-
scripts/qapi.py | 11 ---
5 files changed, 36 insertions(+), 38 deletions(-)
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 93e43f0..8c125ca 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -31,12 +31,13 @@ def generate_command_decl(name, args, ret_type):
for argname, argtype, optional in parse_args(args):
argtype = c_type(argtype, is_param=True)
if optional:
-arglist += "bool has_%s, " % c_var(argname)
-arglist += "%s %s, " % (argtype, c_var(argname))
+arglist += "bool has_%s, " % c_name(argname)
+arglist += "%s %s, " % (argtype, c_name(argname))
return mcgen('''
%(ret_type)s qmp_%(name)s(%(args)sError **errp);
''',
- ret_type=c_type(ret_type), name=c_fun(name),
args=arglist).strip()
+ ret_type=c_type(ret_type), name=c_name(name),
+ args=arglist).strip()
def gen_err_check(errvar):
if errvar:
@@ -55,14 +56,14 @@ def gen_sync_call(name, args, ret_type, indent=0):
retval = "retval = "
for argname, argtype, optional in parse_args(args):
if optional:
-arglist += "has_%s, " % c_var(argname)
-arglist += "%s, " % (c_var(argname))
+arglist += "has_%s, " % c_name(argname)
+arglist += "%s, " % (c_name(argname))
push_indent(indent)
ret = mcgen('''
%(retval)sqmp_%(name)s(%(args)s&local_err);
''',
-name=c_fun(name), args=arglist, retval=retval).rstrip()
+name=c_name(name), args=arglist, retval=retval).rstrip()
if ret_type:
ret += "\n" + gen_err_check('local_err')
ret += "\n" + mcgen(
@@ -76,7 +77,7 @@ def gen_sync_call(name, args, ret_type, indent=0):
def gen_marshal_output_call(name, ret_type):
if not ret_type:
return ""
-return "qmp_marshal_output_%s(retval, ret, &local_err);" % c_fun(name)
+return "qmp_marshal_output_%s(retval, ret, &local_err);" % c_name(name)
def gen_visitor_input_containers_decl(args, obj):
ret = ""
@@ -101,17 +102,17 @@ def gen_visitor_input_vars_decl(args):
ret += mcgen('''
bool has_%(argname)s = false;
''',
- argname=c_var(argname))
+ argname=c_name(argname))
if is_c_ptr(argtype):
ret += mcgen('''
%(argtype)s %(argname)s = NULL;
''',
- argname=c_var(argname), argtype=c_type(argtype))
+ argname=c_name(argname), argtype=c_type(argtype))
else:
ret += mcgen('''
%(argtype)s %(argname)s = {0};
''',
- argname=c_var(argname), argtype=c_type(argtype))
+ argname=c_name(argname), argtype=c_type(argtype))
pop_indent()
return ret.rstrip()
@@ -144,17 +145,17 @@ v = qmp_input_get_visitor(mi);
ret += mcgen('''
visit_optional(v, &has_%(c_name)s, "%(name)s", %(errp)s);
''',
- c_name=c_var(argname), name=argname, errp=errparg)
+ c_name=c_name(argname), name=argname, errp=errparg)
ret += gen_err_check(errarg)
ret += mcgen('''
if (has_%(c_name)s) {
''',
- c_name=c_var(argname))
+ c_name=c_name(argname))
push_indent()
ret += mcgen('''
%(visitor)s(v, &%(c_name)s, "%(name)s", %(errp)s);
''',
- c_name=c_var(argname), name=argname, argtype=argtype,
+ c_name=c_name(argname), name=argname, argtype=argtype,
visitor=type_visitor(argtype), errp=errparg)
ret += gen_err_check(errarg)
if optional:
@@ -198,16 +199,16 @@ out:
qapi_dealloc_visitor_cleanup(md);
}
''',
-c_ret_type=c_type(ret_type), c_name=c_fun(name),
+c_ret_type=c_type(ret_type), c_name=c_name(name),
visitor=type_visitor(ret_type))
return ret
def gen_marshal_input_decl(name, args, ret_type, middle_mode):
if middle_mode:
-return 'int qmp_marshal_input_%s(Monitor *mon, const QDict *qdict,
QObject **ret)' % c_fun(name)
+return 'int qmp_marshal_input_%s(Monitor *mon, const QDict *qdict,
QObject **ret)' % c_name(name)
else:
-return 'static void qmp_marshal_input_%s(QDict *args, QObject **ret,
Error **errp)' %
[Qemu-devel] [PULL 14/26] qapi: Support downstream simple unions
From: Eric Blake
Enhance the testsuite to cover downstream simple unions, including
when a union branch is a downstream name. Update the generator to
mangle the union names in the appropriate places.
Signed-off-by: Eric Blake
Signed-off-by: Markus Armbruster
---
scripts/qapi-types.py | 2 +-
scripts/qapi-visit.py | 8
tests/qapi-schema/qapi-schema-test.json | 1 +
tests/qapi-schema/qapi-schema-test.out | 6 --
4 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 5118151..5b0bc5d 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -193,7 +193,7 @@ const int %(name)s_qtypes[QTYPE_MAX] = {
def generate_union(expr, meta):
-name = expr[meta]
+name = c_name(expr[meta])
typeinfo = expr['data']
base = expr.get('base')
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index b500724..d1ec70b 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -255,9 +255,9 @@ def generate_visit_union(expr):
disc_type = enum_define['enum_name']
else:
# There will always be a discriminator in the C switch code, by default
-# it is an enum type generated silently as "'%sKind' % (name)"
-ret = generate_visit_enum('%sKind' % name, members.keys())
-disc_type = '%sKind' % (name)
+# it is an enum type generated silently
+ret = generate_visit_enum(name + 'Kind', members.keys())
+disc_type = c_name(name) + 'Kind'
if base:
assert discriminator
@@ -281,7 +281,7 @@ void visit_type_%(name)s(Visitor *m, %(name)s **obj, const
char *name, Error **e
}
if (*obj) {
''',
- name=name)
+ name=c_name(name))
if base:
ret += mcgen('''
diff --git a/tests/qapi-schema/qapi-schema-test.json
b/tests/qapi-schema/qapi-schema-test.json
index ca9b34c..6416d85 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -114,3 +114,4 @@
'data': { '__org.qemu_x-member1': '__org.qemu_x-Enum' } }
{ 'struct': '__org.qemu_x-Struct', 'base': '__org.qemu_x-Base',
'data': { '__org.qemu_x-member2': 'str' } }
+{ 'union': '__org.qemu_x-Union1', 'data': { '__org.qemu_x-branch': 'str' } }
diff --git a/tests/qapi-schema/qapi-schema-test.out
b/tests/qapi-schema/qapi-schema-test.out
index 6ab4f00..f9ebe08 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -25,11 +25,13 @@
OrderedDict([('event', 'EVENT_D'), ('data', OrderedDict([('a',
'EventStructOne'), ('b', 'str'), ('*c', 'str'), ('*enum3', 'EnumOne')]))]),
OrderedDict([('enum', '__org.qemu_x-Enum'), ('data',
['__org.qemu_x-value'])]),
OrderedDict([('struct', '__org.qemu_x-Base'), ('data',
OrderedDict([('__org.qemu_x-member1', '__org.qemu_x-Enum')]))]),
- OrderedDict([('struct', '__org.qemu_x-Struct'), ('base',
'__org.qemu_x-Base'), ('data', OrderedDict([('__org.qemu_x-member2',
'str')]))])]
+ OrderedDict([('struct', '__org.qemu_x-Struct'), ('base',
'__org.qemu_x-Base'), ('data', OrderedDict([('__org.qemu_x-member2',
'str')]))]),
+ OrderedDict([('union', '__org.qemu_x-Union1'), ('data',
OrderedDict([('__org.qemu_x-branch', 'str')]))])]
[{'enum_name': 'EnumOne', 'enum_values': ['value1', 'value2', 'value3']},
{'enum_name': '__org.qemu_x-Enum', 'enum_values': ['__org.qemu_x-value']},
{'enum_name': 'UserDefAlternateKind', 'enum_values': None},
- {'enum_name': 'UserDefNativeListUnionKind', 'enum_values': None}]
+ {'enum_name': 'UserDefNativeListUnionKind', 'enum_values': None},
+ {'enum_name': '__org.qemu_x-Union1Kind', 'enum_values': None}]
[OrderedDict([('struct', 'NestedEnumsOne'), ('data', OrderedDict([('enum1',
'EnumOne'), ('*enum2', 'EnumOne'), ('enum3', 'EnumOne'), ('*enum4',
'EnumOne')]))]),
OrderedDict([('struct', 'UserDefZero'), ('data', OrderedDict([('integer',
'int')]))]),
OrderedDict([('struct', 'UserDefOne'), ('base', 'UserDefZero'), ('data',
OrderedDict([('string', 'str'), ('*enum1', 'EnumOne')]))]),
--
1.9.3
[Qemu-devel] [PULL 13/26] qapi: Support downstream structs
From: Eric Blake
Enhance the testsuite to cover downstream structs, including struct
members and base structs. Update the generator to mangle the
struct names in the appropriate places.
Signed-off-by: Eric Blake
Signed-off-by: Markus Armbruster
---
scripts/qapi-types.py | 4 ++--
scripts/qapi-visit.py | 11 ++-
tests/qapi-schema/qapi-schema-test.json | 4
tests/qapi-schema/qapi-schema-test.out | 8 ++--
4 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 1593fc6..5118151 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -45,7 +45,7 @@ typedef struct %(name)sList
struct %(name)sList *next;
} %(name)sList;
''',
- name=name)
+ name=c_name(name))
def generate_fwd_enum_struct(name, members):
return mcgen('''
@@ -87,7 +87,7 @@ def generate_struct(expr):
struct %(name)s
{
''',
- name=structname)
+ name=c_name(structname))
if base:
ret += generate_struct_fields({'base': base})
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 7697ec6..b500724 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -56,7 +56,7 @@ static void visit_type_%(name)s_fields(Visitor *m, %(name)s
**obj, Error **errp)
{
Error *err = NULL;
''',
- name=name)
+ name=c_name(name))
push_indent()
if base:
@@ -111,16 +111,16 @@ def generate_visit_struct_body(name, members):
ret = mcgen('''
Error *err = NULL;
-visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(name)s),
&err);
+visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(c_name)s),
&err);
if (!err) {
if (*obj) {
-visit_type_%(name)s_fields(m, obj, errp);
+visit_type_%(c_name)s_fields(m, obj, errp);
}
visit_end_struct(m, &err);
}
error_propagate(errp, err);
''',
-name=name)
+name=name, c_name=c_name(name))
return ret
@@ -137,7 +137,7 @@ def generate_visit_struct(expr):
void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error
**errp)
{
''',
-name=name)
+ name=c_name(name))
ret += generate_visit_struct_body(name, members)
@@ -347,6 +347,7 @@ out:
def generate_declaration(name, members, builtin_type=False):
ret = ""
if not builtin_type:
+name = c_name(name)
ret += mcgen('''
void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error
**errp);
diff --git a/tests/qapi-schema/qapi-schema-test.json
b/tests/qapi-schema/qapi-schema-test.json
index 5f9af66..ca9b34c 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -110,3 +110,7 @@
# test that we correctly compile downstream extensions
{ 'enum': '__org.qemu_x-Enum', 'data': [ '__org.qemu_x-value' ] }
+{ 'struct': '__org.qemu_x-Base',
+ 'data': { '__org.qemu_x-member1': '__org.qemu_x-Enum' } }
+{ 'struct': '__org.qemu_x-Struct', 'base': '__org.qemu_x-Base',
+ 'data': { '__org.qemu_x-member2': 'str' } }
diff --git a/tests/qapi-schema/qapi-schema-test.out
b/tests/qapi-schema/qapi-schema-test.out
index 40f0f20..6ab4f00 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -23,7 +23,9 @@
OrderedDict([('event', 'EVENT_B'), ('data', OrderedDict())]),
OrderedDict([('event', 'EVENT_C'), ('data', OrderedDict([('*a', 'int'),
('*b', 'UserDefOne'), ('c', 'str')]))]),
OrderedDict([('event', 'EVENT_D'), ('data', OrderedDict([('a',
'EventStructOne'), ('b', 'str'), ('*c', 'str'), ('*enum3', 'EnumOne')]))]),
- OrderedDict([('enum', '__org.qemu_x-Enum'), ('data',
['__org.qemu_x-value'])])]
+ OrderedDict([('enum', '__org.qemu_x-Enum'), ('data',
['__org.qemu_x-value'])]),
+ OrderedDict([('struct', '__org.qemu_x-Base'), ('data',
OrderedDict([('__org.qemu_x-member1', '__org.qemu_x-Enum')]))]),
+ OrderedDict([('struct', '__org.qemu_x-Struct'), ('base',
'__org.qemu_x-Base'), ('data', OrderedDict([('__org.qemu_x-member2',
'str')]))])]
[{'enum_name': 'EnumOne', 'enum_values': ['value1', 'value2', 'value3']},
{'enum_name': '__org.qemu_x-Enum', 'enum_values': ['__org.qemu_x-value']},
{'enum_name': 'UserDefAlternateKind', 'enum_values': None},
@@ -39,4 +41,6 @@
OrderedDict([('struct', 'UserDefC'), ('data', OrderedDict([('string1',
'str'), ('string2', 'str')]))]),
OrderedDict([('struct', 'UserDefUnionBase'), ('data', OrderedDict([('string',
'str'), ('enum1', 'EnumOne')]))]),
OrderedDict([('struct', 'UserDefOptions'), ('data', OrderedDict([('*i64',
['int']), ('*u64', ['uint64']), ('*u16', ['uint16']), ('*i64x', 'int'),
('*u64x', 'uint64')]))]),
- OrderedDict([('struct', 'EventStructOne'), ('data', OrderedDict([('struct1',
'UserDefOne'), ('string', 'str'), ('*enum2', 'EnumOne')]))])]
+ Ordered
[Qemu-devel] [PULL 06/26] qapi: Rename generate_enum_full_value() to c_enum_const()
Signed-off-by: Markus Armbruster
Signed-off-by: Eric Blake
---
scripts/qapi-event.py | 5 ++---
scripts/qapi-types.py | 6 +++---
scripts/qapi-visit.py | 4 ++--
scripts/qapi.py | 6 +++---
4 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index c4612e3..a7e0033 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -177,7 +177,7 @@ typedef enum %(event_enum_name)s
event_enum_name = event_enum_name)
# append automatically generated _MAX value
-enum_max_value = generate_enum_full_value(event_enum_name, "MAX")
+enum_max_value = c_enum_const(event_enum_name, "MAX")
enum_values = event_enum_values + [ enum_max_value ]
i = 0
@@ -343,8 +343,7 @@ for expr in exprs:
fdecl.write(ret)
# We need an enum value per event
-event_enum_value = generate_enum_full_value(event_enum_name,
-event_name)
+event_enum_value = c_enum_const(event_enum_name, event_name)
ret = generate_event_implement(api_name, event_name, params)
fdef.write(ret)
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index e74cabe..6ca48c1 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -118,13 +118,13 @@ const char *%(name)s_lookup[] = {
name=name)
i = 0
for value in values:
-index = generate_enum_full_value(name, value)
+index = c_enum_const(name, value)
ret += mcgen('''
[%(index)s] = "%(value)s",
''',
index = index, value = value)
-max_index = generate_enum_full_value(name, 'MAX')
+max_index = c_enum_const(name, 'MAX')
ret += mcgen('''
[%(max_index)s] = NULL,
};
@@ -150,7 +150,7 @@ typedef enum %(name)s
i = 0
for value in enum_values:
-enum_full_value = generate_enum_full_value(name, value)
+enum_full_value = c_enum_const(name, value)
enum_decl += mcgen('''
%(enum_full_value)s = %(i)d,
''',
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index ba623b1..0368e62 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -214,7 +214,7 @@ void visit_type_%(name)s(Visitor *m, %(name)s **obj, const
char *name, Error **e
or find_union(members[key])
or find_enum(members[key])), "Invalid alternate member"
-enum_full_value = generate_enum_full_value(disc_type, key)
+enum_full_value = c_enum_const(disc_type, key)
ret += mcgen('''
case %(enum_full_value)s:
visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, name, &err);
@@ -315,7 +315,7 @@ void visit_type_%(name)s(Visitor *m, %(name)s **obj, const
char *name, Error **e
else:
fmt = 'visit_type_implicit_%(c_type)s(m, &(*obj)->%(c_name)s,
&err);'
-enum_full_value = generate_enum_full_value(disc_type, key)
+enum_full_value = c_enum_const(disc_type, key)
ret += mcgen('''
case %(enum_full_value)s:
''' + fmt + '''
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 7330f7c..1258f76 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -960,7 +960,7 @@ def camel_to_upper(value):
new_name += c
return new_name.lstrip('_').upper()
-def generate_enum_full_value(enum_name, enum_value):
-abbrev_string = camel_to_upper(enum_name)
-value_string = camel_to_upper(enum_value)
+def c_enum_const(type_name, const_name):
+abbrev_string = camel_to_upper(type_name)
+value_string = camel_to_upper(const_name)
return "%s_%s" % (abbrev_string, value_string)
--
1.9.3
[Qemu-devel] [PULL 19/26] qapi: qapi-event.py option -b does nothing, drop it
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
---
Makefile | 2 +-
scripts/qapi-event.py | 7 ++-
2 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/Makefile b/Makefile
index f032158..bfa5dab 100644
--- a/Makefile
+++ b/Makefile
@@ -273,7 +273,7 @@ $(qapi-modules) $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py)
qapi-event.c qapi-event.h :\
$(qapi-modules) $(SRC_PATH)/scripts/qapi-event.py $(qapi-py)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-event.py \
- $(gen-out-type) -o "." -b -i $<, \
+ $(gen-out-type) -o "." -i $<, \
" GEN $@")
qmp-commands.h qmp-marshal.c :\
$(qapi-modules) $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py)
diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index a7e0033..3e1f4cf 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -220,8 +220,8 @@ const char *%(event_enum_name)s_lookup[] = {
# Start the real job
try:
-opts, args = getopt.gnu_getopt(sys.argv[1:], "chbp:i:o:",
- ["source", "header", "builtins", "prefix=",
+opts, args = getopt.gnu_getopt(sys.argv[1:], "chp:i:o:",
+ ["source", "header", "prefix=",
"input-file=", "output-dir="])
except getopt.GetoptError, err:
print str(err)
@@ -235,7 +235,6 @@ h_file = 'qapi-event.h'
do_c = False
do_h = False
-do_builtins = False
for o, a in opts:
if o in ("-p", "--prefix"):
@@ -248,8 +247,6 @@ for o, a in opts:
do_c = True
elif o in ("-h", "--header"):
do_h = True
-elif o in ("-b", "--builtins"):
-do_builtins = True
if not do_c and not do_h:
do_c = True
--
1.9.3
[Qemu-devel] [PULL 10/26] qapi: Tidy c_type() logic
From: Eric Blake c_type() is designed to be called on both string names and on array designations, so 'name' is a bit misleading because it operates on more than strings. Also, no caller ever passes an empty string. Finally, + notation is a bit nicer to read than '%s' % value for string concatenation. Signed-off-by: Eric Blake Signed-off-by: Markus Armbruster --- scripts/qapi.py | 58 + 1 file changed, 30 insertions(+), 28 deletions(-) diff --git a/scripts/qapi.py b/scripts/qapi.py index cc33355..ecab879 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -801,12 +801,12 @@ def c_name(name, protect=True): return name.translate(c_name_trans) def c_list_type(name): -return '%sList' % name +return name + 'List' -def type_name(name): -if type(name) == list: -return c_list_type(name[0]) -return name +def type_name(value): +if type(value) == list: +return c_list_type(value[0]) +return value def add_name(name, info, meta, implicit = False): global all_names @@ -863,42 +863,44 @@ def is_enum(name): return find_enum(name) != None eatspace = '\033EATSPACE.' +pointer_suffix = ' *' + eatspace # 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': +def c_type(value, is_param=False): +if value == 'str': if is_param: -return 'const char *' + eatspace -return 'char *' + eatspace +return 'const char' + pointer_suffix +return 'char' + pointer_suffix -elif name == 'int': +elif value == 'int': return 'int64_t' -elif (name == 'int8' or name == 'int16' or name == 'int32' or - name == 'int64' or name == 'uint8' or name == 'uint16' or - name == 'uint32' or name == 'uint64'): -return name + '_t' -elif name == 'size': +elif (value == 'int8' or value == 'int16' or value == 'int32' or + value == 'int64' or value == 'uint8' or value == 'uint16' or + value == 'uint32' or value == 'uint64'): +return value + '_t' +elif value == 'size': return 'uint64_t' -elif name == 'bool': +elif value == 'bool': return 'bool' -elif name == 'number': +elif value == 'number': return 'double' -elif type(name) == list: -return '%s *%s' % (c_list_type(name[0]), eatspace) -elif is_enum(name): -return name -elif name == None or len(name) == 0: +elif type(value) == list: +return c_list_type(value[0]) + pointer_suffix +elif is_enum(value): +return value +elif value == None: return 'void' -elif name in events: -return '%sEvent *%s' % (camel_case(name), eatspace) +elif value in events: +return camel_case(value) + 'Event' + pointer_suffix else: -return '%s *%s' % (name, eatspace) +# complex type name +assert isinstance(value, str) and value != "" +return value + pointer_suffix -def is_c_ptr(name): -suffix = "*" + eatspace -return c_type(name).endswith(suffix) +def is_c_ptr(value): +return c_type(value).endswith(pointer_suffix) def genindent(count): ret = "" -- 1.9.3
[Qemu-devel] [PULL 18/26] tests: Add missing dependencies on $(qapi-py)
Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake --- tests/Makefile | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/Makefile b/tests/Makefile index 666aee2..6d2f2e5 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -301,22 +301,22 @@ tests/test-vmstate$(EXESUF): tests/test-vmstate.o \ libqemuutil.a libqemustub.a tests/test-qapi-types.c tests/test-qapi-types.h :\ -$(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-types.py +$(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py) $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py \ $(gen-out-type) -o tests -p "test-" -i $<, \ " GEN $@") tests/test-qapi-visit.c tests/test-qapi-visit.h :\ -$(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-visit.py +$(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py) $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py \ $(gen-out-type) -o tests -p "test-" -i $<, \ " GEN $@") tests/test-qmp-commands.h tests/test-qmp-marshal.c :\ -$(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-commands.py +$(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py) $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py \ $(gen-out-type) -o tests -p "test-" -i $<, \ " GEN $@") tests/test-qapi-event.c tests/test-qapi-event.h :\ -$(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-event.py +$(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-event.py $(qapi-py) $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-event.py \ $(gen-out-type) -o tests -p "test-" -i $<, \ " GEN $@") -- 1.9.3
[Qemu-devel] [PULL 03/26] qapi: Fix C identifiers generated for names containing '.'
c_fun() maps '.' to '_', c_var() doesn't. Nothing prevents '.' in
QAPI names that get passed to c_var().
Which QAPI names get passed to c_fun(), to c_var(), or to both is not
obvious. Names of command parameters and struct type members get
passed to c_var().
c_var() strips a leading '*', but this cannot happen. c_fun()
doesn't.
Fix c_var() to work exactly like c_fun().
Perhaps they should be replaced by a single mapping function.
Signed-off-by: Markus Armbruster
[add 'import string']
Signed-off-by: Eric Blake
Reviewed-by: Alberto Garcia
---
scripts/qapi.py | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 166b74f..4b07805 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -15,6 +15,7 @@ import re
from ordereddict import OrderedDict
import os
import sys
+import string
builtin_types = {
'str': 'QTYPE_QSTRING',
@@ -752,6 +753,8 @@ def camel_case(name):
new_name += ch.lower()
return new_name
+c_var_trans = string.maketrans('.-', '__')
+
def c_var(name, protect=True):
# ANSI X3J11/88-090, 3.1.1
c89_words = set(['auto', 'break', 'case', 'char', 'const', 'continue',
@@ -781,10 +784,10 @@ def c_var(name, protect=True):
polluted_words = set(['unix', 'errno'])
if protect and (name in c89_words | c99_words | c11_words | gcc_words |
cpp_words | polluted_words):
return "q_" + name
-return name.replace('-', '_').lstrip("*")
+return name.translate(c_var_trans)
def c_fun(name, protect=True):
-return c_var(name, protect).replace('.', '_')
+return c_var(name, protect)
def c_list_type(name):
return '%sList' % name
--
1.9.3
[Qemu-devel] [PULL 11/26] qapi: Make c_type() consistently convert qapi names
From: Eric Blake
Continuing the string of cleanups for supporting downstream names
containing '.', this patch focuses on ensuring c_type() can
handle a downstream name. This patch alone does not fix the
places where generator output should be calling this function
but was open-coding things instead, but it gets us a step closer.
In particular, the changes to c_list_type() and type_name() mean
that type_name(FOO) now handles the case when FOO contains '.',
'-', or is a ticklish identifier other than a builtin (builtins
are exempted because ['int'] must remain mapped to 'intList' and
not 'q_intList'). Meanwhile, ['unix'] now maps to 'q_unixList'
rather than 'unixList', to match the fact that 'unix' is ticklish;
however, our naming conventions state that complex types should
start with a capital, so no type name following conventions will
ever have the 'q_' prepended.
Likewise, changes to c_type() mean that c_type(FOO) properly
handles an enum or complex type FOO with '.' or '-' in the
name, or is a ticklish identifier (again, a ticklish identifier
as a type name violates conventions).
Signed-off-by: Eric Blake
Signed-off-by: Markus Armbruster
---
scripts/qapi.py | 33 +
1 file changed, 29 insertions(+), 4 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index ecab879..273afed 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -769,6 +769,15 @@ def c_enum_const(type_name, const_name):
c_name_trans = string.maketrans('.-', '__')
+# Map @name to a valid C identifier.
+# If @protect, avoid returning certain ticklish identifiers (like
+# C keywords) by prepending "q_".
+#
+# Used for converting 'name' from a 'name':'type' qapi definition
+# into a generated struct member, as well as converting type names
+# into substrings of a generated C function name.
+# '__a.b_c' -> '__a_b_c', 'x-foo' -> 'x_foo'
+# protect=True: 'int' -> 'q_int'; protect=False: 'int' -> 'int'
def c_name(name, protect=True):
# ANSI X3J11/88-090, 3.1.1
c89_words = set(['auto', 'break', 'case', 'char', 'const', 'continue',
@@ -800,13 +809,25 @@ def c_name(name, protect=True):
return "q_" + name
return name.translate(c_name_trans)
+# Map type @name to the C typedef name for the list form.
+#
+# ['Name'] -> 'NameList', ['x-Foo'] -> 'x_FooList', ['int'] -> 'intList'
def c_list_type(name):
-return name + 'List'
+return type_name(name) + 'List'
+# Map type @value to the C typedef form.
+#
+# Used for converting 'type' from a 'member':'type' qapi definition
+# into the alphanumeric portion of the type for a generated C parameter,
+# as well as generated C function names. See c_type() for the rest of
+# the conversion such as adding '*' on pointer types.
+# 'int' -> 'int', '[x-Foo]' -> 'x_FooList', '__a.b_c' -> '__a_b_c'
def type_name(value):
if type(value) == list:
return c_list_type(value[0])
-return value
+if value in builtin_types.keys():
+return value
+return c_name(value)
def add_name(name, info, meta, implicit = False):
global all_names
@@ -865,6 +886,10 @@ def is_enum(name):
eatspace = '\033EATSPACE.'
pointer_suffix = ' *' + eatspace
+# Map type @name to its C type expression.
+# If @is_param, const-qualify the string type.
+#
+# This function is used for computing the full C type of 'member':'name'.
# 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().
@@ -889,7 +914,7 @@ def c_type(value, is_param=False):
elif type(value) == list:
return c_list_type(value[0]) + pointer_suffix
elif is_enum(value):
-return value
+return c_name(value)
elif value == None:
return 'void'
elif value in events:
@@ -897,7 +922,7 @@ def c_type(value, is_param=False):
else:
# complex type name
assert isinstance(value, str) and value != ""
-return value + pointer_suffix
+return c_name(value) + pointer_suffix
def is_c_ptr(value):
return c_type(value).endswith(pointer_suffix)
--
1.9.3
[Qemu-devel] [PULL 17/26] qapi: Support downstream events and commands
From: Eric Blake
Enhance the testsuite to cover downstream events and commands.
Events worked without more tweaks, but commands needed a few final
updates in the generator to mangle names in the appropriate places.
In making those tweaks, it was easier to drop type_visitor() and
inline its actions instead.
Signed-off-by: Eric Blake
Signed-off-by: Markus Armbruster
---
scripts/qapi-commands.py| 16 +---
tests/qapi-schema/qapi-schema-test.json | 5 +
tests/qapi-schema/qapi-schema-test.out | 4 +++-
tests/test-qmp-commands.c | 15 +++
4 files changed, 28 insertions(+), 12 deletions(-)
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 8c125ca..0a1d636 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -20,12 +20,6 @@ import os
import getopt
import errno
-def type_visitor(name):
-if type(name) == list:
-return 'visit_type_%sList' % name[0]
-else:
-return 'visit_type_%s' % name
-
def generate_command_decl(name, args, ret_type):
arglist=""
for argname, argtype, optional in parse_args(args):
@@ -153,10 +147,10 @@ if (has_%(c_name)s) {
c_name=c_name(argname))
push_indent()
ret += mcgen('''
-%(visitor)s(v, &%(c_name)s, "%(name)s", %(errp)s);
+visit_type_%(visitor)s(v, &%(c_name)s, "%(name)s", %(errp)s);
''',
c_name=c_name(argname), name=argname, argtype=argtype,
- visitor=type_visitor(argtype), errp=errparg)
+ visitor=type_name(argtype), errp=errparg)
ret += gen_err_check(errarg)
if optional:
pop_indent()
@@ -184,7 +178,7 @@ static void qmp_marshal_output_%(c_name)s(%(c_ret_type)s
ret_in, QObject **ret_o
Visitor *v;
v = qmp_output_get_visitor(mo);
-%(visitor)s(v, &ret_in, "unused", &local_err);
+visit_type_%(visitor)s(v, &ret_in, "unused", &local_err);
if (local_err) {
goto out;
}
@@ -195,12 +189,12 @@ out:
qmp_output_visitor_cleanup(mo);
md = qapi_dealloc_visitor_new();
v = qapi_dealloc_get_visitor(md);
-%(visitor)s(v, &ret_in, "unused", NULL);
+visit_type_%(visitor)s(v, &ret_in, "unused", NULL);
qapi_dealloc_visitor_cleanup(md);
}
''',
c_ret_type=c_type(ret_type), c_name=c_name(name),
-visitor=type_visitor(ret_type))
+visitor=type_name(ret_type))
return ret
diff --git a/tests/qapi-schema/qapi-schema-test.json
b/tests/qapi-schema/qapi-schema-test.json
index d586b56..c7eaa86 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -122,3 +122,8 @@
'data': { '__org.qemu_x-value': '__org.qemu_x-Struct2' } }
{ 'alternate': '__org.qemu_x-Alt',
'data': { '__org.qemu_x-branch': 'str', 'b': '__org.qemu_x-Base' } }
+{ 'event': '__ORG.QEMU_X-EVENT', 'data': '__org.qemu_x-Struct' }
+{ 'command': '__org.qemu_x-command',
+ 'data': { 'a': ['__org.qemu_x-Enum'], 'b': ['__org.qemu_x-Struct'],
+'c': '__org.qemu_x-Union2', 'd': '__org.qemu_x-Alt' },
+ 'returns': '__org.qemu_x-Union1' }
diff --git a/tests/qapi-schema/qapi-schema-test.out
b/tests/qapi-schema/qapi-schema-test.out
index 2161a90..cf0ccc4 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -29,7 +29,9 @@
OrderedDict([('union', '__org.qemu_x-Union1'), ('data',
OrderedDict([('__org.qemu_x-branch', 'str')]))]),
OrderedDict([('struct', '__org.qemu_x-Struct2'), ('data',
OrderedDict([('array', ['__org.qemu_x-Union1'])]))]),
OrderedDict([('union', '__org.qemu_x-Union2'), ('base', '__org.qemu_x-Base'),
('discriminator', '__org.qemu_x-member1'), ('data',
OrderedDict([('__org.qemu_x-value', '__org.qemu_x-Struct2')]))]),
- OrderedDict([('alternate', '__org.qemu_x-Alt'), ('data',
OrderedDict([('__org.qemu_x-branch', 'str'), ('b', '__org.qemu_x-Base')]))])]
+ OrderedDict([('alternate', '__org.qemu_x-Alt'), ('data',
OrderedDict([('__org.qemu_x-branch', 'str'), ('b', '__org.qemu_x-Base')]))]),
+ OrderedDict([('event', '__ORG.QEMU_X-EVENT'), ('data',
'__org.qemu_x-Struct')]),
+ OrderedDict([('command', '__org.qemu_x-command'), ('data', OrderedDict([('a',
['__org.qemu_x-Enum']), ('b', ['__org.qemu_x-Struct']), ('c',
'__org.qemu_x-Union2'), ('d', '__org.qemu_x-Alt')])), ('returns',
'__org.qemu_x-Union1')])]
[{'enum_name': 'EnumOne', 'enum_values': ['value1', 'value2', 'value3']},
{'enum_name': '__org.qemu_x-Enum', 'enum_values': ['__org.qemu_x-value']},
{'enum_name': 'UserDefAlternateKind', 'enum_values': None},
diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c
index ad2e403..9918f23 100644
--- a/tests/test-qmp-commands.c
+++ b/tests/test-qmp-commands.c
@@ -51,6 +51,21 @@ int64_t qmp_user_def_cmd3(int64_t a, bool has_b, int64_t b,
Error **errp)
return a + (has_b ? b : 0);
}
+__org_qemu_x_Union1 *qmp___org_qemu_x_comman
[Qemu-devel] [PULL 23/26] qapi: Turn generators' mandatory option -i into an argument
Mandatory option is silly, and the error handling is missing: the programs crash when -i isn't supplied. Make it an argument, and check it properly. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake --- Makefile | 14 +++--- docs/qapi-code-gen.txt | 10 +- scripts/qapi.py| 12 +--- tests/Makefile | 8 4 files changed, 21 insertions(+), 23 deletions(-) diff --git a/Makefile b/Makefile index bfa5dab..d945804 100644 --- a/Makefile +++ b/Makefile @@ -243,17 +243,17 @@ qapi-py = $(SRC_PATH)/scripts/qapi.py $(SRC_PATH)/scripts/ordereddict.py qga/qapi-generated/qga-qapi-types.c qga/qapi-generated/qga-qapi-types.h :\ $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py) $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py \ - $(gen-out-type) -o qga/qapi-generated -p "qga-" -i $<, \ + $(gen-out-type) -o qga/qapi-generated -p "qga-" $<, \ " GEN $@") qga/qapi-generated/qga-qapi-visit.c qga/qapi-generated/qga-qapi-visit.h :\ $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py) $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py \ - $(gen-out-type) -o qga/qapi-generated -p "qga-" -i $<, \ + $(gen-out-type) -o qga/qapi-generated -p "qga-" $<, \ " GEN $@") qga/qapi-generated/qga-qmp-commands.h qga/qapi-generated/qga-qmp-marshal.c :\ $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py) $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py \ - $(gen-out-type) -o qga/qapi-generated -p "qga-" -i $<, \ + $(gen-out-type) -o qga/qapi-generated -p "qga-" $<, \ " GEN $@") qapi-modules = $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/qapi/common.json \ @@ -263,22 +263,22 @@ qapi-modules = $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/qapi/common.json \ qapi-types.c qapi-types.h :\ $(qapi-modules) $(SRC_PATH)/scripts/qapi-types.py $(qapi-py) $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py \ - $(gen-out-type) -o "." -b -i $<, \ + $(gen-out-type) -o "." -b $<, \ " GEN $@") qapi-visit.c qapi-visit.h :\ $(qapi-modules) $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py) $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py \ - $(gen-out-type) -o "." -b -i $<, \ + $(gen-out-type) -o "." -b $<, \ " GEN $@") qapi-event.c qapi-event.h :\ $(qapi-modules) $(SRC_PATH)/scripts/qapi-event.py $(qapi-py) $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-event.py \ - $(gen-out-type) -o "." -i $<, \ + $(gen-out-type) -o "." $<, \ " GEN $@") qmp-commands.h qmp-marshal.c :\ $(qapi-modules) $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py) $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py \ - $(gen-out-type) -o "." -m -i $<, \ + $(gen-out-type) -o "." -m $<, \ " GEN $@") QGALIB_GEN=$(addprefix qga/qapi-generated/, qga-qapi-types.h qga-qapi-visit.h qga-qmp-commands.h) diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt index 269a1f3..3f0522e 100644 --- a/docs/qapi-code-gen.txt +++ b/docs/qapi-code-gen.txt @@ -536,7 +536,7 @@ created code. Example: $ python scripts/qapi-types.py --output-dir="qapi-generated" \ ---prefix="example-" --input-file=example-schema.json +--prefix="example-" example-schema.json $ cat qapi-generated/example-qapi-types.c [Uninteresting stuff omitted...] @@ -623,7 +623,7 @@ $(prefix)qapi-visit.h: declarations for previously mentioned visitor Example: $ python scripts/qapi-visit.py --output-dir="qapi-generated" ---prefix="example-" --input-file=example-schema.json +--prefix="example-" example-schema.json $ cat qapi-generated/example-qapi-visit.c [Uninteresting stuff omitted...] @@ -681,7 +681,7 @@ Example: error_propagate(errp, err); } $ python scripts/qapi-commands.py --output-dir="qapi-generated" \ ---prefix="example-" --input-file=example-schema.json +--prefix="example-" example-schema.json $ cat qapi-generated/example-qapi-visit.h [Uninteresting stuff omitted...] @@ -715,7 +715,7 @@ $(prefix)qmp-commands.h: Function prototypes for the QMP commands Example: $ python scripts/qapi-commands.py --output-dir="qapi-generated" ---prefix="example-" --input-file=example-schema.json +--prefix="example-" example-schema.json $ cat qapi-generated/example-qmp-marshal.c [Uninteresting stuff omitted...] @@ -806,7 +806,7 @@ $(prefix)qapi-event.c - Implementation of functions to send an event Example: $ python scripts/qapi-event.py --output-dir="qapi-generated" ---prefix="example-" --input-file=e
[Qemu-devel] [PULL 25/26] qapi: Drop pointless flush() before close()
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
---
scripts/qapi.py | 4
1 file changed, 4 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index fbfe050..f96a777 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1064,9 +1064,5 @@ def close_output(fdef, fdecl):
fdecl.write('''
#endif
''')
-
-fdecl.flush()
fdecl.close()
-
-fdef.flush()
fdef.close()
--
1.9.3
[Qemu-devel] [PULL 12/26] qapi: Support downstream enums
From: Eric Blake
Enhance the testsuite to cover a downstream enum type and enum
string. Update the generator to mangle the enum name in the
appropriate places.
Signed-off-by: Eric Blake
Signed-off-by: Markus Armbruster
---
scripts/qapi-types.py | 15 ---
scripts/qapi-visit.py | 8
tests/qapi-schema/qapi-schema-test.json | 3 +++
tests/qapi-schema/qapi-schema-test.out | 4 +++-
4 files changed, 18 insertions(+), 12 deletions(-)
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 9eb08a6..1593fc6 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -58,7 +58,7 @@ typedef struct %(name)sList
struct %(name)sList *next;
} %(name)sList;
''',
- name=name)
+ name=c_name(name))
def generate_struct_fields(members):
ret = ''
@@ -115,7 +115,7 @@ def generate_enum_lookup(name, values):
ret = mcgen('''
const char *%(name)s_lookup[] = {
''',
- name=name)
+name=c_name(name))
i = 0
for value in values:
index = c_enum_const(name, value)
@@ -134,6 +134,7 @@ const char *%(name)s_lookup[] = {
return ret
def generate_enum(name, values):
+name = c_name(name)
lookup_decl = mcgen('''
extern const char *%(name)s_lookup[];
''',
@@ -247,15 +248,15 @@ extern const int %(name)s_qtypes[];
def generate_type_cleanup_decl(name):
ret = mcgen('''
-void qapi_free_%(type)s(%(c_type)s obj);
+void qapi_free_%(name)s(%(c_type)s obj);
''',
-c_type=c_type(name),type=name)
+c_type=c_type(name), name=c_name(name))
return ret
def generate_type_cleanup(name):
ret = mcgen('''
-void qapi_free_%(type)s(%(c_type)s obj)
+void qapi_free_%(name)s(%(c_type)s obj)
{
QapiDeallocVisitor *md;
Visitor *v;
@@ -266,11 +267,11 @@ void qapi_free_%(type)s(%(c_type)s obj)
md = qapi_dealloc_visitor_new();
v = qapi_dealloc_get_visitor(md);
-visit_type_%(type)s(v, &obj, NULL, NULL);
+visit_type_%(name)s(v, &obj, NULL, NULL);
qapi_dealloc_visitor_cleanup(md);
}
''',
-c_type=c_type(name),type=name)
+c_type=c_type(name), name=c_name(name))
return ret
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 0368e62..7697ec6 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -173,7 +173,7 @@ out:
error_propagate(errp, err);
}
''',
-name=name)
+name=type_name(name))
def generate_visit_enum(name, members):
return mcgen('''
@@ -183,7 +183,7 @@ void visit_type_%(name)s(Visitor *m, %(name)s *obj, const
char *name, Error **er
visit_type_enum(m, (int *)obj, %(name)s_lookup, "%(name)s", name, errp);
}
''',
- name=name)
+ name=c_name(name))
def generate_visit_alternate(name, members):
ret = mcgen('''
@@ -364,7 +364,7 @@ def generate_enum_declaration(name, members):
ret = mcgen('''
void visit_type_%(name)sList(Visitor *m, %(name)sList **obj, const char *name,
Error **errp);
''',
-name=name)
+name=c_name(name))
return ret
@@ -373,7 +373,7 @@ def generate_decl_enum(name, members):
void visit_type_%(name)s(Visitor *m, %(name)s *obj, const char *name, Error
**errp);
''',
-name=name)
+ name=c_name(name))
try:
opts, args = getopt.gnu_getopt(sys.argv[1:], "chbp:i:o:",
diff --git a/tests/qapi-schema/qapi-schema-test.json
b/tests/qapi-schema/qapi-schema-test.json
index 8193dc1..5f9af66 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -107,3 +107,6 @@
'data': { '*a': 'int', '*b': 'UserDefOne', 'c': 'str' } }
{ 'event': 'EVENT_D',
'data': { 'a' : 'EventStructOne', 'b' : 'str', '*c': 'str', '*enum3':
'EnumOne' } }
+
+# test that we correctly compile downstream extensions
+{ 'enum': '__org.qemu_x-Enum', 'data': [ '__org.qemu_x-value' ] }
diff --git a/tests/qapi-schema/qapi-schema-test.out
b/tests/qapi-schema/qapi-schema-test.out
index 93c4963..40f0f20 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -22,8 +22,10 @@
OrderedDict([('event', 'EVENT_A')]),
OrderedDict([('event', 'EVENT_B'), ('data', OrderedDict())]),
OrderedDict([('event', 'EVENT_C'), ('data', OrderedDict([('*a', 'int'),
('*b', 'UserDefOne'), ('c', 'str')]))]),
- OrderedDict([('event', 'EVENT_D'), ('data', OrderedDict([('a',
'EventStructOne'), ('b', 'str'), ('*c', 'str'), ('*enum3', 'EnumOne')]))])]
+ OrderedDict([('event', 'EVENT_D'), ('data', OrderedDict([('a',
'EventStructOne'), ('b', 'str'), ('*c', 'str'), ('*enum3', 'EnumOne')]))]),
+ OrderedDict([('enum', '__org.qemu_x-Enum'), ('data',
['__org.qemu_x-value'])])]
[{'enum_name': 'EnumOne', 'enum_values': ['value1', 'value2', 'value3']},
+ {'enum_name': '__org.qemu_x-Enum', 'enum_values': ['__org.qemu
[Qemu-devel] [PULL 15/26] qapi: Support downstream flat unions
From: Eric Blake
Enhance the testsuite to cover downstream flat unions, including
the base type, discriminator name and type, and branch name and
type. Update the generator to mangle the union names in the
appropriate places.
Signed-off-by: Eric Blake
Signed-off-by: Markus Armbruster
---
scripts/qapi-types.py | 2 +-
scripts/qapi-visit.py | 4 ++--
tests/qapi-schema/qapi-schema-test.json | 5 +
tests/qapi-schema/qapi-schema-test.out | 7 +--
4 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 5b0bc5d..13e4b53 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -213,7 +213,7 @@ struct %(name)s
void *data;
''',
name=name,
-discriminator_type_name=discriminator_type_name)
+discriminator_type_name=c_name(discriminator_type_name))
for key in typeinfo:
ret += mcgen('''
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index d1ec70b..c15305f 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -252,7 +252,7 @@ def generate_visit_union(expr):
if enum_define:
# Use the enum type as discriminator
ret = ""
-disc_type = enum_define['enum_name']
+disc_type = c_name(enum_define['enum_name'])
else:
# There will always be a discriminator in the C switch code, by default
# it is an enum type generated silently
@@ -290,7 +290,7 @@ void visit_type_%(name)s(Visitor *m, %(name)s **obj, const
char *name, Error **e
goto out_obj;
}
''',
-name=name)
+ name=c_name(name))
if not discriminator:
disc_key = "type"
diff --git a/tests/qapi-schema/qapi-schema-test.json
b/tests/qapi-schema/qapi-schema-test.json
index 6416d85..ac236e3 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -115,3 +115,8 @@
{ 'struct': '__org.qemu_x-Struct', 'base': '__org.qemu_x-Base',
'data': { '__org.qemu_x-member2': 'str' } }
{ 'union': '__org.qemu_x-Union1', 'data': { '__org.qemu_x-branch': 'str' } }
+{ 'struct': '__org.qemu_x-Struct2',
+ 'data': { 'array': ['__org.qemu_x-Union1'] } }
+{ 'union': '__org.qemu_x-Union2', 'base': '__org.qemu_x-Base',
+ 'discriminator': '__org.qemu_x-member1',
+ 'data': { '__org.qemu_x-value': '__org.qemu_x-Struct2' } }
diff --git a/tests/qapi-schema/qapi-schema-test.out
b/tests/qapi-schema/qapi-schema-test.out
index f9ebe08..3fc24e8 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -26,7 +26,9 @@
OrderedDict([('enum', '__org.qemu_x-Enum'), ('data',
['__org.qemu_x-value'])]),
OrderedDict([('struct', '__org.qemu_x-Base'), ('data',
OrderedDict([('__org.qemu_x-member1', '__org.qemu_x-Enum')]))]),
OrderedDict([('struct', '__org.qemu_x-Struct'), ('base',
'__org.qemu_x-Base'), ('data', OrderedDict([('__org.qemu_x-member2',
'str')]))]),
- OrderedDict([('union', '__org.qemu_x-Union1'), ('data',
OrderedDict([('__org.qemu_x-branch', 'str')]))])]
+ OrderedDict([('union', '__org.qemu_x-Union1'), ('data',
OrderedDict([('__org.qemu_x-branch', 'str')]))]),
+ OrderedDict([('struct', '__org.qemu_x-Struct2'), ('data',
OrderedDict([('array', ['__org.qemu_x-Union1'])]))]),
+ OrderedDict([('union', '__org.qemu_x-Union2'), ('base', '__org.qemu_x-Base'),
('discriminator', '__org.qemu_x-member1'), ('data',
OrderedDict([('__org.qemu_x-value', '__org.qemu_x-Struct2')]))])]
[{'enum_name': 'EnumOne', 'enum_values': ['value1', 'value2', 'value3']},
{'enum_name': '__org.qemu_x-Enum', 'enum_values': ['__org.qemu_x-value']},
{'enum_name': 'UserDefAlternateKind', 'enum_values': None},
@@ -45,4 +47,5 @@
OrderedDict([('struct', 'UserDefOptions'), ('data', OrderedDict([('*i64',
['int']), ('*u64', ['uint64']), ('*u16', ['uint16']), ('*i64x', 'int'),
('*u64x', 'uint64')]))]),
OrderedDict([('struct', 'EventStructOne'), ('data', OrderedDict([('struct1',
'UserDefOne'), ('string', 'str'), ('*enum2', 'EnumOne')]))]),
OrderedDict([('struct', '__org.qemu_x-Base'), ('data',
OrderedDict([('__org.qemu_x-member1', '__org.qemu_x-Enum')]))]),
- OrderedDict([('struct', '__org.qemu_x-Struct'), ('base',
'__org.qemu_x-Base'), ('data', OrderedDict([('__org.qemu_x-member2',
'str')]))])]
+ OrderedDict([('struct', '__org.qemu_x-Struct'), ('base',
'__org.qemu_x-Base'), ('data', OrderedDict([('__org.qemu_x-member2',
'str')]))]),
+ OrderedDict([('struct', '__org.qemu_x-Struct2'), ('data',
OrderedDict([('array', ['__org.qemu_x-Union1'])]))])]
--
1.9.3
[Qemu-devel] [PULL 16/26] qapi: Support downstream alternates
From: Eric Blake
Enhance the testsuite to cover downstream alternates, including
whether the branch name or type is downstream. Update the
generator to mangle alternate names in the appropriate places.
Signed-off-by: Eric Blake
Signed-off-by: Markus Armbruster
---
scripts/qapi-types.py | 7 ---
scripts/qapi-visit.py | 6 +++---
tests/qapi-schema/qapi-schema-test.json | 2 ++
tests/qapi-schema/qapi-schema-test.out | 6 --
4 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 13e4b53..5665145 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -174,16 +174,17 @@ def generate_alternate_qtypes(expr):
ret = mcgen('''
const int %(name)s_qtypes[QTYPE_MAX] = {
''',
-name=name)
+name=c_name(name))
for key in members:
qtype = find_alternate_member_qtype(members[key])
assert qtype, "Invalid alternate member"
ret += mcgen('''
-[ %(qtype)s ] = %(enum_const)s,
+[%(qtype)s] = %(enum_const)s,
''',
-qtype = qtype, enum_const = c_enum_const(name + 'Kind', key))
+ qtype = qtype,
+ enum_const = c_enum_const(name + 'Kind', key))
ret += mcgen('''
};
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index c15305f..e511be3 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -202,11 +202,11 @@ void visit_type_%(name)s(Visitor *m, %(name)s **obj,
const char *name, Error **e
}
switch ((*obj)->kind) {
''',
-name=name)
+name=c_name(name))
# For alternate, always use the default enum type automatically generated
-# as "'%sKind' % (name)"
-disc_type = '%sKind' % (name)
+# as name + 'Kind'
+disc_type = c_name(name) + 'Kind'
for key in members:
assert (members[key] in builtin_types.keys()
diff --git a/tests/qapi-schema/qapi-schema-test.json
b/tests/qapi-schema/qapi-schema-test.json
index ac236e3..d586b56 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -120,3 +120,5 @@
{ 'union': '__org.qemu_x-Union2', 'base': '__org.qemu_x-Base',
'discriminator': '__org.qemu_x-member1',
'data': { '__org.qemu_x-value': '__org.qemu_x-Struct2' } }
+{ 'alternate': '__org.qemu_x-Alt',
+ 'data': { '__org.qemu_x-branch': 'str', 'b': '__org.qemu_x-Base' } }
diff --git a/tests/qapi-schema/qapi-schema-test.out
b/tests/qapi-schema/qapi-schema-test.out
index 3fc24e8..2161a90 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -28,12 +28,14 @@
OrderedDict([('struct', '__org.qemu_x-Struct'), ('base',
'__org.qemu_x-Base'), ('data', OrderedDict([('__org.qemu_x-member2',
'str')]))]),
OrderedDict([('union', '__org.qemu_x-Union1'), ('data',
OrderedDict([('__org.qemu_x-branch', 'str')]))]),
OrderedDict([('struct', '__org.qemu_x-Struct2'), ('data',
OrderedDict([('array', ['__org.qemu_x-Union1'])]))]),
- OrderedDict([('union', '__org.qemu_x-Union2'), ('base', '__org.qemu_x-Base'),
('discriminator', '__org.qemu_x-member1'), ('data',
OrderedDict([('__org.qemu_x-value', '__org.qemu_x-Struct2')]))])]
+ OrderedDict([('union', '__org.qemu_x-Union2'), ('base', '__org.qemu_x-Base'),
('discriminator', '__org.qemu_x-member1'), ('data',
OrderedDict([('__org.qemu_x-value', '__org.qemu_x-Struct2')]))]),
+ OrderedDict([('alternate', '__org.qemu_x-Alt'), ('data',
OrderedDict([('__org.qemu_x-branch', 'str'), ('b', '__org.qemu_x-Base')]))])]
[{'enum_name': 'EnumOne', 'enum_values': ['value1', 'value2', 'value3']},
{'enum_name': '__org.qemu_x-Enum', 'enum_values': ['__org.qemu_x-value']},
{'enum_name': 'UserDefAlternateKind', 'enum_values': None},
{'enum_name': 'UserDefNativeListUnionKind', 'enum_values': None},
- {'enum_name': '__org.qemu_x-Union1Kind', 'enum_values': None}]
+ {'enum_name': '__org.qemu_x-Union1Kind', 'enum_values': None},
+ {'enum_name': '__org.qemu_x-AltKind', 'enum_values': None}]
[OrderedDict([('struct', 'NestedEnumsOne'), ('data', OrderedDict([('enum1',
'EnumOne'), ('*enum2', 'EnumOne'), ('enum3', 'EnumOne'), ('*enum4',
'EnumOne')]))]),
OrderedDict([('struct', 'UserDefZero'), ('data', OrderedDict([('integer',
'int')]))]),
OrderedDict([('struct', 'UserDefOne'), ('base', 'UserDefZero'), ('data',
OrderedDict([('string', 'str'), ('*enum1', 'EnumOne')]))]),
--
1.9.3
[Qemu-devel] [PULL 24/26] qapi: Factor open_output(), close_output() out of generators
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
---
scripts/qapi-commands.py | 101 +--
scripts/qapi-event.py| 85 ---
scripts/qapi-types.py| 81 -
scripts/qapi-visit.py| 101 ---
scripts/qapi.py | 50 +++
5 files changed, 172 insertions(+), 246 deletions(-)
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 2889877..c3e420e 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -15,8 +15,6 @@
from ordereddict import OrderedDict
from qapi import *
import re
-import os
-import errno
def generate_command_decl(name, args, ret_type):
arglist=""
@@ -311,51 +309,18 @@ qapi_init(qmp_init_marshal);
registry=registry.rstrip())
return ret
-def gen_command_decl_prologue(header, guard, prefix=""):
+def gen_command_decl_prologue(prefix=""):
ret = mcgen('''
-/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
-
-/*
- * schema-defined QAPI function prototypes
- *
- * Copyright IBM, Corp. 2011
- *
- * Authors:
- * Anthony Liguori
- *
- * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
- * See the COPYING.LIB file in the top-level directory.
- *
- */
-
-#ifndef %(guard)s
-#define %(guard)s
-
#include "%(prefix)sqapi-types.h"
#include "qapi/qmp/qdict.h"
#include "qapi/error.h"
''',
- header=basename(header), guard=guardname(header),
prefix=prefix)
+ prefix=prefix)
return ret
def gen_command_def_prologue(prefix="", proxy=False):
ret = mcgen('''
-/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
-
-/*
- * schema-defined QMP->QAPI command dispatch
- *
- * Copyright IBM, Corp. 2011
- *
- * Authors:
- * Anthony Liguori
- *
- * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
- * See the COPYING.LIB file in the top-level directory.
- *
- */
-
#include "qemu-common.h"
#include "qemu/module.h"
#include "qapi/qmp/qerror.h"
@@ -374,8 +339,6 @@ def gen_command_def_prologue(prefix="", proxy=False):
ret += '#include "%sqmp-commands.h"' % prefix
return ret + "\n\n"
-c_file = 'qmp-marshal.c'
-h_file = 'qmp-commands.h'
middle_mode = False
(input_file, output_dir, do_c, do_h, prefix, opts) = \
@@ -385,29 +348,44 @@ for o, a in opts:
if o in ("-m", "--middle"):
middle_mode = True
-c_file = output_dir + prefix + c_file
-h_file = output_dir + prefix + h_file
-
-def maybe_open(really, name, opt):
-if really:
-return open(name, opt)
-else:
-import StringIO
-return StringIO.StringIO()
-
-try:
-os.makedirs(output_dir)
-except os.error, e:
-if e.errno != errno.EEXIST:
-raise
-
exprs = parse_schema(input_file)
commands = filter(lambda expr: expr.has_key('command'), exprs)
commands = filter(lambda expr: not expr.has_key('gen'), commands)
-fdecl = maybe_open(do_h, h_file, 'w')
-fdef = maybe_open(do_c, c_file, 'w')
-ret = gen_command_decl_prologue(header=basename(h_file),
guard=guardname(h_file), prefix=prefix)
+c_comment = '''
+/*
+ * schema-defined QMP->QAPI command dispatch
+ *
+ * Copyright IBM, Corp. 2011
+ *
+ * Authors:
+ * Anthony Liguori
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+'''
+h_comment = '''
+/*
+ * schema-defined QAPI function prototypes
+ *
+ * Copyright IBM, Corp. 2011
+ *
+ * Authors:
+ * Anthony Liguori
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+'''
+
+(fdef, fdecl) = open_output(output_dir, do_c, do_h, prefix,
+'qmp-marshal.c', 'qmp-commands.h',
+c_comment, h_comment)
+
+ret = gen_command_decl_prologue(prefix=prefix)
fdecl.write(ret)
ret = gen_command_def_prologue(prefix=prefix)
fdef.write(ret)
@@ -431,13 +409,8 @@ for cmd in commands:
ret = gen_marshal_input(cmd['command'], arglist, ret_type, middle_mode) +
"\n"
fdef.write(ret)
-fdecl.write("\n#endif\n");
-
if not middle_mode:
ret = gen_registry(commands)
fdef.write(ret)
-fdef.flush()
-fdef.close()
-fdecl.flush()
-fdecl.close()
+close_output(fdef, fdecl)
diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index bc5ca4a..56bc602 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -11,8 +11,6 @@
from ordereddict import OrderedDict
from qapi import *
-import os
-import errno
def _generate_event_api_name(event_name, params):
api_name = "void qapi_event_send_%s(" % c_name(event_name).lower();
@@ -214,36 +212,9 @@ const char *%(event_enum_name)s_lookup[] = {
''')
return ret
-
-# Start the real job
-
-c_file = 'qapi-event.c'
-h_fil
[Qemu-devel] [PULL 09/26] qapi: Move camel_to_upper(), c_enum_const() to closely related code
Signed-off-by: Markus Armbruster
Signed-off-by: Eric Blake
---
scripts/qapi.py | 50 +-
1 file changed, 25 insertions(+), 25 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 3757a91..cc33355 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -742,6 +742,31 @@ def camel_case(name):
new_name += ch.lower()
return new_name
+# ENUMName -> ENUM_NAME, EnumName1 -> ENUM_NAME1
+# ENUM_NAME -> ENUM_NAME, ENUM_NAME1 -> ENUM_NAME1, ENUM_Name2 -> ENUM_NAME2
+# ENUM24_Name -> ENUM24_NAME
+def camel_to_upper(value):
+c_fun_str = c_name(value, False)
+if value.isupper():
+return c_fun_str
+
+new_name = ''
+l = len(c_fun_str)
+for i in range(l):
+c = c_fun_str[i]
+# When c is upper and no "_" appears before, do more checks
+if c.isupper() and (i > 0) and c_fun_str[i - 1] != "_":
+# Case 1: next string is lower
+# Case 2: previous string is digit
+if (i < (l - 1) and c_fun_str[i + 1].islower()) or \
+c_fun_str[i - 1].isdigit():
+new_name += '_'
+new_name += c
+return new_name.lstrip('_').upper()
+
+def c_enum_const(type_name, const_name):
+return camel_to_upper(type_name + '_' + const_name)
+
c_name_trans = string.maketrans('.-', '__')
def c_name(name, protect=True):
@@ -926,28 +951,3 @@ def guardend(name):
''',
name=guardname(name))
-
-# ENUMName -> ENUM_NAME, EnumName1 -> ENUM_NAME1
-# ENUM_NAME -> ENUM_NAME, ENUM_NAME1 -> ENUM_NAME1, ENUM_Name2 -> ENUM_NAME2
-# ENUM24_Name -> ENUM24_NAME
-def camel_to_upper(value):
-c_fun_str = c_name(value, False)
-if value.isupper():
-return c_fun_str
-
-new_name = ''
-l = len(c_fun_str)
-for i in range(l):
-c = c_fun_str[i]
-# When c is upper and no "_" appears before, do more checks
-if c.isupper() and (i > 0) and c_fun_str[i - 1] != "_":
-# Case 1: next string is lower
-# Case 2: previous string is digit
-if (i < (l - 1) and c_fun_str[i + 1].islower()) or \
-c_fun_str[i - 1].isdigit():
-new_name += '_'
-new_name += c
-return new_name.lstrip('_').upper()
-
-def c_enum_const(type_name, const_name):
-return camel_to_upper(type_name + '_' + const_name)
--
1.9.3
[Qemu-devel] [PULL 22/26] qapi: Fix generators to report command line errors decently
Report to stderr, prefix with the program name. Also reject extra arguments. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake --- scripts/qapi.py | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/scripts/qapi.py b/scripts/qapi.py index b97dd0b..df6e5aa 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -989,7 +989,7 @@ def parse_command_line(extra_options = "", extra_long_options = []): "input-file=", "output-dir="] + extra_long_options) except getopt.GetoptError, err: -print str(err) +print >>sys.stderr, "%s: %s" % (sys.argv[0], str(err)) sys.exit(1) output_dir = "" @@ -1017,4 +1017,8 @@ def parse_command_line(extra_options = "", extra_long_options = []): do_c = True do_h = True +if len(args) != 0: +print >>sys.stderr, "%s: too many arguments" % sys.argv[0] +sys.exit(1) + return (input_file, output_dir, do_c, do_h, prefix, extra_opts) -- 1.9.3
Re: [Qemu-devel] [PATCH 1/2] block: Detect multiplication overflow in bdrv_getlength
On Fri, 05/15 10:10, Markus Armbruster wrote:
> Fam Zheng writes:
>
> > Bogus image may have a large total_sectors that will overflow the
> > multiplication. For cleanness, fix the return code so the error message
> > will be meaningful.
> >
> > Reported-by: Richard W.M. Jones
> > Signed-off-by: Fam Zheng
> > ---
> > block.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/block.c b/block.c
> > index 7904098..5ee3fdf 100644
> > --- a/block.c
> > +++ b/block.c
> > @@ -2330,6 +2330,7 @@ int64_t bdrv_getlength(BlockDriverState *bs)
> > {
> > int64_t ret = bdrv_nb_sectors(bs);
> >
> > +ret = (int64_t)(ret * BDRV_SECTOR_SIZE) < 0 ? -EFBIG : ret;
> > return ret < 0 ? ret : ret * BDRV_SECTOR_SIZE;
> > }
>
> Signed integer overflow is undefined behavior. Your code works just
> fine on any remotely sane machine, *except* when the optimizer decides
> to use its undefined behavior license to mess with you.
>
> A more prudent way to test for overflow would be something like
>
> ret > INT64_MAX / BDRV_SECTOR_SIZE
Yes, this is better, will fix.
Thanks,
Fam
[Qemu-devel] [PULL 21/26] qapi: Factor parse_command_line() out of the generators
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
---
scripts/qapi-commands.py | 34 +++---
scripts/qapi-event.py| 32 +---
scripts/qapi-types.py| 36
scripts/qapi-visit.py| 35 ---
scripts/qapi.py | 40
5 files changed, 52 insertions(+), 125 deletions(-)
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index c94a19b..2889877 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -15,9 +15,7 @@
from ordereddict import OrderedDict
from qapi import *
import re
-import sys
import os
-import getopt
import errno
def generate_command_decl(name, args, ret_type):
@@ -376,42 +374,16 @@ def gen_command_def_prologue(prefix="", proxy=False):
ret += '#include "%sqmp-commands.h"' % prefix
return ret + "\n\n"
-
-try:
-opts, args = getopt.gnu_getopt(sys.argv[1:], "chp:i:o:m",
- ["source", "header", "prefix=",
-"input-file=", "output-dir=",
-"middle"])
-except getopt.GetoptError, err:
-print str(err)
-sys.exit(1)
-
-output_dir = ""
-prefix = ""
c_file = 'qmp-marshal.c'
h_file = 'qmp-commands.h'
middle_mode = False
-do_c = False
-do_h = False
+(input_file, output_dir, do_c, do_h, prefix, opts) = \
+parse_command_line("m", ["middle"])
for o, a in opts:
-if o in ("-p", "--prefix"):
-prefix = a
-elif o in ("-i", "--input-file"):
-input_file = a
-elif o in ("-o", "--output-dir"):
-output_dir = a + "/"
-elif o in ("-m", "--middle"):
+if o in ("-m", "--middle"):
middle_mode = True
-elif o in ("-c", "--source"):
-do_c = True
-elif o in ("-h", "--header"):
-do_h = True
-
-if not do_c and not do_h:
-do_c = True
-do_h = True
c_file = output_dir + prefix + c_file
h_file = output_dir + prefix + h_file
diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index 3e1f4cf..bc5ca4a 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -11,9 +11,7 @@
from ordereddict import OrderedDict
from qapi import *
-import sys
import os
-import getopt
import errno
def _generate_event_api_name(event_name, params):
@@ -219,38 +217,10 @@ const char *%(event_enum_name)s_lookup[] = {
# Start the real job
-try:
-opts, args = getopt.gnu_getopt(sys.argv[1:], "chp:i:o:",
- ["source", "header", "prefix=",
-"input-file=", "output-dir="])
-except getopt.GetoptError, err:
-print str(err)
-sys.exit(1)
-
-input_file = ""
-output_dir = ""
-prefix = ""
c_file = 'qapi-event.c'
h_file = 'qapi-event.h'
-do_c = False
-do_h = False
-
-for o, a in opts:
-if o in ("-p", "--prefix"):
-prefix = a
-elif o in ("-i", "--input-file"):
-input_file = a
-elif o in ("-o", "--output-dir"):
-output_dir = a + "/"
-elif o in ("-c", "--source"):
-do_c = True
-elif o in ("-h", "--header"):
-do_h = True
-
-if not do_c and not do_h:
-do_c = True
-do_h = True
+(input_file, output_dir, do_c, do_h, prefix, dummy) = parse_command_line()
c_file = output_dir + prefix + c_file
h_file = output_dir + prefix + h_file
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 5665145..62044c1 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -11,9 +11,7 @@
from ordereddict import OrderedDict
from qapi import *
-import sys
import os
-import getopt
import errno
def generate_fwd_struct(name, members, builtin_type=False):
@@ -275,43 +273,17 @@ void qapi_free_%(name)s(%(c_type)s obj)
c_type=c_type(name), name=c_name(name))
return ret
-
-try:
-opts, args = getopt.gnu_getopt(sys.argv[1:], "chbp:i:o:",
- ["source", "header", "builtins",
-"prefix=", "input-file=", "output-dir="])
-except getopt.GetoptError, err:
-print str(err)
-sys.exit(1)
-
-output_dir = ""
-input_file = ""
-prefix = ""
c_file = 'qapi-types.c'
h_file = 'qapi-types.h'
-
-do_c = False
-do_h = False
do_builtins = False
+(input_file, output_dir, do_c, do_h, prefix, opts) = \
+parse_command_line("b", ["builtins"])
+
for o, a in opts:
-if o in ("-p", "--prefix"):
-prefix = a
-elif o in ("-i", "--input-file"):
-input_file = a
-elif o in ("-o", "--output-dir"):
-output_dir = a + "/"
-elif o in ("-c", "--source"):
-do_c = True
-elif o in ("-h", "--header"):
-do_h = True
-elif o in ("-b", "--builtins"):
+if o in ("-b", "--builtins"):
do_builtins = True
-if not do_c and not do_h:
-do_c = True
-do_h = True
-
c_file = output_dir + prefix + c_file
h_file = output_dir + prefix
[Qemu-devel] [PATCH v2 1/2] block: Detect multiplication overflow in bdrv_getlength
Bogus image may have a large total_sectors that will overflow the
multiplication. For cleanness, fix the return code so the error message
will be meaningful.
Reported-by: Richard W.M. Jones
Signed-off-by: Fam Zheng
---
block.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/block.c b/block.c
index 7904098..5d271a1 100644
--- a/block.c
+++ b/block.c
@@ -2330,6 +2330,7 @@ int64_t bdrv_getlength(BlockDriverState *bs)
{
int64_t ret = bdrv_nb_sectors(bs);
+ret = ret > INT64_MAX / BDRV_SECTOR_SIZE ? -EFBIG : ret;
return ret < 0 ? ret : ret * BDRV_SECTOR_SIZE;
}
--
2.4.0
[Qemu-devel] [PATCH v2 2/2] qemu-iotests: qemu-img info on afl VMDK image with a huge capacity
The image is contributed by Richard W.M. Jones.
Cc: Richard W.M. Jones
Signed-off-by: Fam Zheng
Reviewed-by: Alberto Garcia
---
tests/qemu-iotests/059 | 5 +
tests/qemu-iotests/059.out | 3 +++
tests/qemu-iotests/sample_images/afl9.vmdk.bz2 | Bin 0 -> 178 bytes
3 files changed, 8 insertions(+)
create mode 100644 tests/qemu-iotests/sample_images/afl9.vmdk.bz2
diff --git a/tests/qemu-iotests/059 b/tests/qemu-iotests/059
index 50ca5ce..0ded0c3 100755
--- a/tests/qemu-iotests/059
+++ b/tests/qemu-iotests/059
@@ -132,6 +132,11 @@ _img_info
$QEMU_IO -c "write -P 0xa 900G 512" "$TEST_IMG" | _filter_qemu_io
$QEMU_IO -c "read -v 900G 1024" "$TEST_IMG" | _filter_qemu_io
+echo
+echo "=== Testing afl image with a very large capacity ==="
+_use_sample_img afl9.vmdk.bz2
+_img_info
+
# success, all done
echo "*** done"
rm -f $seq.full
diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out
index cbb0de4..67e3cf5 100644
--- a/tests/qemu-iotests/059.out
+++ b/tests/qemu-iotests/059.out
@@ -2336,4 +2336,7 @@ e103e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00
e103f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
read 1024/1024 bytes at offset 966367641600
1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+=== Testing afl image with a very large capacity ===
+qemu-img: Can't get size of device 'image': File too large
*** done
diff --git a/tests/qemu-iotests/sample_images/afl9.vmdk.bz2
b/tests/qemu-iotests/sample_images/afl9.vmdk.bz2
new file mode 100644
index
..03615d36a12425cf4240bab86f4cfe648db14572
GIT binary patch
literal 178
zcmV;j08RfwT4*^jL0KkKS>A08g#Z9x|HJ$H)ZJi0004xF0SE*D03g5s00IDLSQelF
ziVX^$pfWNUJrmRhn2k52pQ;Rs0EQC;(S%|!m`2~BZ@b++;etskRJUVl!Kt)wu7?VN
zl;%JdqX2?TgsNVJP?87M*MvL1qQnBkCES&?0@MeaN-bL4;bDzxmMm|da4fuh!=#fu
g@i9R@5z!av{9tA
[Qemu-devel] [PATCH v2 0/2] block: Fix error code for bdrv_getlength when the image is too big
v2: Correct detection of overflow. [Markus, Berto] If the image has a huge enough virtual size, $ qemu-img info afl9.img qemu-img: Can't get size of device 'image': Unknown error -512 It's because of the multiplication overflow in the return statement in bdrv_getlength (the big nagetive value is later truncated to 0x200). Fix it to return -EFBIG: qemu-img: Can't get size of device 'image': File too large Bug reported by Richard Jones in: https://bugzilla.redhat.com/show_bug.cgi?id=1221499 Fam Zheng (2): block: Detect multiplication overflow in bdrv_getlength qemu-iotests: qemu-img info on afl VMDK image with a huge capacity block.c| 1 + tests/qemu-iotests/059 | 5 + tests/qemu-iotests/059.out | 3 +++ tests/qemu-iotests/sample_images/afl9.vmdk.bz2 | Bin 0 -> 178 bytes 4 files changed, 9 insertions(+) create mode 100644 tests/qemu-iotests/sample_images/afl9.vmdk.bz2 -- 2.4.0
Re: [Qemu-devel] [PATCH 05/15] tap: net_tap_fd_init() can't fail, drop dead error handling
Eric Blake writes: > On 05/12/2015 06:02 AM, Markus Armbruster wrote: >> Signed-off-by: Markus Armbruster >> --- >> net/tap.c | 14 +- >> 1 file changed, 1 insertion(+), 13 deletions(-) >> > >> @@ -552,14 +551,8 @@ int net_init_bridge(const NetClientOptions *opts, const >> char *name, >> } >> >> fcntl(fd, F_SETFL, O_NONBLOCK); > > Well, fcntl() could fail. And don't we have the helper function > util/oslib-posix.c:qemu_set_nonblock() for doing this correctly? (Then > again, that helper also ignores failures). > > But the raw use of fcntl here is pre-existing and you didn't touch it, > so it doesn't affect my review of this patch. I guess using the helper makes some sense here, but it's outside this series' scope. > Reviewed-by: Eric Blake Thanks!
Re: [Qemu-devel] [PATCH 03/15] net: Improve -net nic error reporting
Eric Blake writes:
> On 05/12/2015 06:02 AM, Markus Armbruster wrote:
>> When -net nic fails, it first reports a specific error, then a generic
>> one, like this:
>>
>> $ qemu-system-x86_64 -net nic,netdev=nonexistant
>> qemu-system-x86_64: -net nic,netdev=nonexistant: netdev
>> nonexistant' not found
>> qemu-system-x86_64: -net nic,netdev=nonexistant: Device 'nic'
>> could not be initialized
>
> s/nonexistant/nonexistent/g
Fixing...
>>
>> Convert net_init_nic() to Error to get rid of the unwanted second
>> error message.
>>
>> Signed-off-by: Markus Armbruster
>> ---
>> net/net.c | 12 ++--
>> 1 file changed, 6 insertions(+), 6 deletions(-)
>>
>
>> @@ -745,7 +744,7 @@ static int net_init_nic(const NetClientOptions *opts,
>> const char *name,
>>
>> idx = nic_get_free_idx();
>> if (idx == -1 || nb_nics >= MAX_NICS) {
>> -error_report("Too Many NICs");
>> +error_setg(errp, "Too Many NICs");
>
> worth s/Many/many/ while touching this?
Yup.
> Reviewed-by: Eric Blake
Thanks!
Re: [Qemu-devel] [PATCH 12/15] tap-bsd: Convert tap_open() to Error
Eric Blake writes:
> On 05/12/2015 06:03 AM, Markus Armbruster wrote:
>> Fixes inappropriate use of stderr in monitor command handler.
>>
>> While there, improve some of the messages a bit.
>>
>> Signed-off-by: Markus Armbruster
>> ---
>> net/tap-bsd.c | 33 ++---
>> 1 file changed, 14 insertions(+), 19 deletions(-)
>>
>
>> @@ -139,14 +133,15 @@ int tap_open(char *ifname, int ifname_size, int
>> *vnet_hdr,
>> /* User requested the interface to have a specific name */
>> s = socket(AF_LOCAL, SOCK_DGRAM, 0);
>> if (s < 0) {
>> -error_report("could not open socket to set interface name");
>> +error_setg_errno(errp, errno,
>> + "could not open socket to set interface name");
>> goto error;
>> }
>> ifr.ifr_data = ifname;
>> ret = ioctl(s, SIOCSIFNAME, (void *)&ifr);
>> close(s);
>> if (ret < 0) {
>> -error_report("could not set tap interface name");
>> +error_setg_errno(errp, errno, "could not set tap interface
>> name");
>
> Bad. close() may have clobbered errno before you get to report it.
You're right. I'll dumb this down again.
>> goto error;
>> }
>> } else {
>> @@ -158,14 +153,14 @@ int tap_open(char *ifname, int ifname_size, int
>> *vnet_hdr,
>> *vnet_hdr = 0;
>>
>> if (vnet_hdr_required && !*vnet_hdr) {
>> -error_report("vnet_hdr=1 requested, but no kernel "
>> - "support for IFF_VNET_HDR available");
>> +error_setg(errp, "vnet_hdr=1 requested, but no kernel "
>> + "support for IFF_VNET_HDR available");
>> goto error;
>> }
>> }
>> if (mq_required) {
>> -error_report("mq_required requested, but not kernel support"
>> - "for IFF_MULTI_QUEUE available");
>> +error_setg(errp, "mq_required requested, but not kernel support"
>
> As long as you are touching this, s/not/no/
Fixing...
>> + "for IFF_MULTI_QUEUE available");
>> goto error;
>> }
Re: [Qemu-devel] [PATCH v2 1/2] block: Detect multiplication overflow in bdrv_getlength
On Fri 15 May 2015 10:36:05 AM CEST, Fam Zheng wrote: > Bogus image may have a large total_sectors that will overflow the > multiplication. For cleanness, fix the return code so the error message > will be meaningful. > > Reported-by: Richard W.M. Jones > Signed-off-by: Fam Zheng Reviewed-by: Alberto Garcia Berto
Re: [Qemu-devel] [PATCH 13/15] tap-solaris: Convert tap_open() to Error
Eric Blake writes:
> On 05/12/2015 06:03 AM, Markus Armbruster wrote:
>> Fixes inappropriate use of syslog().
>>
>> Not fixed: leaks on error paths, suspicious non-fatal errors. FIXMEs
>> added instead.
>
> At least you're admitting where the code is still bad.
Actually, git-rm felt pretty tempting.
>> Signed-off-by: Markus Armbruster
>> ---
>> net/tap-solaris.c | 59
>> ---
>> 1 file changed, 30 insertions(+), 29 deletions(-)
>>
>
>> @@ -99,20 +100,20 @@ static int tap_alloc(char *dev, size_t dev_size)
>> strioc_ppa.ic_len = sizeof(ppa);
>> strioc_ppa.ic_dp = (char *)&ppa;
>> if ((ppa = ioctl (tap_fd, I_STR, &strioc_ppa)) < 0)
>> - syslog (LOG_ERR, "Can't assign new interface");
>> +error_report("Can't assign new interface");
>
> I see you're fixing spacing while at it, as well.
>
>>
>> TFR(if_fd = open("/dev/tap", O_RDWR, 0));
>> if (if_fd < 0) {
>> - syslog(LOG_ERR, "Can't open /dev/tap (2)");
>> - return -1;
>> +error_setg(errp, "Can't open /dev/tap (2)");
>> +return -1;
>> }
>> if(ioctl(if_fd, I_PUSH, "ip") < 0){
>> - syslog(LOG_ERR, "Can't push IP module");
>
> Should you add the space after 'if' while touching this?
Fixing up just enough to make checkpatch happy. Also keeps git-blame
useful even without -w.
>> - return -1;
>> +error_setg(errp, "Can't push IP module");
>> +return -1;
>> }
>>
>> if (ioctl(if_fd, SIOCGLIFFLAGS, &ifr) < 0)
>> -syslog(LOG_ERR, "Can't get flags\n");
>> +error_report("Can't get flags");
>
> What about adding missing {} while touching this file? Hmm - there's
> enough cruft that it may involve a separate patch just to clean up
> style. For this patch, I'm not going to hold up review on style.
>
> Reviewed-by: Eric Blake
Thanks!
Re: [Qemu-devel] [PATCH 15/15] tap: Improve -netdev/netdev_add/-net/... tap error reporting
Eric Blake writes:
> On 05/12/2015 06:03 AM, Markus Armbruster wrote:
>> When -netdev tap fails, it first reports a specific error, then a
>> generic one, like this:
>>
>> $ qemu-system-x86_64 -netdev tap,id=foo
>> qemu-system-x86_64: -netdev tap,id=foo: could not configure
>> /dev/net/tun: Operation not permitted
>> qemu-system-x86_64: -netdev tap,id=foo: Device 'tap' could not
>> be initialized
>>
>> With the command line, the messages go to stderr. In HMP, they go to
>> the monitor. In QMP, the second one becomes the error reply, and the
>> first one goes to stderr.
>>
>> Convert net_init_tap() to Error. This suppresses the unwanted second
>> message, and and makes the specific error the QMP error reply.
>
> s/and and/and/
Fixing...
>> Signed-off-by: Markus Armbruster
>> ---
>> net/tap.c | 45 ++---
>> 1 file changed, 22 insertions(+), 23 deletions(-)
>>
>
>
>> @@ -807,15 +807,15 @@ int net_init_tap(const NetClientOptions *opts, const
>> char *name,
>> tap->has_vhostfds ? vhost_fds[i] : NULL,
>> vnet_hdr, fd, &err);
>> if (err) {
>> -error_report_err(err);
>> +error_propagate(errp, err);
>> return -1;
>> }
>> }
>> } else if (tap->has_helper) {
>> if (tap->has_ifname || tap->has_script || tap->has_downscript ||
>> tap->has_vnet_hdr || tap->has_queues || tap->has_vhostfds) {
>> -error_report("ifname=, script=, downscript=, and vnet_hdr= "
>> - "queues=, and vhostfds= are invalid with helper=");
>> +error_setg(errp, "ifname=, script=, downscript=, and vnet_hdr= "
>> + "queues=, and vhostfds= are invalid with helper=");
>
> As long as you are touching this, s/and vnet_hdr=/vnet_hdr=,/
Yes.
> Minor enough that I'm okay with fixing it, and adding:
> Reviewed-by: Eric Blake
Thanks!
Re: [Qemu-devel] [PATCH 11/11] qemu-iotests: test overlapping block-stream operations
On Fri 15 May 2015 04:56:09 AM CEST, Fam Zheng wrote: >> +# If node4 is the active node, the id of the block job is drive0 >> +if self.num_imgs == 5: > > Isn't self.num_imgs a constant (9) ? Yes, but the number is arbitrary. The idea is to let users change it if they want to, so the test case must be prepared for that. Berto
Re: [Qemu-devel] [RFC/RFT PATCH v2 0/3] KVM: Introduce KVM_MEM_UNCACHED
On 14 May 2015 at 16:41, Michael S. Tsirkin wrote: > On Thu, May 14, 2015 at 04:19:23PM +0200, Laszlo Ersek wrote: >> On 05/14/15 15:48, Michael S. Tsirkin wrote: >> > On Thu, May 14, 2015 at 03:32:10PM +0200, Laszlo Ersek wrote: >> >> On 05/14/15 15:00, Andrew Jones wrote: >> >>> On Thu, May 14, 2015 at 01:38:11PM +0100, Peter Maydell wrote: >> On 14 May 2015 at 13:28, Paolo Bonzini wrote: >> > Well, PCI BARs are generally MMIO resources, and hence should not be >> > cached. >> > >> > As an optimization, OS drivers can mark them as cacheable or >> > write-combining or something like that, but in general it's a safe >> > default to leave them uncached---one would think. >> >> Isn't this handled by the OS mapping them in the 'prefetchable' >> MMIO window rather than the 'non-prefetchable' one? (QEMU's >> generic-PCIe device doesn't yet support the prefetchable window.) >> >>> >> >>> I was thinking (with my limited PCI knowledge) the same thing, and >> >>> was planning on experimenting with that. >> >> >> >> This could be supported in UEFI as well, with the following steps: >> >> - the DTB that QEMU provides UEFI with should advertise such a >> >> prefetchable window. >> >> - The driver in UEFI that parses the DTB should understand that DTB >> >> node (well, record type), and store the appropriate base & size into >> >> some new dynamic PCDs (= basically, firmware wide global variables; >> >> PCD = platform configuration database) >> >> - The entry point of the host bridge driver would call >> >> gDS->AddMemorySpace() twice, separately for the two different windows, >> >> with their appropriate caching attributes. >> >> - The host bridge driver needs to be extended so that TypePMem32 >> >> requests are not rejected (like now); they should be handled >> >> similarly to TypeMem32. Except, the gDS->AllocateMemorySpace() call >> >> should allocate from the prefetchable range (determined by the new >> >> PCDs above). >> >> - QEMU's emulated devices should then expose their BARs as prefetchable >> >> (so that the above branch would be taken in the host bridge driver). >> >> >> >> (Of course, if QEMU intends to emulate PCI devices somewhat >> >> realistically, then QEMU should claim "non-prefetchable" for BARs that >> >> would not be prefetchable on physical hardware either, and then the >> >> hypervisor should accommodate the firmware's UC mapping and say "hey I >> >> know better, we're virtual in fact", and override the attribute (-> use >> >> WB instead of UC). With which we'd be back to square one...) >> >> >> >> Thanks >> >> Laszlo >> > >> > Prefetcheable is unrelated to BAR caching or drivers, it's a way to tell >> > host bridges they can do limited tweaks to downstream transactions in a >> > specific range. >> > >> > Really non-prefetcheable BARs are mostly those where read has >> > side-effects, which is best avoided. this does not mean it's ok to >> > reorder transactions or cache them. >> >> I believe I understood that (although certainly not in the depth that >> you do), because when the idea had come up first (ie. equating cacheable >> with prefetchable, or at least "repurposing" the latter for the former) >> I had tried to read up on prefetchable (just on the web; no time for >> reading the PCI spec. ... I peeked now, it also mentions "write merging" >> for bridges.) > > Read up on what it is if you like, it is much weaker than WC not to > mention cacheable. > >> The way I perceived it, the idea was to give the guest a >> hint about caching with the prefetchable bit / DTB entry. Sorry if I was >> mistaken. >> >> Thanks >> Laszlo > > And what I am saying is that prefetchable bit would be a PV solution - > on real devices it is not a hint about caching and can't be used as > such. > On a general note, may I point out that while this discussion now focuses heavily on PCI and its metadata that could potentially describe the cached/uncached nature of a region, there are other emulated devices that are affected as well. Most notably, there is the emulated NOR flash which is backed by a read-only memslot while in array mode, but treated as a device by the guest and hence mapped uncached. Since the NOR flash contains the executable image of the firmware (in case of UEFI), it must be backed by actual host RAM or the CPU won't be able to fetch instructions from it (since instruction fetches cannot be emulated like ordinary loads and stores). On the other hand, since the guest treats it as a ROM, it is totally oblivious of any caching concerns that may exist.
Re: [Qemu-devel] [PATCH 04/11] block: Support streaming to an intermediate layer
On Fri 15 May 2015 04:33:19 AM CEST, Fam Zheng wrote:
>> +/* Make sure that the image is opened in read-write mode */
>> +orig_bs_flags = bdrv_get_flags(bs);
>> +if (!(orig_bs_flags & BDRV_O_RDWR)) {
>> +if (bdrv_reopen(bs, orig_bs_flags | BDRV_O_RDWR, errp) != 0) {
>
> I don't think all bdrv_reopen implementation will guarantee to set errp
> on error :(. For example:
>
> static int qcow2_reopen_prepare(BDRVReopenState *state,
> BlockReopenQueue *queue, Error **errp)
> {
bdrv_reopen_prepare() already sets a generic error if the implementation
(qcow2_reopen_prepare() in this case) doesn't provide one:
ret = drv->bdrv_reopen_prepare(reopen_state, queue, &local_err);
if (ret) {
if (local_err != NULL) {
error_propagate(errp, local_err);
} else {
error_setg(errp, "failed while preparing to reopen image '%s'",
reopen_state->bs->filename);
}
goto error;
}
Berto
Re: [Qemu-devel] [PATCH 0/3] scripts/qemu-gdb: Add event tracing support
On 15 May 2015 at 08:58, Markus Armbruster wrote: > Since you're touching qemu-gdb.py anyway, could you stick in a brief > comment explaining how to put it to use? Good idea. It turns out the answer is just "source it from gdb", but it took me a little while to find that out, so worth commenting. I also have a patch which makes it do the 'ignore SIGUSR1' bit by doing 'handle SIGUSR1 pass noprint nostop' for you. -- PMM
Re: [Qemu-devel] [PATCH 04/11] block: Support streaming to an intermediate layer
On Fri, 05/15 11:04, Alberto Garcia wrote:
> On Fri 15 May 2015 04:33:19 AM CEST, Fam Zheng wrote:
>
> >> +/* Make sure that the image is opened in read-write mode */
> >> +orig_bs_flags = bdrv_get_flags(bs);
> >> +if (!(orig_bs_flags & BDRV_O_RDWR)) {
> >> +if (bdrv_reopen(bs, orig_bs_flags | BDRV_O_RDWR, errp) != 0) {
> >
> > I don't think all bdrv_reopen implementation will guarantee to set errp
> > on error :(. For example:
> >
> > static int qcow2_reopen_prepare(BDRVReopenState *state,
> > BlockReopenQueue *queue, Error **errp)
> > {
>
> bdrv_reopen_prepare() already sets a generic error if the implementation
> (qcow2_reopen_prepare() in this case) doesn't provide one:
>
> ret = drv->bdrv_reopen_prepare(reopen_state, queue, &local_err);
> if (ret) {
> if (local_err != NULL) {
> error_propagate(errp, local_err);
> } else {
> error_setg(errp, "failed while preparing to reopen image '%s'",
>reopen_state->bs->filename);
> }
> goto error;
> }
>
You're right, I missed that. So,
Reviewed-by: Fam Zheng
Re: [Qemu-devel] [PATCH 11/11] qemu-iotests: test overlapping block-stream operations
On Fri, 05/15 10:58, Alberto Garcia wrote: > On Fri 15 May 2015 04:56:09 AM CEST, Fam Zheng wrote: > > >> +# If node4 is the active node, the id of the block job is drive0 > >> +if self.num_imgs == 5: > > > > Isn't self.num_imgs a constant (9) ? > > Yes, but the number is arbitrary. The idea is to let users change it if > they want to, so the test case must be prepared for that. > They have to change the code to get a different value, no? This is unreachable code as is and IMO even if you're not as considerate on this, whoever changes num_ops should make sure code that uses it still works. This is bikeshedding, this is a great series, so even if you insist, Reviewed-by: Fam Zheng
[Qemu-devel] [PATCH v2] util: socket: Add missing localaddr and localport option for DGRAM socket
The 'socket_optslist' structure does not contain the 'localaddr' and
'localport' options that are parsed in case you are creating a
'connect' type UDP character device.
I've noticed it happening after commit f43e47dbf6de24db20ec9b588bb6cc762
made qemu abort() after seeing the invalid option.
A minimal reproducer for the case is:
$ qemu-system-x86_64 -chardev
udp,id=charrng0,host=127.0.0.1,port=1234,localaddr=,localport=1234
qemu-system-x86_64: -chardev
udp,id=charrng0,host=127.0.0.1,port=1234,localaddr=,localport=1234: Invalid
parameter 'localaddr'
Aborted (core dumped)
Prior to the commit mentioned above the error would be printed but the
value for localaddr and localport was simply ignored. I did not go
trhough the code to find out when it was broken.
Add the two fields so that the options can again be parsed correctly and
qemu doesn't abort().
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1220252
Signed-off-by: Peter Krempa
Reviewed-by: Eric Blake
---
Notes:
Version 2:
- improved commit message as suggested by Markus
Cc: Markus Armbruster
util/qemu-sockets.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 87c9bc6..72066be 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -45,6 +45,12 @@ QemuOptsList socket_optslist = {
.name = "port",
.type = QEMU_OPT_STRING,
},{
+.name = "localaddr",
+.type = QEMU_OPT_STRING,
+},{
+.name = "localport",
+.type = QEMU_OPT_STRING,
+},{
.name = "to",
.type = QEMU_OPT_NUMBER,
},{
--
2.3.5
Re: [Qemu-devel] [PATCH] Do not emulate a floppy drive when -nodefaults
On Fri, 15 May 2015, Paolo Bonzini wrote: > On 15/05/2015 09:50, Markus Armbruster wrote: > > > --nodefaults must continue to disable all optional parts of the board. > > > > What exactly is optional is for the board / machine type to define. It > > can't be changed once the machine type is released. > > > > When in doubt, make it optional. > > I agree entirely. This unfortunately applies only to future boards. > > > Since Q35 is just starting to become migratable, the time to painlessly > > change its optional parts is *now*. > > I can buy this. > > Let's just not divert attention from more important stuff. I may be > more inclined to accept patches, if we add something to our security > policy about disliking cute backronyms that attract bad journalism like > magnets. And I'm only half joking; more like only 1% joking. +1
Re: [Qemu-devel] Regression: qemu crash of hvm domUs with spice (backtrace included)
On Wed, 13 May 2015, Fabio Fantoni wrote: > Il 12/05/2015 16:44, Stefano Stabellini ha scritto: > > On Tue, 12 May 2015, Stefano Stabellini wrote: > > > On Tue, 12 May 2015, Fabio Fantoni wrote: > > > > Il 12/05/2015 12:26, Fabio Fantoni ha scritto: > > > > > Il 12/05/2015 11:23, Fabio Fantoni ha scritto: > > > > > > Il 11/05/2015 17:04, Fabio Fantoni ha scritto: > > > > > > > Il 21/04/2015 14:53, Stefano Stabellini ha scritto: > > > > > > > > On Tue, 21 Apr 2015, Fabio Fantoni wrote: > > > > > > > > > Il 21/04/2015 12:49, Stefano Stabellini ha scritto: > > > > > > > > > > On Mon, 20 Apr 2015, Fabio Fantoni wrote: > > > > > > > > > > > I updated xen and qemu from xen 4.5.0 with its upstream > > > > > > > > > > > qemu > > > > > > > > > > > included to > > > > > > > > > > > xen > > > > > > > > > > > 4.5.1-pre with qemu upstream from stable-4.5 (changed > > > > > > > > > > > Config.mk > > > > > > > > > > > to use > > > > > > > > > > > revision "master"). > > > > > > > > > > > After few minutes I booted windows 7 64 bit domU qemu > > > > > > > > > > > crash, > > > > > > > > > > > tried 2 times > > > > > > > > > > > with same result. > > > > > > > > > > > > > > > > > > > > > > In the domU's qemu log: > > > > > > > > > > > > qemu-system-i386: malloc.c:3096: sYSMALLOc: Assertion > > > > > > > > > > > > `(old_top == > > > > > > > > > > > > (((mbinptr) (((char *) &((av)->bins[((1) - 1) * 2])) - > > > > > > > > > > > > __builtin_offsetof > > > > > > > > > > > > (struct malloc_chunk, fd && old_size == 0) || > > > > > > > > > > > > ((unsigned > > > > > > > > > > > > long) > > > > > > > > > > > > (old_size) >= (unsigned long)__builtin_offsetof > > > > > > > > > > > > (struct > > > > > > > > > > > > malloc_chunk, > > > > > > > > > > > > fd_nextsize))+((2 * (sizeof(size_t))) - 1)) & ~((2 * > > > > > > > > > > > > (sizeof(size_t))) - > > > > > > > > > > > > 1))) && ((old_top)->size & 0x1) && ((unsigned > > > > > > > > > > > > long)old_end & > > > > > > > > > > > > pagemask) > > > > > > > > > > > > == > > > > > > > > > > > > 0)' failed. > > > > > > > > > > > > Killing all inferiors > > > > > > > > > > > In attachment the full backtrace of qemu crash. > > > > > > > > > > > > > > > > > > > > > > With a fast search after I saw the backtrace I found a > > > > > > > > > > > probable > > > > > > > > > > > cause of > > > > > > > > > > > regression (I'm not sure): > > > > > > > > > > > http://xenbits.xen.org/gitweb/?p=staging/qemu-upstream-4.5-testing.git;a=commit;h=5c3402816aaddb15156c69df73c54abe4e1c76aa > > > > > > > > > > > spice: make sure we don't overflow ssd->buf > > > > > > > > > > > > > > > > > > > > > > Added also qemu-devel and spice-devel as cc. > > > > > > > > > > > > > > > > > > > > > > If you need more informations/tests tell me and I'll post > > > > > > > > > > > them. > > > > > > > > > > Maybe you could try to revert the offending commit > > > > > > > > > > (5c3402816aaddb15156c69df73c54abe4e1c76aa)? Or even better > > > > > > > > > > bisect > > > > > > > > > > the > > > > > > > > > > crash? > > > > > > > > > Thanks for your reply. > > > > > > > > > > > > > > > > > > I reverted to 4.5.0 on dom0 for now on that system because I'm > > > > > > > > > busy > > > > > > > > > trying to > > > > > > > > > found another problem that cause very bad performance without > > > > > > > > > errors > > > > > > > > > or > > > > > > > > > nothing in logs :( I don't know if if xen related, kernel > > > > > > > > > related or > > > > > > > > > other for > > > > > > > > > now. > > > > > > > > > > > > > > > > > > About this regression with spice I'll do further tests in next > > > > > > > > > days > > > > > > > > > (probably > > > > > > > > > starting reverting the spice patch in qemu) but any help is > > > > > > > > > appreciated. > > > > > > > > > Based on data I have for now is possible that the problem is > > > > > > > > > that > > > > > > > > > qemu try to > > > > > > > > > allocate other ram or videoram after domU create but with xen > > > > > > > > > is not > > > > > > > > > possible? > > > > > > > > > In the spice related patch I saw something about dynamic > > > > > > > > > allocation > > > > > > > > > for > > > > > > > > > example. > > > > > > > > It is probably caused by a commit in the range: > > > > > > > > > > > > > > > > 1ebb75b1fee779621b63e84fefa7b07354c43a99..0b8fb1ec3d666d1eb8bbff56c76c5e6daa2789e4 > > > > > > > > > > > > > > > > there are only 10 commits in that range. By using git bisect you > > > > > > > > should > > > > > > > > be able to narrow it down in just 3 tests. > > > > > > > Sorry for delay, I was busy with many things, today I retried with > > > > > > > updated stable-4.5 and also reverting "spice: make sure we don't > > > > > > > overflow ssd->buf" (in a second test) but in both case regression > > > > > > > remain > > > > > > > :( > > > > > > > Tomorrow probably I'll do other tests. > > > > > > I did another test, reverting this instead: > > > > > > http://xenbits.xen.org/gitweb/?p
Re: [Qemu-devel] [PATCH v6 03/47] qemu_ram_foreach_block: pass up error value, and down the ramblock name
On (Tue) 14 Apr 2015 [18:03:29], Dr. David Alan Gilbert (git) wrote: > From: "Dr. David Alan Gilbert" > > check the return value of the function it calls and error if it's non-0 > Fixup qemu_rdma_init_one_block that is the only current caller, > and rdma_add_block the only function it calls using it. > > Pass the name of the ramblock to the function; helps in debugging. Reviewed-by: Amit Shah Amit
Re: [Qemu-devel] [RFC PATCH 21/34] arm: Rename all exceptions
Am 15.05.2015 um 07:43 schrieb Peter Crosthwaite: > On Sun, May 10, 2015 at 11:29 PM, Peter Crosthwaite > wrote: >> These are architecture specific, and via cpu.h visibile in common >> and global namespaces. Preface them with "ARMAR_" to avoid namespace >> collisions. Prepares support for multi-arch where multiple cpu.h's >> can be included by device land code and namespace issues happen with >> such generic names. >> >> Use prefix ARM"AR" as the trap table is separate from the M-profile >> support, so qualify with AR to make it specific to A/R profile. ARM_AR_ would sound more appealing to me. > So I am not exactly sure what to do here going forward. This is going > to get messy with all the other arches. There are alternatives: > > 1: Split these arch-specific private defs to a new header. internals.h > or a new header. which every way we go though the header needs to be > exported to linux-user code (awkward). > 2: Purge all device-land uses of cpu.h. They should be able to use > cpu-qom.h Negative, my plans to make cpu-qom.h generally usable failed as env turned out as embedded struct rather than pointer, and cpu-qom.h thus depends on stuff defined in cpu.h before its inclusion of cpu-qom.h. Therefore I told contributors of new targets that the current split makes no sense for their new targets. I would prefer 1. independent of whether we rename them or not. We need a better distinction of internal vs. external for targets. Regards, Andreas > and the random bits of machine-model code reaching into the > env or strobing interrupts needs to be fixed. > 3: This patch or something like it. > > Regards, > Peter -- SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB 21284 (AG Nürnberg)
Re: [Qemu-devel] [RFC PATCH 00/34] Multi Architecture System Emulation
Am 15.05.2015 um 08:47 schrieb Peter Crosthwaite: > On Mon, May 11, 2015 at 3:27 AM, Andreas Färber wrote: >> Hi, >> >> Am 11.05.2015 um 08:29 schrieb Peter Crosthwaite: >>> Microblaze translation needs a change pattern to allow conversion to 64-bit >>> TARGET_LONG. Uses of TCGv need to be removed and explicited to 32-bit. >> >> I did have a patchset doing exactly that for one of my microcontroller >> targets but someone (Paul?) turned it down as unnecessary, saying that >> by definition there were no difference between TCGv and TCGv_iXX. ;) >> >> Reviving it should be rather easy. >> > > Is it an out-of-tree new-target or conversion of something existing? > For the latter cc me, i'll give a review. It's not about a conversion, it's about the helper infrastructure to do compile-time detection of such mismatches of TCGv vs. TCGv_i32 for select targets. http://patchwork.ozlabs.org/patch/130512/ (and preceding ones) Regards, Andreas -- SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB 21284 (AG Nürnberg)
Re: [Qemu-devel] [RFC v1 3/3] target-microblaze: Convert use-fpu to a CPU property
Am 15.05.2015 um 08:36 schrieb Alistair Francis:
> On Fri, May 15, 2015 at 4:32 PM, Peter Crosthwaite
> wrote:
>> On Thu, May 14, 2015 at 10:56 PM, Alistair Francis
>> wrote:
>>> On Fri, May 15, 2015 at 3:52 PM, Peter Crosthwaite
>>> wrote:
On Thu, May 14, 2015 at 10:48 PM, Alistair Francis
wrote:
> On Fri, May 15, 2015 at 3:22 PM, Peter Crosthwaite
> wrote:
>> On Wed, May 13, 2015 at 11:08 PM, Alistair Francis
>> wrote:
>>> | PVR2_FPU_EXC_MASK \
>>> | 0;
>>> +
>>> +if (cpu->cfg.usefpu) {
>>> +env->pvr.regs[0] |= PVR0_USE_FPU_MASK;
>>> +env->pvr.regs[2] |= PVR2_USE_FPU_MASK;
>>> +
>>> +if (cpu->cfg.usefpu > 1) {
>>> +env->pvr.regs[2] |= PVR2_USE_FPU2_MASK;
>>> +}
>>> +}
>>
>> This should be handled at realize time. See pl330_realize for example
>> of realize-time PVR ("cfg" in that case) population.
>
> But the env state gets wiped out at reset, so the information will be
> lost.
Ahh, so the solution there is to do what ARM does and have a section
at the back of the env which survives reset. Check the memset in
arm_cpu_reset.
>>>
>>> Ok, just to clarify we want all of the pvr registers to be preserved on
>>> reset?
>>
>> yes. But something that just occured to me, does it make sense to move
>> it outside the env? into the CPUState? Andreas mentioned that fields
>> in the CPU state before the env can be used with negative env* offsets
>> by translated code. This means the PVR could just be pushed up to
>> CPUState.
>
> Do any other machines do that?
>
> I'm happy with leaving it in the env (partly because I just did it)
> and it also matches the way that ARM does it.
All targets had everything in env originally, so that's not really an
argument for anything. You need to decide what makes sense for your
target - icount is an example using negative offsets in CPUState now,
whereas TCGv* variables declared via offset from env remained in env.
If moving something to the end of env is an option, then moving it to
MicroBlazeCPU (not CPUState!) would seem better, especially when it's
not supposed to be reset. Going from CPUMBState *env to MicroBlazeCPU
*cpu is a cheap offset operation, whereas CPUState *cs involves a QOM cast.
Regards,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB
21284 (AG Nürnberg)
Re: [Qemu-devel] [RFC PATCH v4 07/28] COLO: Add a new RunState RUN_STATE_COLO
* zhanghailiang (zhang.zhanghaili...@huawei.com) wrote:
> Guest will enter this state when paused to save/restore VM state
> under colo checkpoint.
>
> Signed-off-by: zhanghailiang
> Signed-off-by: Gonglei
> Signed-off-by: Lai Jiangshan
Reviewed-by: Dr. David Alan Gilbert
(Do suspend and watchdog-with-pause work with colo - they sound odd
combinations).
> ---
> qapi-schema.json | 5 -
> vl.c | 8
> 2 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 172aae3..43a964b 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -148,12 +148,15 @@
> # @watchdog: the watchdog action is configured to pause and has been
> triggered
> #
> # @guest-panicked: guest has been panicked as a result of guest OS panic
> +#
> +# @colo: guest is paused to save/restore VM state under colo checkpoint
> (since
> +# 2.4)
> ##
> { 'enum': 'RunState',
>'data': [ 'debug', 'inmigrate', 'internal-error', 'io-error', 'paused',
> 'postmigrate', 'prelaunch', 'finish-migrate', 'restore-vm',
> 'running', 'save-vm', 'shutdown', 'suspended', 'watchdog',
> -'guest-panicked' ] }
> +'guest-panicked', 'colo' ] }
>
> ##
> # @StatusInfo:
> diff --git a/vl.c b/vl.c
> index 9724992..8c07244 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -550,6 +550,7 @@ static const RunStateTransition
> runstate_transitions_def[] = {
>
> { RUN_STATE_INMIGRATE, RUN_STATE_RUNNING },
> { RUN_STATE_INMIGRATE, RUN_STATE_PAUSED },
> +{ RUN_STATE_INMIGRATE, RUN_STATE_COLO },
>
> { RUN_STATE_INTERNAL_ERROR, RUN_STATE_PAUSED },
> { RUN_STATE_INTERNAL_ERROR, RUN_STATE_FINISH_MIGRATE },
> @@ -559,6 +560,7 @@ static const RunStateTransition
> runstate_transitions_def[] = {
>
> { RUN_STATE_PAUSED, RUN_STATE_RUNNING },
> { RUN_STATE_PAUSED, RUN_STATE_FINISH_MIGRATE },
> +{ RUN_STATE_PAUSED, RUN_STATE_COLO},
>
> { RUN_STATE_POSTMIGRATE, RUN_STATE_RUNNING },
> { RUN_STATE_POSTMIGRATE, RUN_STATE_FINISH_MIGRATE },
> @@ -569,9 +571,12 @@ static const RunStateTransition
> runstate_transitions_def[] = {
>
> { RUN_STATE_FINISH_MIGRATE, RUN_STATE_RUNNING },
> { RUN_STATE_FINISH_MIGRATE, RUN_STATE_POSTMIGRATE },
> +{ RUN_STATE_FINISH_MIGRATE, RUN_STATE_COLO},
>
> { RUN_STATE_RESTORE_VM, RUN_STATE_RUNNING },
>
> +{ RUN_STATE_COLO, RUN_STATE_RUNNING },
> +
> { RUN_STATE_RUNNING, RUN_STATE_DEBUG },
> { RUN_STATE_RUNNING, RUN_STATE_INTERNAL_ERROR },
> { RUN_STATE_RUNNING, RUN_STATE_IO_ERROR },
> @@ -582,6 +587,7 @@ static const RunStateTransition
> runstate_transitions_def[] = {
> { RUN_STATE_RUNNING, RUN_STATE_SHUTDOWN },
> { RUN_STATE_RUNNING, RUN_STATE_WATCHDOG },
> { RUN_STATE_RUNNING, RUN_STATE_GUEST_PANICKED },
> +{ RUN_STATE_RUNNING, RUN_STATE_COLO},
>
> { RUN_STATE_SAVE_VM, RUN_STATE_RUNNING },
>
> @@ -592,9 +598,11 @@ static const RunStateTransition
> runstate_transitions_def[] = {
> { RUN_STATE_RUNNING, RUN_STATE_SUSPENDED },
> { RUN_STATE_SUSPENDED, RUN_STATE_RUNNING },
> { RUN_STATE_SUSPENDED, RUN_STATE_FINISH_MIGRATE },
> +{ RUN_STATE_SUSPENDED, RUN_STATE_COLO},
>
> { RUN_STATE_WATCHDOG, RUN_STATE_RUNNING },
> { RUN_STATE_WATCHDOG, RUN_STATE_FINISH_MIGRATE },
> +{ RUN_STATE_WATCHDOG, RUN_STATE_COLO},
>
> { RUN_STATE_GUEST_PANICKED, RUN_STATE_RUNNING },
> { RUN_STATE_GUEST_PANICKED, RUN_STATE_FINISH_MIGRATE },
> --
> 1.7.12.4
>
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH] monitor: suggest running "help" for command errors
Bandan Das writes:
> Markus Armbruster writes:
>
>> Bandan Das writes:
>>
> ...
>>> -static void user_async_cmd_handler(Monitor *mon, const mon_cmd_t *cmd,
>>> +static int user_async_cmd_handler(Monitor *mon, const mon_cmd_t *cmd,
>>> const QDict *params)
>>> {
>>> int ret;
>>> @@ -954,6 +954,8 @@ static void user_async_cmd_handler(Monitor *mon, const
>>> mon_cmd_t *cmd,
>>> monitor_resume(mon);
>>> g_free(cb_data);
>>> }
>>> +
>>> +return ret;
>>> }
>>>
>>
>> user_async_cmd_handler() is unreachable since commit 3b5704b. I got
>> code cleaning that up in my tree. Don't worry about it.
>
> Ok or if you prefer, I can skip this part and other similar cases below.
> ...
I think limiting the additional help to argument syntax errors for now
will be easiest.
>>>
>>> fail:
>>> +*failed = 1;
>>> g_free(key);
>>> -return NULL;
>>> +return cmd;
>>> }
>>>
>>
>> From the function's contract:
>>
>> * If @cmdline is blank, return NULL.
>> * If it can't be parsed, report to @mon, and return NULL.
>> * Else, insert command arguments into @qdict, and return the command.
>>
>> Your patch splits the "it can't be parsed" case into "command not found"
>> and "arguments can't be parsed", but neglects to update the contract.
>> Needs fixing. Perhaps like this:
>>
>> * If @cmdline is blank, return NULL.
>> * If @cmdline doesn't start with a valid command name, report to @mon,
>> * and return NULL.
>> * Else, if the command's arguments can't be parsed, report to @mon,
>> * store 1 through @failed, and return the command.
>> * Else, insert command arguments into @qdict, and return the command.
>
>
>> The contract wasn't exactly pretty before, and I'm afraid it's
>> positively ugly now.
>>
>> The common cause for such ugliness is doing too much in one function.
>> I'd try splitting into a command part and an argument part, so that
>>
>> cmd = monitor_parse_command(mon, cmdline, start, table, qdict);
>> if (!cmd) {
>> // bail out
>> }
>>
>> becomes something like
>>
>> cmd = monitor_parse_command(mon, cmdline, start, table, &args);
>> if (!cmd) {
>> // bail out
>> }
>> qdict = monitor_parse_arguments(mon, cmd, args)
>> if (!qdict) {
>> // bail out
>> }
>>
>> While I encourage you to do this, I won't reject your patch just because
>> you don't.
>
> Ok understood, makes sense. Will fix this in next version.
Looking forward to it :)
>>> void monitor_set_error(Monitor *mon, QError *qerror)
>>> @@ -4114,20 +4118,22 @@ static void handle_user_command(Monitor *mon, const
>>> char *cmdline)
>>> {
>>> QDict *qdict;
>>> const mon_cmd_t *cmd;
>>> +int failed = 0;
>>>
>>> qdict = qdict_new();
>>>
>>> -cmd = monitor_parse_command(mon, cmdline, 0, mon->cmd_table, qdict);
>>> -if (!cmd)
>>> +cmd = monitor_parse_command(mon, cmdline, 0, mon->cmd_table,
>>> +qdict, &failed);
>>> +if (!cmd || failed) {
>>> goto out;
>>> +}
>>
>> cmd implies !failed, therefore !cmd || failed == !cmd here. Why not
>> simply stick to if (!cmd)?
>
> Note that I changed the old behavior so that now monitor_parse_command()
> returns cmd when failed is set. This is so that we have cmd->name. "if (!cmd)"
> will be false in that case.
You're right. I got confused, figured out what's up, updated my review,
but missed this paragraph.
[...]
Re: [Qemu-devel] [RFC PATCH 00/34] Multi Architecture System Emulation
Am 15.05.2015 um 08:59 schrieb Peter Crosthwaite: > On Sun, May 10, 2015 at 11:29 PM, Peter Crosthwaite > wrote: >> The helper function namespace is going to be tricky. I haven't tackled the >> problem just yet, but looking for ideas on how we can avoid prefacing all >> helpers with arch prefixes to avoid link-time collisions because multiple >> arches use the same helper names. > > Ok so have half a plan here now for the helper link-time namespace > problem. The top level HELPER() macro can be patched to glue() in an > architecture name set by cpu.h. The every use of HELPER(foo) will be > namespace collision free. The catch? Some arches mass-use literal > helper_foo for defs and calls. I've always disliked that inconcistency, so +1 for forcing the use of HELPER(). Will also make it easier to switch the CPU argument some day. Cheers, Andreas > Fortunately ARM is well behaved and > only has about 4 usages so thats an easy convert. Microblaze has more. > It will be a reasonably simple (but tedious) scripted changed to > convert uses and definitions of helper_foo to HELPER(foo) but it saves > on the verbosity of having to go in and preface everything (all of > callsites, definitions and helper.h defs themselves). > > Regards, > Peter -- SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB 21284 (AG Nürnberg)
Re: [Qemu-devel] [RFC PATCH v4 03/28] COLO: migrate colo related info to slave
* zhanghailiang (zhang.zhanghaili...@huawei.com) wrote:
> We can know if VM in destination should go into COLO mode by refer to
> the info that has been migrated from PVM.
>
> Signed-off-by: zhanghailiang
> Signed-off-by: Yang Hongyang
> Signed-off-by: Lai Jiangshan
> Signed-off-by: Gonglei
> ---
> include/migration/migration-colo.h | 2 ++
> migration/Makefile.objs| 1 +
> migration/colo-comm.c | 55
> ++
> vl.c | 5 +++-
> 4 files changed, 62 insertions(+), 1 deletion(-)
> create mode 100644 migration/colo-comm.c
>
> diff --git a/include/migration/migration-colo.h
> b/include/migration/migration-colo.h
> index 6fdbb94..de68c72 100644
> --- a/include/migration/migration-colo.h
> +++ b/include/migration/migration-colo.h
> @@ -14,7 +14,9 @@
> #define QEMU_MIGRATION_COLO_H
>
> #include "qemu-common.h"
> +#include "migration/migration.h"
>
> bool colo_supported(void);
> +void colo_info_mig_init(void);
>
> #endif
> diff --git a/migration/Makefile.objs b/migration/Makefile.objs
> index 5a25d39..cb7bd30 100644
> --- a/migration/Makefile.objs
> +++ b/migration/Makefile.objs
> @@ -1,5 +1,6 @@
> common-obj-y += migration.o tcp.o
> common-obj-$(CONFIG_COLO) += colo.o
> +common-obj-y += colo-comm.o
> common-obj-y += vmstate.o
> common-obj-y += qemu-file.o qemu-file-buf.o qemu-file-unix.o
> qemu-file-stdio.o
> common-obj-y += xbzrle.o
> diff --git a/migration/colo-comm.c b/migration/colo-comm.c
> new file mode 100644
> index 000..cab97e9
> --- /dev/null
> +++ b/migration/colo-comm.c
> @@ -0,0 +1,55 @@
> +/*
> + * COarse-grain LOck-stepping Virtual Machines for Non-stop Service (COLO)
> + * (a.k.a. Fault Tolerance or Continuous Replication)
> + *
> + * Copyright (c) 2015 HUAWEI TECHNOLOGIES CO.,LTD.
> + * Copyright (c) 2015 FUJITSU LIMITED
> + * Copyright (c) 2015 Intel Corporation
> + *
> + * 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
> +
> +#define DEBUG_COLO_COMMON 0
> +
> +#define DPRINTF(fmt, ...) \
> +do { \
> +if (DEBUG_COLO_COMMON) { \
> +fprintf(stderr, "COLO: " fmt, ## __VA_ARGS__); \
> +} \
> +} while (0)
I'm trying to get rid of all the DPRINTF's in migration - I've turned
most of the existing DPRINTF into trace_ calls already;
using the stderr trace backend it's very easy (--enable-trace-backends=stderr)
and then you can switch them on individually, it also prints times with
each message that's useful for performance tuning.
Dave
> +static bool colo_requested;
> +
> +/* save */
> +static void colo_info_save(QEMUFile *f, void *opaque)
> +{
> +qemu_put_byte(f, migrate_enable_colo());
> +}
> +
> +/* restore */
> +static int colo_info_load(QEMUFile *f, void *opaque, int version_id)
> +{
> +int value = qemu_get_byte(f);
> +
> +if (value && !colo_requested) {
> +DPRINTF("COLO requested!\n");
> +}
> +colo_requested = value;
> +
> +return 0;
> +}
> +
> +static SaveVMHandlers savevm_colo_info_handlers = {
> +.save_state = colo_info_save,
> +.load_state = colo_info_load,
> +};
> +
> +void colo_info_mig_init(void)
> +{
> +register_savevm_live(NULL, "colo", -1, 1,
> + &savevm_colo_info_handlers, NULL);
> +}
> diff --git a/vl.c b/vl.c
> index 75ec292..9724992 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -90,6 +90,7 @@ int main(int argc, char **argv)
> #include "sysemu/dma.h"
> #include "audio/audio.h"
> #include "migration/migration.h"
> +#include "migration/migration-colo.h"
> #include "sysemu/kvm.h"
> #include "qapi/qmp/qjson.h"
> #include "qemu/option.h"
> @@ -4149,7 +4150,9 @@ int main(int argc, char **argv, char **envp)
>
> blk_mig_init();
> ram_mig_init();
> -
> +#ifdef CONFIG_COLO
> +colo_info_mig_init();
> +#endif
> /* If the currently selected machine wishes to override the units-per-bus
> * property of its default HBA interface type, do so now. */
> if (machine_class->units_per_default_bus) {
> --
> 1.7.12.4
>
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [RFC] ARM/ARM64: KVM: Implement KVM_FLUSH_DCACHE_GPA ioctl
On 05/07/15 19:01, Paolo Bonzini wrote: > > > On 07/05/2015 18:56, Jérémy Fanguède wrote: >> USB devices fail with a timeout error, as if the communication between >> the kernel and the devices fail at a certain point: >> usb 1-1: device not accepting address 5, error -110 >> usb usb1-port1: unable to enumerate USB device This is consistent with what I saw in my earlier testing. >> e1000 fails when the userspace tries to use it, with these type of >> kernel messages: >> e1000 :00:02.0 eth0: Detected Tx Unit Hang >> Tx Queue <0> >> TDH >> TDT >> next_to_use >> next_to_clean<9> >> buffer_info[next_to_clean] >> time_stamp >> next_to_watch >> jiffies >> next_to_watch.status <0> > > Can you find out what memory attributes the guest is using for the > memory---and if it's uncached, why? For USB, see "drivers/usb/core/hcd-pci.c", function usb_hcd_pci_probe(): it uses ioremap_nocache(). On the "why", that ioremap_nocache() call can be tracked to http://git.kernel.org/cgit/linux/kernel/git/tglx/history.git/commit/?id=a914dd8b (Feb 2002), which predates the kernel's move to git. I guess ioremap_nocache() is used simply because USB host controllers are supposed to programmed like that. And, from "arch/arm64/include/asm/io.h": #define ioremap_nocache(addr, size) __ioremap((addr), (size), __pgprot(PROT_DEVICE_nGnRE)) Thanks Laszlo
[Qemu-devel] [PATCH][RFC] libxl: use new qemu parameters for emulated qemuu disks
NOTES:
This patch is a only a fast draft for testing.
Some tests result:
At xl create cdrom empty or not are both working, xl cd-insert is
working, xl cd-eject seems working but on xl command in linux hvm domU
return qmp error of "Device 'ide-N' is locked", in windows 7 instead
don't show the errror.
xl block-attach seems working correctly and xl block-detach works
correctly with linux hvm but not with windows 7 (seems block the disk
remove, I don't know if do the same without this patch)
Scsi disk case not tested for now.
Any comment is appreciated.
Signed-off-by: Fabio Fantoni
---
tools/libxl/libxl_dm.c | 35 ++-
1 file changed, 18 insertions(+), 17 deletions(-)
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 4bec5ba..6d00e38 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -811,7 +811,6 @@ static char ** libxl__build_device_model_args_new(libxl__gc
*gc,
int dev_number =
libxl__device_disk_dev_number(disks[i].vdev, &disk, &part);
const char *format = qemu_disk_format_string(disks[i].format);
-char *drive;
const char *pdev_path;
if (dev_number == -1) {
@@ -822,13 +821,14 @@ static char **
libxl__build_device_model_args_new(libxl__gc *gc,
if (disks[i].is_cdrom) {
if (disks[i].format == LIBXL_DISK_FORMAT_EMPTY)
-drive = libxl__sprintf
-(gc,
"if=ide,index=%d,media=cdrom,cache=writeback,id=ide-%i",
- disk, dev_number);
+flexarray_vappend(dm_args, "-drive",
+GCSPRINTF("if=none,id=ide-%i,cache=writeback",
dev_number), "-device",
+GCSPRINTF("ide-cd,drive=ide-%i", dev_number), NULL);
else
-drive = libxl__sprintf
-(gc,
"file=%s,if=ide,index=%d,media=cdrom,format=%s,cache=writeback,id=ide-%i",
- disks[i].pdev_path, disk, format, dev_number);
+flexarray_vappend(dm_args, "-drive",
+
GCSPRINTF("file=%s,if=none,id=ide-%i,format=%s,cache=writeback",
+disks[i].pdev_path, dev_number, format), "-device",
+GCSPRINTF("ide-cd,drive=ide-%i", dev_number), NULL);
} else {
if (disks[i].format == LIBXL_DISK_FORMAT_EMPTY) {
LIBXL__LOG(ctx, LIBXL__LOG_WARNING, "cannot support"
@@ -857,25 +857,26 @@ static char **
libxl__build_device_model_args_new(libxl__gc *gc,
* hd[a-d] and ignore the rest.
*/
if (strncmp(disks[i].vdev, "sd", 2) == 0)
-drive = libxl__sprintf
-(gc,
"file=%s,if=scsi,bus=0,unit=%d,format=%s,cache=writeback",
- pdev_path, disk, format);
+flexarray_vappend(dm_args, "-drive",
+
GCSPRINTF("file=%s,if=none,id=scsidisk-%d,format=%s,cache=writeback",
+pdev_path, disk, format), "-device",
+GCSPRINTF("scsi-hd,drive=scsidisk-%d,scsi-id=%d",
+disk, disk), NULL);
else if (disk < 6 && libxl_defbool_val(b_info->u.hvm.ahci)){
flexarray_vappend(dm_args, "-drive",
GCSPRINTF("file=%s,if=none,id=ahcidisk-%d,format=%s,cache=writeback",
-pdev_path, disk, format), "-device",
GCSPRINTF("ide-hd,bus=ahci0.%d,unit=0,drive=ahcidisk-%d",
+pdev_path, disk, format), "-device",
+
GCSPRINTF("ide-hd,bus=ahci0.%d,unit=0,drive=ahcidisk-%d",
disk, disk), NULL);
-continue;
}else if (disk < 4)
-drive = libxl__sprintf
-(gc,
"file=%s,if=ide,index=%d,media=disk,format=%s,cache=writeback",
- pdev_path, disk, format);
+flexarray_vappend(dm_args, "-drive",
+
GCSPRINTF("file=%s,if=none,id=idedisk-%d,format=%s,cache=writeback",
+pdev_path, disk, format), "-device",
GCSPRINTF("ide-hd,drive=idedisk-%d",
+disk), NULL);
else
continue; /* Do not emulate this disk */
}
-flexarray_append(dm_args, "-drive");
-flexarray_append(dm_args, drive);
}
switch (b_info->u.hvm.vendor_device) {
--
1.9.1
Re: [Qemu-devel] [RFC PATCH v4 08/28] QEMUSizedBuffer: Introduce two help functions for qsb
* zhanghailiang (zhang.zhanghaili...@huawei.com) wrote:
> Introduce two new QEMUSizedBuffer APIs which will be used by COLO to buffer
> VM state:
> One is qsb_put_buffer(), which put the content of a given QEMUSizedBuffer
> into QEMUFile, this is used to send buffered VM state to secondary.
> Another is qsb_fill_buffer(), read 'size' bytes of data from the file into
> qsb, this is used to get VM state from socket into a buffer.
>
> Signed-off-by: Yang Hongyang
> Signed-off-by: zhanghailiang
Reviewed-by: Dr. David Alan Gilbert
(I could use these in my postcopy world)
Dave
> ---
> include/migration/qemu-file.h | 3 ++-
> migration/qemu-file-buf.c | 58
> +++
> 2 files changed, 60 insertions(+), 1 deletion(-)
>
> diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
> index 745a850..09a0e2a 100644
> --- a/include/migration/qemu-file.h
> +++ b/include/migration/qemu-file.h
> @@ -140,7 +140,8 @@ ssize_t qsb_get_buffer(const QEMUSizedBuffer *, off_t
> start, size_t count,
> uint8_t *buf);
> ssize_t qsb_write_at(QEMUSizedBuffer *qsb, const uint8_t *buf,
> off_t pos, size_t count);
> -
> +void qsb_put_buffer(QEMUFile *f, QEMUSizedBuffer *qsb, int size);
> +int qsb_fill_buffer(QEMUSizedBuffer *qsb, QEMUFile *f, int size);
>
> /*
> * For use on files opened with qemu_bufopen
> diff --git a/migration/qemu-file-buf.c b/migration/qemu-file-buf.c
> index 16a51a1..686f417 100644
> --- a/migration/qemu-file-buf.c
> +++ b/migration/qemu-file-buf.c
> @@ -365,6 +365,64 @@ ssize_t qsb_write_at(QEMUSizedBuffer *qsb, const uint8_t
> *source,
> return count;
> }
>
> +
> +/**
> + * Put the content of a given QEMUSizedBuffer into QEMUFile.
> + *
> + * @f: A QEMUFile
> + * @qsb: A QEMUSizedBuffer
> + * @size: size of content to write
> + */
> +void qsb_put_buffer(QEMUFile *f, QEMUSizedBuffer *qsb, int size)
> +{
> +int i, l;
> +
> +for (i = 0; i < qsb->n_iov && size > 0; i++) {
> +l = MIN(qsb->iov[i].iov_len, size);
> +qemu_put_buffer(f, qsb->iov[i].iov_base, l);
> +size -= l;
> +}
> +}
> +
> +/*
> + * Read 'size' bytes of data from the file into qsb.
> + * always fill from pos 0 and used after qsb_create().
> + *
> + * It will return size bytes unless there was an error, in which case it will
> + * return as many as it managed to read (assuming blocking fd's which
> + * all current QEMUFile are)
> + */
> +int qsb_fill_buffer(QEMUSizedBuffer *qsb, QEMUFile *f, int size)
> +{
> +ssize_t rc = qsb_grow(qsb, size);
> +int pending = size, i;
> +qsb->used = 0;
> +uint8_t *buf = NULL;
> +
> +if (rc < 0) {
> +return rc;
> +}
> +
> +for (i = 0; i < qsb->n_iov && pending > 0; i++) {
> +int doneone = 0;
> +/* read until iov full */
> +while (doneone < qsb->iov[i].iov_len && pending > 0) {
> +int readone = 0;
> +buf = qsb->iov[i].iov_base;
> +readone = qemu_get_buffer(f, buf,
> +MIN(qsb->iov[i].iov_len - doneone, pending));
> +if (readone == 0) {
> +return qsb->used;
> +}
> +buf += readone;
> +doneone += readone;
> +pending -= readone;
> +qsb->used += readone;
> +}
> +}
> +return qsb->used;
> +}
> +
> typedef struct QEMUBuffer {
> QEMUSizedBuffer *qsb;
> QEMUFile *file;
> --
> 1.7.12.4
>
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
[Qemu-devel] [PATCH v2 14/15] tap: Finish conversion of tap_open() to Error
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
---
net/tap.c | 4
1 file changed, 4 deletions(-)
diff --git a/net/tap.c b/net/tap.c
index 348b786..f7db9dc 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -597,10 +597,6 @@ static int net_tap_init(const NetdevTapOptions *tap, int
*vnet_hdr,
TFR(fd = tap_open(ifname, ifname_sz, vnet_hdr, vnet_hdr_required,
mq_required, errp));
if (fd < 0) {
-/* FIXME drop when all tap_open() store an Error */
-if (errp && !*errp) {
-error_setg(errp, "can't open tap device");
-}
return -1;
}
--
1.9.3
[Qemu-devel] [PATCH v2 01/15] net: Improve error message for -net hubport a bit
Type "hubport" is valid only with -netdev. Unfortunately, that's
detected late and the error message doesn't explain why:
$ qemu-system-i386 -net hubport,id=foo,hubid=0
qemu-system-i386: -net hubport,id=foo,hubid=0: Device 'hubport' could not
be initialized
Improve the error message to "Parameter 'type' expects a net type".
Not fixed: -net hubport without the parameters required by -netdev
hubport still asks for those parameters:
$ qemu-system-i386 -net hubport
qemu-system-i386: -net hubport: Parameter 'hubid' is missing
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
---
net/hub.c | 5 +
net/net.c | 5 +
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/net/hub.c b/net/hub.c
index 2b60ab9..261f8cc 100644
--- a/net/hub.c
+++ b/net/hub.c
@@ -286,12 +286,9 @@ int net_init_hubport(const NetClientOptions *opts, const
char *name,
const NetdevHubPortOptions *hubport;
assert(opts->kind == NET_CLIENT_OPTIONS_KIND_HUBPORT);
+assert(!peer);
hubport = opts->hubport;
-if (peer) {
-return -EINVAL;
-}
-
net_hub_add_port(hubport->hubid, name);
return 0;
}
diff --git a/net/net.c b/net/net.c
index 7427f6a..d9aaeb5 100644
--- a/net/net.c
+++ b/net/net.c
@@ -882,6 +882,11 @@ static int net_client_init1(const void *object, int
is_netdev, Error **errp)
} else {
u.net = object;
opts = u.net->opts;
+if (opts->kind == NET_CLIENT_OPTIONS_KIND_HUBPORT) {
+error_set(errp, QERR_INVALID_PARAMETER_VALUE, "type",
+ "a net type");
+return -1;
+}
/* missing optional values have been initialized to "all bits zero" */
name = u.net->has_id ? u.net->id : u.net->name;
}
--
1.9.3
[Qemu-devel] [PATCH v2 06/15] tap: Improve -netdev/netdev_add/-net/... bridge error reporting
When -netdev bridge fails, it first reports a specific error, then a
generic one, like this:
$ qemu-system-x86_64 -netdev bridge,id=foo
failed to launch bridge helper
qemu-system-x86_64: -netdev bridge,id=foo: Device 'bridge' could not be
initialized
The first message goes to stderr. Wrong for HMP, because errors need
to go to the monitor there.
The second message goes to stderr for -netdev, to the monitor for HMP
netdev_add, and becomes the error reply for QMP netdev_add.
Convert net_bridge_run_helper() to Error, and propagate its errors
through net_init_bridge(). This ensures the error gets reported where
the user is, and suppresses the unwanted second message.
While there, improve the error messages a bit.
The above example becomes:
$ qemu-system-x86_64 -netdev bridge,id=foo
qemu-system-x86_64: -netdev bridge,id=foo: bridge helper failed
net_init_tap() also uses net_bridge_run_helper(). Propagate its
errors there as well. Improves reporting these errors with -netdev
tap & friends.
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
---
net/tap.c | 30 +++---
1 file changed, 19 insertions(+), 11 deletions(-)
diff --git a/net/tap.c b/net/tap.c
index adb1022..23c81fa 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -437,7 +437,8 @@ static int recv_fd(int c)
return len;
}
-static int net_bridge_run_helper(const char *helper, const char *bridge)
+static int net_bridge_run_helper(const char *helper, const char *bridge,
+ Error **errp)
{
sigset_t oldmask, mask;
int pid, status;
@@ -450,11 +451,16 @@ static int net_bridge_run_helper(const char *helper,
const char *bridge)
sigprocmask(SIG_BLOCK, &mask, &oldmask);
if (socketpair(PF_UNIX, SOCK_STREAM, 0, sv) == -1) {
+error_setg_errno(errp, errno, "socketpair() failed");
return -1;
}
/* try to launch bridge helper */
pid = fork();
+if (pid < 0) {
+error_setg_errno(errp, errno, "Can't fork bridge helper");
+return -1;
+}
if (pid == 0) {
int open_max = sysconf(_SC_OPEN_MAX), i;
char fd_buf[6+10];
@@ -502,14 +508,16 @@ static int net_bridge_run_helper(const char *helper,
const char *bridge)
}
_exit(1);
-} else if (pid > 0) {
+} else {
int fd;
+int saved_errno;
close(sv[1]);
do {
fd = recv_fd(sv[0]);
} while (fd == -1 && errno == EINTR);
+saved_errno = errno;
close(sv[0]);
@@ -518,22 +526,21 @@ static int net_bridge_run_helper(const char *helper,
const char *bridge)
}
sigprocmask(SIG_SETMASK, &oldmask, NULL);
if (fd < 0) {
-fprintf(stderr, "failed to recv file descriptor\n");
+error_setg_errno(errp, saved_errno,
+ "failed to recv file descriptor");
return -1;
}
-
-if (WIFEXITED(status) && WEXITSTATUS(status) == 0) {
-return fd;
+if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) {
+error_setg(errp, "bridge helper failed");
+return -1;
}
+return fd;
}
-fprintf(stderr, "failed to launch bridge helper\n");
-return -1;
}
int net_init_bridge(const NetClientOptions *opts, const char *name,
NetClientState *peer, Error **errp)
{
-/* FIXME error_setg(errp, ...) on failure */
const NetdevBridgeOptions *bridge;
const char *helper, *br;
TAPState *s;
@@ -545,7 +552,7 @@ int net_init_bridge(const NetClientOptions *opts, const
char *name,
helper = bridge->has_helper ? bridge->helper : DEFAULT_BRIDGE_HELPER;
br = bridge->has_br ? bridge->br : DEFAULT_BRIDGE_INTERFACE;
-fd = net_bridge_run_helper(helper, br);
+fd = net_bridge_run_helper(helper, br, errp);
if (fd == -1) {
return -1;
}
@@ -792,7 +799,8 @@ int net_init_tap(const NetClientOptions *opts, const char
*name,
return -1;
}
-fd = net_bridge_run_helper(tap->helper, DEFAULT_BRIDGE_INTERFACE);
+fd = net_bridge_run_helper(tap->helper, DEFAULT_BRIDGE_INTERFACE,
+ errp);
if (fd == -1) {
return -1;
}
--
1.9.3
[Qemu-devel] [PATCH v2 00/15] net: Improve error reporting
v2: - Touch up a few commit messages [Eric] - Touch up a an error message in PATCH 03+15 [Eric] - Don't report possibly bogus errno in PATCH 12 [Eric] Markus Armbruster (15): net: Improve error message for -net hubport a bit net: Permit incremental conversion of init functions to Error net: Improve -net nic error reporting net/dump: Improve -net/host_net_add dump error reporting tap: net_tap_fd_init() can't fail, drop dead error handling tap: Improve -netdev/netdev_add/-net/... bridge error reporting tap: Convert tap_set_sndbuf() to Error tap: Convert net_init_tap_one() to Error tap: Convert launch_script() to Error tap: Permit incremental conversion of tap_open() to Error tap-linux: Convert tap_open() to Error tap-bsd: Convert tap_open() to Error tap-solaris: Convert tap_open() to Error tap: Finish conversion of tap_open() to Error tap: Improve -netdev/netdev_add/-net/... tap error reporting net/clients.h | 20 +++--- net/dump.c| 13 ++-- net/hub.c | 7 +- net/l2tpv3.c | 5 +- net/net.c | 30 ++--- net/netmap.c | 3 +- net/slirp.c | 3 +- net/socket.c | 3 +- net/tap-aix.c | 7 +- net/tap-bsd.c | 38 +-- net/tap-haiku.c | 7 +- net/tap-linux.c | 24 +++ net/tap-solaris.c | 63 - net/tap-win32.c | 3 +- net/tap.c | 198 ++ net/tap_int.h | 4 +- net/vde.c | 3 +- net/vhost-user.c | 3 +- 18 files changed, 231 insertions(+), 203 deletions(-) -- 1.9.3
[Qemu-devel] [PATCH v2 07/15] tap: Convert tap_set_sndbuf() to Error
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
---
net/tap-aix.c | 3 +--
net/tap-bsd.c | 3 +--
net/tap-haiku.c | 3 +--
net/tap-linux.c | 6 ++
net/tap-solaris.c | 3 +--
net/tap.c | 4 +++-
net/tap_int.h | 2 +-
7 files changed, 10 insertions(+), 14 deletions(-)
diff --git a/net/tap-aix.c b/net/tap-aix.c
index 804d164..0a3d461 100644
--- a/net/tap-aix.c
+++ b/net/tap-aix.c
@@ -32,9 +32,8 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
return -1;
}
-int tap_set_sndbuf(int fd, const NetdevTapOptions *tap)
+void tap_set_sndbuf(int fd, const NetdevTapOptions *tap, Error **errp)
{
-return 0;
}
int tap_probe_vnet_hdr(int fd)
diff --git a/net/tap-bsd.c b/net/tap-bsd.c
index bf91bd0..53cdd9f 100644
--- a/net/tap-bsd.c
+++ b/net/tap-bsd.c
@@ -176,9 +176,8 @@ error:
}
#endif /* __FreeBSD__ */
-int tap_set_sndbuf(int fd, const NetdevTapOptions *tap)
+void tap_set_sndbuf(int fd, const NetdevTapOptions *tap, Error **errp)
{
-return 0;
}
int tap_probe_vnet_hdr(int fd)
diff --git a/net/tap-haiku.c b/net/tap-haiku.c
index e5ce436..0905b28 100644
--- a/net/tap-haiku.c
+++ b/net/tap-haiku.c
@@ -32,9 +32,8 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
return -1;
}
-int tap_set_sndbuf(int fd, const NetdevTapOptions *tap)
+void tap_set_sndbuf(int fd, const NetdevTapOptions *tap, Error **errp)
{
-return 0;
}
int tap_probe_vnet_hdr(int fd)
diff --git a/net/tap-linux.c b/net/tap-linux.c
index 812bf2d..6fa2744 100644
--- a/net/tap-linux.c
+++ b/net/tap-linux.c
@@ -126,7 +126,7 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
*/
#define TAP_DEFAULT_SNDBUF 0
-int tap_set_sndbuf(int fd, const NetdevTapOptions *tap)
+void tap_set_sndbuf(int fd, const NetdevTapOptions *tap, Error **errp)
{
int sndbuf;
@@ -139,10 +139,8 @@ int tap_set_sndbuf(int fd, const NetdevTapOptions *tap)
}
if (ioctl(fd, TUNSETSNDBUF, &sndbuf) == -1 && tap->has_sndbuf) {
-error_report("TUNSETSNDBUF ioctl failed: %s", strerror(errno));
-return -1;
+error_setg_errno(errp, errno, "TUNSETSNDBUF ioctl failed");
}
-return 0;
}
int tap_probe_vnet_hdr(int fd)
diff --git a/net/tap-solaris.c b/net/tap-solaris.c
index 9c7278f..7839323 100644
--- a/net/tap-solaris.c
+++ b/net/tap-solaris.c
@@ -198,9 +198,8 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
return fd;
}
-int tap_set_sndbuf(int fd, const NetdevTapOptions *tap)
+void tap_set_sndbuf(int fd, const NetdevTapOptions *tap, Error **errp)
{
-return 0;
}
int tap_probe_vnet_hdr(int fd)
diff --git a/net/tap.c b/net/tap.c
index 23c81fa..d54222d 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -610,7 +610,9 @@ static int net_init_tap_one(const NetdevTapOptions *tap,
NetClientState *peer,
TAPState *s = net_tap_fd_init(peer, model, name, fd, vnet_hdr);
int vhostfd;
-if (tap_set_sndbuf(s->fd, tap) < 0) {
+tap_set_sndbuf(s->fd, tap, &err);
+if (err) {
+error_report_err(err);
return -1;
}
diff --git a/net/tap_int.h b/net/tap_int.h
index 79afdf2..6df271f 100644
--- a/net/tap_int.h
+++ b/net/tap_int.h
@@ -34,7 +34,7 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
ssize_t tap_read_packet(int tapfd, uint8_t *buf, int maxlen);
-int tap_set_sndbuf(int fd, const NetdevTapOptions *tap);
+void tap_set_sndbuf(int fd, const NetdevTapOptions *tap, Error **errp);
int tap_probe_vnet_hdr(int fd);
int tap_probe_vnet_hdr_len(int fd, int len);
int tap_probe_has_ufo(int fd);
--
1.9.3
[Qemu-devel] [PATCH v2 09/15] tap: Convert launch_script() to Error
Fixes inappropriate use of stderr in monitor command handler.
While there, improve the messages some.
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
---
net/tap.c | 40
1 file changed, 28 insertions(+), 12 deletions(-)
diff --git a/net/tap.c b/net/tap.c
index d1f5644..08bfc51 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -59,7 +59,8 @@ typedef struct TAPState {
unsigned host_vnet_hdr_len;
} TAPState;
-static int launch_script(const char *setup_script, const char *ifname, int fd);
+static void launch_script(const char *setup_script, const char *ifname,
+ int fd, Error **errp);
static int tap_can_send(void *opaque);
static void tap_send(void *opaque);
@@ -288,6 +289,7 @@ static void tap_set_offload(NetClientState *nc, int csum,
int tso4,
static void tap_cleanup(NetClientState *nc)
{
TAPState *s = DO_UPCAST(TAPState, nc, nc);
+Error **err = NULL;
if (s->vhost_net) {
vhost_net_cleanup(s->vhost_net);
@@ -296,8 +298,12 @@ static void tap_cleanup(NetClientState *nc)
qemu_purge_queued_packets(nc);
-if (s->down_script[0])
-launch_script(s->down_script, s->down_script_arg, s->fd);
+if (s->down_script[0]) {
+launch_script(s->down_script, s->down_script_arg, s->fd, &err);
+if (err) {
+error_report_err(err);
+}
+}
tap_read_poll(s, false);
tap_write_poll(s, false);
@@ -368,7 +374,8 @@ static TAPState *net_tap_fd_init(NetClientState *peer,
return s;
}
-static int launch_script(const char *setup_script, const char *ifname, int fd)
+static void launch_script(const char *setup_script, const char *ifname,
+ int fd, Error **errp)
{
int pid, status;
char *args[3];
@@ -376,6 +383,11 @@ static int launch_script(const char *setup_script, const
char *ifname, int fd)
/* try to launch network script */
pid = fork();
+if (pid < 0) {
+error_setg_errno(errp, errno, "could not launch network script %s",
+ setup_script);
+return;
+}
if (pid == 0) {
int open_max = sysconf(_SC_OPEN_MAX), i;
@@ -390,17 +402,17 @@ static int launch_script(const char *setup_script, const
char *ifname, int fd)
*parg = NULL;
execv(setup_script, args);
_exit(1);
-} else if (pid > 0) {
+} else {
while (waitpid(pid, &status, 0) != pid) {
/* loop */
}
if (WIFEXITED(status) && WEXITSTATUS(status) == 0) {
-return 0;
+return;
}
+error_setg(errp, "network script %s failed with status %d",
+ setup_script, status);
}
-fprintf(stderr, "%s: could not launch network script\n", setup_script);
-return -1;
}
static int recv_fd(int c)
@@ -571,6 +583,7 @@ static int net_tap_init(const NetdevTapOptions *tap, int
*vnet_hdr,
const char *setup_script, char *ifname,
size_t ifname_sz, int mq_required)
{
+Error **err = NULL;
int fd, vnet_hdr_required;
if (tap->has_vnet_hdr) {
@@ -589,10 +602,13 @@ static int net_tap_init(const NetdevTapOptions *tap, int
*vnet_hdr,
if (setup_script &&
setup_script[0] != '\0' &&
-strcmp(setup_script, "no") != 0 &&
-launch_script(setup_script, ifname, fd)) {
-close(fd);
-return -1;
+strcmp(setup_script, "no") != 0) {
+launch_script(setup_script, ifname, fd, &err);
+if (err) {
+error_report_err(err);
+close(fd);
+return -1;
+}
}
return fd;
--
1.9.3
[Qemu-devel] [PATCH v2 02/15] net: Permit incremental conversion of init functions to Error
Error reporting for netdev_add is broken: the net_client_init_fun[]
report the actual errors with (at best) error_report(), and their
caller net_client_init1() makes up a generic error on top.
For command line and HMP, this produces an mildly ugly error cascade.
In QMP, the actual errors go to stderr, and the generic error becomes
the command's error reply.
To fix this, we need to convert the net_client_init_fun[] to Error.
To permit fixing them one by one, add an Error ** parameter to the
net_client_init_fun[]. If the call fails without returning an Error,
make up the same generic Error as before. But if it returns one, use
that instead. Since none of them does so far, no functional change.
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
---
net/clients.h| 20 ++--
net/dump.c | 3 ++-
net/hub.c| 2 +-
net/l2tpv3.c | 5 ++---
net/net.c| 15 +--
net/netmap.c | 3 ++-
net/slirp.c | 3 ++-
net/socket.c | 3 ++-
net/tap-win32.c | 3 ++-
net/tap.c| 6 --
net/vde.c| 3 ++-
net/vhost-user.c | 3 ++-
12 files changed, 40 insertions(+), 29 deletions(-)
diff --git a/net/clients.h b/net/clients.h
index 2e8feda..d47530e 100644
--- a/net/clients.h
+++ b/net/clients.h
@@ -28,38 +28,38 @@
#include "qapi-types.h"
int net_init_dump(const NetClientOptions *opts, const char *name,
- NetClientState *peer);
+ NetClientState *peer, Error **errp);
#ifdef CONFIG_SLIRP
int net_init_slirp(const NetClientOptions *opts, const char *name,
- NetClientState *peer);
+ NetClientState *peer, Error **errp);
#endif
int net_init_hubport(const NetClientOptions *opts, const char *name,
- NetClientState *peer);
+ NetClientState *peer, Error **errp);
int net_init_socket(const NetClientOptions *opts, const char *name,
-NetClientState *peer);
+NetClientState *peer, Error **errp);
int net_init_tap(const NetClientOptions *opts, const char *name,
- NetClientState *peer);
+ NetClientState *peer, Error **errp);
int net_init_bridge(const NetClientOptions *opts, const char *name,
-NetClientState *peer);
+NetClientState *peer, Error **errp);
int net_init_l2tpv3(const NetClientOptions *opts, const char *name,
-NetClientState *peer);
+NetClientState *peer, Error **errp);
#ifdef CONFIG_VDE
int net_init_vde(const NetClientOptions *opts, const char *name,
- NetClientState *peer);
+ NetClientState *peer, Error **errp);
#endif
#ifdef CONFIG_NETMAP
int net_init_netmap(const NetClientOptions *opts, const char *name,
-NetClientState *peer);
+NetClientState *peer, Error **errp);
#endif
int net_init_vhost_user(const NetClientOptions *opts, const char *name,
-NetClientState *peer);
+NetClientState *peer, Error **errp);
#endif /* QEMU_NET_CLIENTS_H */
diff --git a/net/dump.c b/net/dump.c
index 9d3a09e..214e88a 100644
--- a/net/dump.c
+++ b/net/dump.c
@@ -146,8 +146,9 @@ static int net_dump_init(NetClientState *peer, const char
*device,
}
int net_init_dump(const NetClientOptions *opts, const char *name,
- NetClientState *peer)
+ NetClientState *peer, Error **errp)
{
+/* FIXME error_setg(errp, ...) on failure */
int len;
const char *file;
char def_file[128];
diff --git a/net/hub.c b/net/hub.c
index 261f8cc..3047f12 100644
--- a/net/hub.c
+++ b/net/hub.c
@@ -281,7 +281,7 @@ int net_hub_id_for_client(NetClientState *nc, int *id)
}
int net_init_hubport(const NetClientOptions *opts, const char *name,
- NetClientState *peer)
+ NetClientState *peer, Error **errp)
{
const NetdevHubPortOptions *hubport;
diff --git a/net/l2tpv3.c b/net/l2tpv3.c
index 8c598b0..ed395dc 100644
--- a/net/l2tpv3.c
+++ b/net/l2tpv3.c
@@ -536,10 +536,9 @@ static NetClientInfo net_l2tpv3_info = {
int net_init_l2tpv3(const NetClientOptions *opts,
const char *name,
-NetClientState *peer)
+NetClientState *peer, Error **errp)
{
-
-
+/* FIXME error_setg(errp, ...) on failure */
const NetdevL2TPv3Options *l2tpv3;
NetL2TPV3State *s;
NetClientState *nc;
diff --git a/net/net.c b/net/net.c
index d9aaeb5..3295741 100644
--- a/net/net.c
+++ b/net/net.c
@@ -740,8 +740,9 @@ int qemu_find_nic_model(NICInfo *nd, const char * const
*models,
}
static int net_init_nic(const NetClientOptions *opts, const char *name,
-NetClientState *peer)
+NetClientState *peer, Error **errp)
{
+/* FIXME error_setg(errp, ...) on failure */
int idx;
NIC
[Qemu-devel] [PATCH v2 05/15] tap: net_tap_fd_init() can't fail, drop dead error handling
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
---
net/tap.c | 14 +-
1 file changed, 1 insertion(+), 13 deletions(-)
diff --git a/net/tap.c b/net/tap.c
index 8f06cb7..adb1022 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -536,7 +536,6 @@ int net_init_bridge(const NetClientOptions *opts, const
char *name,
/* FIXME error_setg(errp, ...) on failure */
const NetdevBridgeOptions *bridge;
const char *helper, *br;
-
TAPState *s;
int fd, vnet_hdr;
@@ -552,14 +551,8 @@ int net_init_bridge(const NetClientOptions *opts, const
char *name,
}
fcntl(fd, F_SETFL, O_NONBLOCK);
-
vnet_hdr = tap_probe_vnet_hdr(fd);
-
s = net_tap_fd_init(peer, "bridge", name, fd, vnet_hdr);
-if (!s) {
-close(fd);
-return -1;
-}
snprintf(s->nc.info_str, sizeof(s->nc.info_str), "helper=%s,br=%s", helper,
br);
@@ -607,14 +600,9 @@ static int net_init_tap_one(const NetdevTapOptions *tap,
NetClientState *peer,
int vnet_hdr, int fd)
{
Error *err = NULL;
-TAPState *s;
+TAPState *s = net_tap_fd_init(peer, model, name, fd, vnet_hdr);
int vhostfd;
-s = net_tap_fd_init(peer, model, name, fd, vnet_hdr);
-if (!s) {
-return -1;
-}
-
if (tap_set_sndbuf(s->fd, tap) < 0) {
return -1;
}
--
1.9.3
[Qemu-devel] [PATCH v2 03/15] net: Improve -net nic error reporting
When -net nic fails, it first reports a specific error, then a generic
one, like this:
$ qemu-system-x86_64 -net nic,netdev=nonexistent
qemu-system-x86_64: -net nic,netdev=nonexistent: netdev 'nonexistent' not
found
qemu-system-x86_64: -net nic,netdev=nonexistent: Device 'nic' could not be
initialized
Convert net_init_nic() to Error to get rid of the unwanted second
error message.
While there, tidy up an Overcapitalized Error Message.
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
---
net/net.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/net/net.c b/net/net.c
index 3295741..0fe5b8b 100644
--- a/net/net.c
+++ b/net/net.c
@@ -742,7 +742,6 @@ int qemu_find_nic_model(NICInfo *nd, const char * const
*models,
static int net_init_nic(const NetClientOptions *opts, const char *name,
NetClientState *peer, Error **errp)
{
-/* FIXME error_setg(errp, ...) on failure */
int idx;
NICInfo *nd;
const NetLegacyNicOptions *nic;
@@ -752,7 +751,7 @@ static int net_init_nic(const NetClientOptions *opts, const
char *name,
idx = nic_get_free_idx();
if (idx == -1 || nb_nics >= MAX_NICS) {
-error_report("Too Many NICs");
+error_setg(errp, "too many NICs");
return -1;
}
@@ -763,7 +762,7 @@ static int net_init_nic(const NetClientOptions *opts, const
char *name,
if (nic->has_netdev) {
nd->netdev = qemu_find_netdev(nic->netdev);
if (!nd->netdev) {
-error_report("netdev '%s' not found", nic->netdev);
+error_setg(errp, "netdev '%s' not found", nic->netdev);
return -1;
}
} else {
@@ -780,19 +779,20 @@ static int net_init_nic(const NetClientOptions *opts,
const char *name,
if (nic->has_macaddr &&
net_parse_macaddr(nd->macaddr.a, nic->macaddr) < 0) {
-error_report("invalid syntax for ethernet address");
+error_setg(errp, "invalid syntax for ethernet address");
return -1;
}
if (nic->has_macaddr &&
is_multicast_ether_addr(nd->macaddr.a)) {
-error_report("NIC cannot have multicast MAC address (odd 1st byte)");
+error_setg(errp,
+ "NIC cannot have multicast MAC address (odd 1st byte)");
return -1;
}
qemu_macaddr_default_if_unset(&nd->macaddr);
if (nic->has_vectors) {
if (nic->vectors > 0x7ff) {
-error_report("invalid # of vectors: %"PRIu32, nic->vectors);
+error_setg(errp, "invalid # of vectors: %"PRIu32, nic->vectors);
return -1;
}
nd->nvectors = nic->vectors;
--
1.9.3
[Qemu-devel] [PATCH v2 08/15] tap: Convert net_init_tap_one() to Error
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
---
net/tap.c | 70 ++-
1 file changed, 38 insertions(+), 32 deletions(-)
diff --git a/net/tap.c b/net/tap.c
index d54222d..d1f5644 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -600,11 +600,11 @@ static int net_tap_init(const NetdevTapOptions *tap, int
*vnet_hdr,
#define MAX_TAP_QUEUES 1024
-static int net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
-const char *model, const char *name,
-const char *ifname, const char *script,
-const char *downscript, const char *vhostfdname,
-int vnet_hdr, int fd)
+static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
+ const char *model, const char *name,
+ const char *ifname, const char *script,
+ const char *downscript, const char *vhostfdname,
+ int vnet_hdr, int fd, Error **errp)
{
Error *err = NULL;
TAPState *s = net_tap_fd_init(peer, model, name, fd, vnet_hdr);
@@ -612,8 +612,8 @@ static int net_init_tap_one(const NetdevTapOptions *tap,
NetClientState *peer,
tap_set_sndbuf(s->fd, tap, &err);
if (err) {
-error_report_err(err);
-return -1;
+error_propagate(errp, err);
+return;
}
if (tap->has_fd || tap->has_fds) {
@@ -644,30 +644,28 @@ static int net_init_tap_one(const NetdevTapOptions *tap,
NetClientState *peer,
if (tap->has_vhostfd || tap->has_vhostfds) {
vhostfd = monitor_fd_param(cur_mon, vhostfdname, &err);
if (vhostfd == -1) {
-error_report_err(err);
-return -1;
+error_propagate(errp, err);
+return;
}
} else {
vhostfd = open("/dev/vhost-net", O_RDWR);
if (vhostfd < 0) {
-error_report("tap: open vhost char device failed: %s",
- strerror(errno));
-return -1;
+error_setg_errno(errp, errno,
+ "tap: open vhost char device failed: %s");
+return;
}
}
options.opaque = (void *)(uintptr_t)vhostfd;
s->vhost_net = vhost_net_init(&options);
if (!s->vhost_net) {
-error_report("vhost-net requested but could not be initialized");
-return -1;
+error_setg(errp,
+ "vhost-net requested but could not be initialized");
+return;
}
} else if (tap->has_vhostfd || tap->has_vhostfds) {
-error_report("vhostfd= is not valid without vhost");
-return -1;
+error_setg(errp, "vhostfd= is not valid without vhost");
}
-
-return 0;
}
static int get_fds(char *str, char *fds[], int max)
@@ -741,9 +739,11 @@ int net_init_tap(const NetClientOptions *opts, const char
*name,
vnet_hdr = tap_probe_vnet_hdr(fd);
-if (net_init_tap_one(tap, peer, "tap", name, NULL,
- script, downscript,
- vhostfdname, vnet_hdr, fd)) {
+net_init_tap_one(tap, peer, "tap", name, NULL,
+ script, downscript,
+ vhostfdname, vnet_hdr, fd, &err);
+if (err) {
+error_report_err(err);
return -1;
}
} else if (tap->has_fds) {
@@ -786,10 +786,12 @@ int net_init_tap(const NetClientOptions *opts, const char
*name,
return -1;
}
-if (net_init_tap_one(tap, peer, "tap", name, ifname,
- script, downscript,
- tap->has_vhostfds ? vhost_fds[i] : NULL,
- vnet_hdr, fd)) {
+net_init_tap_one(tap, peer, "tap", name, ifname,
+ script, downscript,
+ tap->has_vhostfds ? vhost_fds[i] : NULL,
+ vnet_hdr, fd, &err);
+if (err) {
+error_report_err(err);
return -1;
}
}
@@ -810,9 +812,11 @@ int net_init_tap(const NetClientOptions *opts, const char
*name,
fcntl(fd, F_SETFL, O_NONBLOCK);
vnet_hdr = tap_probe_vnet_hdr(fd);
-if (net_init_tap_one(tap, peer, "bridge", name, ifname,
- script, downscript, vhostfdname,
- vnet_hdr, fd)) {
+net_init_tap_one(tap, peer, "bridge", name, ifname,
+ script, downscript, vhostfdname,
+ vnet_hdr, fd, &err);
+if (err) {
+error_report_err(err);
close(fd);
return -1;
}
@@ -846,10 +8
[Qemu-devel] [PATCH v2 10/15] tap: Permit incremental conversion of tap_open() to Error
Convert the trivial ones immediately: tap-aix and tap-haiku.
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
---
net/tap-aix.c | 4 ++--
net/tap-bsd.c | 6 --
net/tap-haiku.c | 4 ++--
net/tap-linux.c | 3 ++-
net/tap-solaris.c | 3 ++-
net/tap.c | 13 +
net/tap_int.h | 2 +-
7 files changed, 22 insertions(+), 13 deletions(-)
diff --git a/net/tap-aix.c b/net/tap-aix.c
index 0a3d461..18fdbf3 100644
--- a/net/tap-aix.c
+++ b/net/tap-aix.c
@@ -26,9 +26,9 @@
#include
int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
- int vnet_hdr_required, int mq_required)
+ int vnet_hdr_required, int mq_required, Error **errp)
{
-fprintf(stderr, "no tap on AIX\n");
+error_setg(errp, "no tap on AIX");
return -1;
}
diff --git a/net/tap-bsd.c b/net/tap-bsd.c
index 53cdd9f..bbbe446 100644
--- a/net/tap-bsd.c
+++ b/net/tap-bsd.c
@@ -35,8 +35,9 @@
#ifndef __FreeBSD__
int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
- int vnet_hdr_required, int mq_required)
+ int vnet_hdr_required, int mq_required, Error **errp)
{
+/* FIXME error_setg(errp, ...) on failure */
int fd;
#ifdef TAPGIFNAME
struct ifreq ifr;
@@ -114,8 +115,9 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
#define PATH_NET_TAP "/dev/tap"
int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
- int vnet_hdr_required, int mq_required)
+ int vnet_hdr_required, int mq_required, Error **errp)
{
+/* FIXME error_setg(errp, ...) on failure */
int fd, s, ret;
struct ifreq ifr;
diff --git a/net/tap-haiku.c b/net/tap-haiku.c
index 0905b28..d18590c 100644
--- a/net/tap-haiku.c
+++ b/net/tap-haiku.c
@@ -26,9 +26,9 @@
#include
int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
- int vnet_hdr_required, int mq_required)
+ int vnet_hdr_required, int mq_required, Error **errp)
{
-fprintf(stderr, "no tap on Haiku\n");
+error_setg(errp, "no tap on Haiku");
return -1;
}
diff --git a/net/tap-linux.c b/net/tap-linux.c
index 6fa2744..be18382 100644
--- a/net/tap-linux.c
+++ b/net/tap-linux.c
@@ -37,8 +37,9 @@
#define PATH_NET_TUN "/dev/net/tun"
int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
- int vnet_hdr_required, int mq_required)
+ int vnet_hdr_required, int mq_required, Error **errp)
{
+/* FIXME error_setg(errp, ...) on failure */
struct ifreq ifr;
int fd, ret;
int len = sizeof(struct virtio_net_hdr);
diff --git a/net/tap-solaris.c b/net/tap-solaris.c
index 7839323..079046b 100644
--- a/net/tap-solaris.c
+++ b/net/tap-solaris.c
@@ -174,8 +174,9 @@ static int tap_alloc(char *dev, size_t dev_size)
}
int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
- int vnet_hdr_required, int mq_required)
+ int vnet_hdr_required, int mq_required, Error **errp)
{
+/* FIXME error_setg(errp, ...) on failure */
char dev[10]="";
int fd;
if( (fd = tap_alloc(dev, sizeof(dev))) < 0 ){
diff --git a/net/tap.c b/net/tap.c
index 08bfc51..348b786 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -581,7 +581,7 @@ int net_init_bridge(const NetClientOptions *opts, const
char *name,
static int net_tap_init(const NetdevTapOptions *tap, int *vnet_hdr,
const char *setup_script, char *ifname,
-size_t ifname_sz, int mq_required)
+size_t ifname_sz, int mq_required, Error **errp)
{
Error **err = NULL;
int fd, vnet_hdr_required;
@@ -595,8 +595,12 @@ static int net_tap_init(const NetdevTapOptions *tap, int
*vnet_hdr,
}
TFR(fd = tap_open(ifname, ifname_sz, vnet_hdr, vnet_hdr_required,
- mq_required));
+ mq_required, errp));
if (fd < 0) {
+/* FIXME drop when all tap_open() store an Error */
+if (errp && !*errp) {
+error_setg(errp, "can't open tap device");
+}
return -1;
}
@@ -605,7 +609,7 @@ static int net_tap_init(const NetdevTapOptions *tap, int
*vnet_hdr,
strcmp(setup_script, "no") != 0) {
launch_script(setup_script, ifname, fd, &err);
if (err) {
-error_report_err(err);
+error_propagate(errp, err);
close(fd);
return -1;
}
@@ -853,8 +857,9 @@ int net_init_tap(const NetClientOptions *opts, const char
*name,
for (i = 0; i < queues; i++) {
fd = net_tap_init(tap, &vnet_hdr, i >= 1 ? "no" : script,
- ifname, sizeof ifname, queues > 1);
+ ifname, sizeof ifname, queues > 1, &err);
if (fd == -1) {
+error_report_err(err);
return -1;
}
diff --git a/net/tap_int.h b/net/tap_int.h
index 6df271f..d12a409 100644
--- a/net/tap_int.h
+++ b/n
[Qemu-devel] [PATCH v2 04/15] net/dump: Improve -net/host_net_add dump error reporting
When -net dump fails, it first reports a specific error, then a
generic one, like this:
$ qemu-system-x86_64 -net dump,id=foo,file=/eperm
qemu-system-x86_64: -net dump,id=foo,file=/eperm: -net dump: can't open
/eperm
qemu-system-x86_64: -net dump,id=foo,file=/eperm: Device 'dump' could not
be initialized
Convert net_init_tap() to Error. This suppresses the unwanted second
message.
Improve the error messages to include strerror(errno) where
appropriate.
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
---
net/dump.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/net/dump.c b/net/dump.c
index 214e88a..02c8064 100644
--- a/net/dump.c
+++ b/net/dump.c
@@ -101,7 +101,8 @@ static NetClientInfo net_dump_info = {
};
static int net_dump_init(NetClientState *peer, const char *device,
- const char *name, const char *filename, int len)
+ const char *name, const char *filename, int len,
+ Error **errp)
{
struct pcap_file_hdr hdr;
NetClientState *nc;
@@ -111,7 +112,7 @@ static int net_dump_init(NetClientState *peer, const char
*device,
fd = open(filename, O_CREAT | O_TRUNC | O_WRONLY | O_BINARY, 0644);
if (fd < 0) {
-error_report("-net dump: can't open %s", filename);
+error_setg_errno(errp, errno, "-net dump: can't open %s", filename);
return -1;
}
@@ -124,7 +125,7 @@ static int net_dump_init(NetClientState *peer, const char
*device,
hdr.linktype = 1;
if (write(fd, &hdr, sizeof(hdr)) < sizeof(hdr)) {
-error_report("-net dump write error: %s", strerror(errno));
+error_setg_errno(errp, errno, "-net dump write error");
close(fd);
return -1;
}
@@ -148,7 +149,6 @@ static int net_dump_init(NetClientState *peer, const char
*device,
int net_init_dump(const NetClientOptions *opts, const char *name,
NetClientState *peer, Error **errp)
{
-/* FIXME error_setg(errp, ...) on failure */
int len;
const char *file;
char def_file[128];
@@ -174,7 +174,7 @@ int net_init_dump(const NetClientOptions *opts, const char
*name,
if (dump->has_len) {
if (dump->len > INT_MAX) {
-error_report("invalid length: %"PRIu64, dump->len);
+error_setg(errp, "invalid length: %"PRIu64, dump->len);
return -1;
}
len = dump->len;
@@ -182,5 +182,5 @@ int net_init_dump(const NetClientOptions *opts, const char
*name,
len = 65536;
}
-return net_dump_init(peer, "dump", name, file, len);
+return net_dump_init(peer, "dump", name, file, len, errp);
}
--
1.9.3
[Qemu-devel] [PATCH v2 11/15] tap-linux: Convert tap_open() to Error
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
---
net/tap-linux.c | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/net/tap-linux.c b/net/tap-linux.c
index be18382..6c3caef 100644
--- a/net/tap-linux.c
+++ b/net/tap-linux.c
@@ -39,7 +39,6 @@
int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
int vnet_hdr_required, int mq_required, Error **errp)
{
-/* FIXME error_setg(errp, ...) on failure */
struct ifreq ifr;
int fd, ret;
int len = sizeof(struct virtio_net_hdr);
@@ -47,7 +46,7 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
TFR(fd = open(PATH_NET_TUN, O_RDWR));
if (fd < 0) {
-error_report("could not open %s: %m", PATH_NET_TUN);
+error_setg_errno(errp, errno, "could not open %s", PATH_NET_TUN);
return -1;
}
memset(&ifr, 0, sizeof(ifr));
@@ -71,8 +70,8 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
}
if (vnet_hdr_required && !*vnet_hdr) {
-error_report("vnet_hdr=1 requested, but no kernel "
- "support for IFF_VNET_HDR available");
+error_setg(errp, "vnet_hdr=1 requested, but no kernel "
+ "support for IFF_VNET_HDR available");
close(fd);
return -1;
}
@@ -87,8 +86,8 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
if (mq_required) {
if (!(features & IFF_MULTI_QUEUE)) {
-error_report("multiqueue required, but no kernel "
- "support for IFF_MULTI_QUEUE available");
+error_setg(errp, "multiqueue required, but no kernel "
+ "support for IFF_MULTI_QUEUE available");
close(fd);
return -1;
} else {
@@ -103,9 +102,11 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
ret = ioctl(fd, TUNSETIFF, (void *) &ifr);
if (ret != 0) {
if (ifname[0] != '\0') {
-error_report("could not configure %s (%s): %m", PATH_NET_TUN,
ifr.ifr_name);
+error_setg_errno(errp, errno, "could not configure %s (%s)",
+ PATH_NET_TUN, ifr.ifr_name);
} else {
-error_report("could not configure %s: %m", PATH_NET_TUN);
+error_setg_errno(errp, errno, "could not configure %s",
+ PATH_NET_TUN);
}
close(fd);
return -1;
--
1.9.3
[Qemu-devel] [PATCH v2 13/15] tap-solaris: Convert tap_open() to Error
Fixes inappropriate use of syslog().
Not fixed: leaks on error paths, suspicious non-fatal errors. FIXMEs
added instead.
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
---
net/tap-solaris.c | 59 ---
1 file changed, 30 insertions(+), 29 deletions(-)
diff --git a/net/tap-solaris.c b/net/tap-solaris.c
index 079046b..90b2fd1 100644
--- a/net/tap-solaris.c
+++ b/net/tap-solaris.c
@@ -36,7 +36,6 @@
#include
#include
#include
-#include
#include
#include "qemu/error-report.h"
@@ -56,8 +55,10 @@ ssize_t tap_read_packet(int tapfd, uint8_t *buf, int maxlen)
* Allocate TAP device, returns opened fd.
* Stores dev name in the first arg(must be large enough).
*/
-static int tap_alloc(char *dev, size_t dev_size)
+static int tap_alloc(char *dev, size_t dev_size, Error **errp)
{
+/* FIXME leaks like a sieve on error paths */
+/* FIXME suspicious: many errors are reported, then ignored */
int tap_fd, if_fd, ppa = -1;
static int ip_fd = 0;
char *ptr;
@@ -83,14 +84,14 @@ static int tap_alloc(char *dev, size_t dev_size)
TFR(ip_fd = open("/dev/udp", O_RDWR, 0));
if (ip_fd < 0) {
- syslog(LOG_ERR, "Can't open /dev/ip (actually /dev/udp)");
- return -1;
+error_setg(errp, "Can't open /dev/ip (actually /dev/udp)");
+return -1;
}
TFR(tap_fd = open("/dev/tap", O_RDWR, 0));
if (tap_fd < 0) {
- syslog(LOG_ERR, "Can't open /dev/tap");
- return -1;
+error_setg(errp, "Can't open /dev/tap");
+return -1;
}
/* Assign a new PPA and get its unit number. */
@@ -99,20 +100,20 @@ static int tap_alloc(char *dev, size_t dev_size)
strioc_ppa.ic_len = sizeof(ppa);
strioc_ppa.ic_dp = (char *)&ppa;
if ((ppa = ioctl (tap_fd, I_STR, &strioc_ppa)) < 0)
- syslog (LOG_ERR, "Can't assign new interface");
+error_report("Can't assign new interface");
TFR(if_fd = open("/dev/tap", O_RDWR, 0));
if (if_fd < 0) {
- syslog(LOG_ERR, "Can't open /dev/tap (2)");
- return -1;
+error_setg(errp, "Can't open /dev/tap (2)");
+return -1;
}
if(ioctl(if_fd, I_PUSH, "ip") < 0){
- syslog(LOG_ERR, "Can't push IP module");
- return -1;
+error_setg(errp, "Can't push IP module");
+return -1;
}
if (ioctl(if_fd, SIOCGLIFFLAGS, &ifr) < 0)
- syslog(LOG_ERR, "Can't get flags\n");
+error_report("Can't get flags");
snprintf (actual_name, 32, "tap%d", ppa);
pstrcpy(ifr.lifr_name, sizeof(ifr.lifr_name), actual_name);
@@ -121,22 +122,22 @@ static int tap_alloc(char *dev, size_t dev_size)
/* Assign ppa according to the unit number returned by tun device */
if (ioctl (if_fd, SIOCSLIFNAME, &ifr) < 0)
-syslog (LOG_ERR, "Can't set PPA %d", ppa);
+error_report("Can't set PPA %d", ppa);
if (ioctl(if_fd, SIOCGLIFFLAGS, &ifr) <0)
-syslog (LOG_ERR, "Can't get flags\n");
+error_report("Can't get flags");
/* Push arp module to if_fd */
if (ioctl (if_fd, I_PUSH, "arp") < 0)
-syslog (LOG_ERR, "Can't push ARP module (2)");
+error_report("Can't push ARP module (2)");
/* Push arp module to ip_fd */
if (ioctl (ip_fd, I_POP, NULL) < 0)
-syslog (LOG_ERR, "I_POP failed\n");
+error_report("I_POP failed");
if (ioctl (ip_fd, I_PUSH, "arp") < 0)
-syslog (LOG_ERR, "Can't push ARP module (3)\n");
+error_report("Can't push ARP module (3)");
/* Open arp_fd */
TFR(arp_fd = open ("/dev/tap", O_RDWR, 0));
if (arp_fd < 0)
- syslog (LOG_ERR, "Can't open %s\n", "/dev/tap");
+error_report("Can't open %s", "/dev/tap");
/* Set ifname to arp */
strioc_if.ic_cmd = SIOCSLIFNAME;
@@ -144,16 +145,16 @@ static int tap_alloc(char *dev, size_t dev_size)
strioc_if.ic_len = sizeof(ifr);
strioc_if.ic_dp = (char *)𝔦
if (ioctl(arp_fd, I_STR, &strioc_if) < 0){
-syslog (LOG_ERR, "Can't set ifname to arp\n");
+error_report("Can't set ifname to arp");
}
if((ip_muxid = ioctl(ip_fd, I_LINK, if_fd)) < 0){
- syslog(LOG_ERR, "Can't link TAP device to IP");
- return -1;
+error_setg(errp, "Can't link TAP device to IP");
+return -1;
}
if ((arp_muxid = ioctl (ip_fd, link_type, arp_fd)) < 0)
-syslog (LOG_ERR, "Can't link TAP device to ARP");
+error_report("Can't link TAP device to ARP");
close (if_fd);
@@ -166,7 +167,7 @@ static int tap_alloc(char *dev, size_t dev_size)
{
ioctl (ip_fd, I_PUNLINK , arp_muxid);
ioctl (ip_fd, I_PUNLINK, ip_muxid);
- syslog (LOG_ERR, "Can't set multiplexor id");
+ error_report("Can't set multiplexor id");
}
snprintf(dev, dev_size, "tap%d", ppa);
@@ -176,12 +177,12 @@ static int tap_alloc(char *dev, size_t dev_size)
int tap_open(char *ifname, int ifname_size, int *vnet_hdr
[Qemu-devel] [PATCH v2 12/15] tap-bsd: Convert tap_open() to Error
Fixes inappropriate use of stderr in monitor command handler.
While there, improve some of the messages a bit.
Signed-off-by: Markus Armbruster
---
net/tap-bsd.c | 33 ++---
1 file changed, 14 insertions(+), 19 deletions(-)
diff --git a/net/tap-bsd.c b/net/tap-bsd.c
index bbbe446..5889920 100644
--- a/net/tap-bsd.c
+++ b/net/tap-bsd.c
@@ -37,7 +37,6 @@
int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
int vnet_hdr_required, int mq_required, Error **errp)
{
-/* FIXME error_setg(errp, ...) on failure */
int fd;
#ifdef TAPGIFNAME
struct ifreq ifr;
@@ -72,23 +71,19 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
}
}
if (fd < 0) {
-error_report("warning: could not open %s (%s): no virtual network
emulation",
- dname, strerror(errno));
+error_setg_errno(errp, errno, "could not open %s", dname);
return -1;
}
#ifdef TAPGIFNAME
if (ioctl(fd, TAPGIFNAME, (void *)&ifr) < 0) {
-fprintf(stderr, "warning: could not get tap name: %s\n",
-strerror(errno));
+error_setg_errno(errp, errno, "could not get tap name");
return -1;
}
pstrcpy(ifname, ifname_size, ifr.ifr_name);
#else
if (fstat(fd, &s) < 0) {
-fprintf(stderr,
-"warning: could not stat /dev/tap: no virtual network emulation:
%s\n",
-strerror(errno));
+error_setg_errno(errp, errno, "could not stat %s", dname);
return -1;
}
dev = devname(s.st_rdev, S_IFCHR);
@@ -100,8 +95,8 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
*vnet_hdr = 0;
if (vnet_hdr_required && !*vnet_hdr) {
-error_report("vnet_hdr=1 requested, but no kernel "
- "support for IFF_VNET_HDR available");
+error_setg(errp, "vnet_hdr=1 requested, but no kernel "
+ "support for IFF_VNET_HDR available");
close(fd);
return -1;
}
@@ -117,13 +112,12 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
int vnet_hdr_required, int mq_required, Error **errp)
{
-/* FIXME error_setg(errp, ...) on failure */
int fd, s, ret;
struct ifreq ifr;
TFR(fd = open(PATH_NET_TAP, O_RDWR));
if (fd < 0) {
-error_report("could not open %s: %s", PATH_NET_TAP, strerror(errno));
+error_setg_errno(errp, errno, "could not open %s", PATH_NET_TAP);
return -1;
}
@@ -131,7 +125,7 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
ret = ioctl(fd, TAPGIFNAME, (void *)&ifr);
if (ret < 0) {
-error_report("could not get tap interface name");
+error_setg_errno(errp, errno, "could not get tap interface name");
goto error;
}
@@ -139,14 +133,15 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
/* User requested the interface to have a specific name */
s = socket(AF_LOCAL, SOCK_DGRAM, 0);
if (s < 0) {
-error_report("could not open socket to set interface name");
+error_setg_errno(errp, errno,
+ "could not open socket to set interface name");
goto error;
}
ifr.ifr_data = ifname;
ret = ioctl(s, SIOCSIFNAME, (void *)&ifr);
close(s);
if (ret < 0) {
-error_report("could not set tap interface name");
+error_setg(errp, "could not set tap interface name");
goto error;
}
} else {
@@ -158,14 +153,14 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
*vnet_hdr = 0;
if (vnet_hdr_required && !*vnet_hdr) {
-error_report("vnet_hdr=1 requested, but no kernel "
- "support for IFF_VNET_HDR available");
+error_setg(errp, "vnet_hdr=1 requested, but no kernel "
+ "support for IFF_VNET_HDR available");
goto error;
}
}
if (mq_required) {
-error_report("mq_required requested, but not kernel support"
- "for IFF_MULTI_QUEUE available");
+error_setg(errp, "mq_required requested, but no kernel support"
+ " for IFF_MULTI_QUEUE available");
goto error;
}
--
1.9.3
[Qemu-devel] [PATCH v2 15/15] tap: Improve -netdev/netdev_add/-net/... tap error reporting
When -netdev tap fails, it first reports a specific error, then a
generic one, like this:
$ qemu-system-x86_64 -netdev tap,id=foo
qemu-system-x86_64: -netdev tap,id=foo: could not configure /dev/net/tun:
Operation not permitted
qemu-system-x86_64: -netdev tap,id=foo: Device 'tap' could not be
initialized
With the command line, the messages go to stderr. In HMP, they go to
the monitor. In QMP, the second one becomes the error reply, and the
first one goes to stderr.
Convert net_init_tap() to Error. This suppresses the unwanted second
message, and makes the specific error the QMP error reply.
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
---
net/tap.c | 45 ++---
1 file changed, 22 insertions(+), 23 deletions(-)
diff --git a/net/tap.c b/net/tap.c
index f7db9dc..319eac0 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -713,7 +713,6 @@ static int get_fds(char *str, char *fds[], int max)
int net_init_tap(const NetClientOptions *opts, const char *name,
NetClientState *peer, Error **errp)
{
-/* FIXME error_setg(errp, ...) on failure */
const NetdevTapOptions *tap;
int fd, vnet_hdr = 0, i = 0, queues;
/* for the no-fd, no-helper case */
@@ -731,7 +730,7 @@ int net_init_tap(const NetClientOptions *opts, const char
*name,
/* QEMU vlans does not support multiqueue tap, in this case peer is set.
* For -netdev, peer is always NULL. */
if (peer && (tap->has_queues || tap->has_fds || tap->has_vhostfds)) {
-error_report("Multiqueue tap cannot be used with QEMU vlans");
+error_setg(errp, "Multiqueue tap cannot be used with QEMU vlans");
return -1;
}
@@ -739,15 +738,15 @@ int net_init_tap(const NetClientOptions *opts, const char
*name,
if (tap->has_ifname || tap->has_script || tap->has_downscript ||
tap->has_vnet_hdr || tap->has_helper || tap->has_queues ||
tap->has_fds || tap->has_vhostfds) {
-error_report("ifname=, script=, downscript=, vnet_hdr=, "
- "helper=, queues=, fds=, and vhostfds= "
- "are invalid with fd=");
+error_setg(errp, "ifname=, script=, downscript=, vnet_hdr=, "
+ "helper=, queues=, fds=, and vhostfds= "
+ "are invalid with fd=");
return -1;
}
fd = monitor_fd_param(cur_mon, tap->fd, &err);
if (fd == -1) {
-error_report_err(err);
+error_propagate(errp, err);
return -1;
}
@@ -759,7 +758,7 @@ int net_init_tap(const NetClientOptions *opts, const char
*name,
script, downscript,
vhostfdname, vnet_hdr, fd, &err);
if (err) {
-error_report_err(err);
+error_propagate(errp, err);
return -1;
}
} else if (tap->has_fds) {
@@ -770,9 +769,9 @@ int net_init_tap(const NetClientOptions *opts, const char
*name,
if (tap->has_ifname || tap->has_script || tap->has_downscript ||
tap->has_vnet_hdr || tap->has_helper || tap->has_queues ||
tap->has_vhostfd) {
-error_report("ifname=, script=, downscript=, vnet_hdr=, "
- "helper=, queues=, and vhostfd= "
- "are invalid with fds=");
+error_setg(errp, "ifname=, script=, downscript=, vnet_hdr=, "
+ "helper=, queues=, and vhostfd= "
+ "are invalid with fds=");
return -1;
}
@@ -780,8 +779,8 @@ int net_init_tap(const NetClientOptions *opts, const char
*name,
if (tap->has_vhostfds) {
nvhosts = get_fds(tap->vhostfds, vhost_fds, MAX_TAP_QUEUES);
if (nfds != nvhosts) {
-error_report("The number of fds passed does not match the "
- "number of vhostfds passed");
+error_setg(errp, "The number of fds passed does not match "
+ "the number of vhostfds passed");
return -1;
}
}
@@ -789,7 +788,7 @@ int net_init_tap(const NetClientOptions *opts, const char
*name,
for (i = 0; i < nfds; i++) {
fd = monitor_fd_param(cur_mon, fds[i], &err);
if (fd == -1) {
-error_report_err(err);
+error_propagate(errp, err);
return -1;
}
@@ -798,7 +797,8 @@ int net_init_tap(const NetClientOptions *opts, const char
*name,
if (i == 0) {
vnet_hdr = tap_probe_vnet_hdr(fd);
} else if (vnet_hdr != tap_probe_vnet_hdr(fd)) {
-error_report("vnet_hdr not consistent across given tap fds");
+error_setg(errp,
+ "vnet_hdr not consistent across given tap fds");
return -1;
}
@
Re: [Qemu-devel] [PATCH v3 2/2] target-mips: Misaligned memory accesses for MSA
On 14/05/2015 20:12, Richard Henderson wrote:
> /* We know both pages are present and writable. */
> if (eaddr == baddr + 15) {
> /* Consecutive pages in RAM. */
> memcpy(baddr, register, 16);
> } else {
> /* Someone's doing an MSA store to device memory. */
> for (i = 0; i < 2; ++i) {
> helper_ret_stq_mmu(env, vaddr + i*8, register.d[0],
> make_memop_idx(MO_UNALN | MO_TEQ, mmu_idx),
> GETRA());
> }
> }
We would additionally need to take care of vector elements' endianness
before using this code. Therefore always using helper_ret_st[bwlq]_mmu
(depending on data format) in a loop probably is simpler.
Leon
Re: [Qemu-devel] [RFC PATCH v4 09/28] COLO: Save VM state to slave when do checkpoint
* zhanghailiang (zhang.zhanghaili...@huawei.com) wrote:
> We should save PVM's RAM/device to slave when needed.
>
> For VM state, we will cache them in slave, we use QEMUSizedBuffer
> to store the data, we need know the data size of VM state, so in master,
> we use qsb to store VM state temporarily, and then migrate the data to
> slave.
>
> Signed-off-by: zhanghailiang
> Signed-off-by: Yang Hongyang
> Signed-off-by: Gonglei
> Signed-off-by: Lai Jiangshan
> Signed-off-by: Li Zhijian
> ---
> arch_init.c | 22 ++--
> migration/colo.c | 62
>
> savevm.c | 2 +-
> 3 files changed, 79 insertions(+), 7 deletions(-)
>
> diff --git a/arch_init.c b/arch_init.c
> index fcfa328..e928e11 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -53,6 +53,7 @@
> #include "hw/acpi/acpi.h"
> #include "qemu/host-utils.h"
> #include "qemu/rcu_queue.h"
> +#include "migration/migration-colo.h"
>
> #ifdef DEBUG_ARCH_INIT
> #define DPRINTF(fmt, ...) \
> @@ -845,6 +846,13 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
> RAMBlock *block;
> int64_t ram_bitmap_pages; /* Size of bitmap in pages, including gaps */
>
> +/*
> + * migration has already setup the bitmap, reuse it.
> + */
> +if (migrate_in_colo_state()) {
> +goto setup_part;
> +}
> +
This is a bit odd. It would be easier if you just moved the init code
inside this if, rather than goto'ing over it (or move the other code that
you actually want into another function that then gets called from the bottom
of here?)
The thing that also makes it especially odd is that you goto over
the rcu_read_lock and then have to fix it up; that's getting messy.
(The qemu style seems to be OK to use goto to jump to a shared error
block at the end of a function but otherwise it should be rare).
> mig_throttle_on = false;
> dirty_rate_high_cnt = 0;
> bitmap_sync_count = 0;
> @@ -901,9 +909,12 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
> migration_bitmap_sync();
> qemu_mutex_unlock_ramlist();
> qemu_mutex_unlock_iothread();
> -
> +setup_part:
> qemu_put_be64(f, ram_bytes_total() | RAM_SAVE_FLAG_MEM_SIZE);
>
> +if (migrate_in_colo_state()) {
> +rcu_read_lock();
> +}
> QLIST_FOREACH_RCU(block, &ram_list.blocks, next) {
> qemu_put_byte(f, strlen(block->idstr));
> qemu_put_buffer(f, (uint8_t *)block->idstr, strlen(block->idstr));
> @@ -1007,7 +1018,14 @@ static int ram_save_complete(QEMUFile *f, void *opaque)
> }
>
> ram_control_after_iterate(f, RAM_CONTROL_FINISH);
> -migration_end();
> +
> +/*
> + * Since we need to reuse dirty bitmap in colo,
> + * don't cleanup the bitmap.
> + */
> +if (!migrate_enable_colo() ||
> migration_has_failed(migrate_get_current())) {
> +migration_end();
> +}
>
> rcu_read_unlock();
> qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
> diff --git a/migration/colo.c b/migration/colo.c
> index 5a8ed1b..64e3f3a 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -60,6 +60,9 @@ enum {
>
> static QEMUBH *colo_bh;
> static Coroutine *colo;
> +/* colo buffer */
> +#define COLO_BUFFER_BASE_SIZE (1000*1000*4ULL)
Surely you want that as 4*1024*1024 ? Anyway, now that qemu has
migrate_set_parameter, it's probably best to wire these magic numbers
to parameters that can be configured.
> +QEMUSizedBuffer *colo_buffer;
>
> bool colo_supported(void)
> {
> @@ -123,6 +126,8 @@ static int colo_ctl_get(QEMUFile *f, uint64_t require)
> static int colo_do_checkpoint_transaction(MigrationState *s, QEMUFile
> *control)
> {
> int ret;
> +size_t size;
> +QEMUFile *trans = NULL;
>
> ret = colo_ctl_put(s->file, COLO_CHECKPOINT_NEW);
> if (ret < 0) {
> @@ -133,16 +138,47 @@ static int
> colo_do_checkpoint_transaction(MigrationState *s, QEMUFile *control)
> if (ret < 0) {
> goto out;
> }
> +/* Reset colo buffer and open it for write */
> +qsb_set_length(colo_buffer, 0);
> +trans = qemu_bufopen("w", colo_buffer);
> +if (!trans) {
> +error_report("Open colo buffer for write failed");
> +goto out;
> +}
> +
> +/* suspend and save vm state to colo buffer */
> +qemu_mutex_lock_iothread();
> +vm_stop_force_state(RUN_STATE_COLO);
> +qemu_mutex_unlock_iothread();
> +DPRINTF("vm is stoped\n");
> +
> +/* Disable block migration */
> +s->params.blk = 0;
> +s->params.shared = 0;
> +qemu_savevm_state_begin(trans, &s->params);
> +qemu_mutex_lock_iothread();
> +qemu_savevm_state_complete(trans);
> +qemu_mutex_unlock_iothread();
>
> -/* TODO: suspend and save vm state to colo buffer */
> +qemu_fflush(trans);
>
> ret = colo_ctl_put(s->file, COLO_CHECKPOINT_SEND);
> if (ret < 0) {
> goto out;
> }
> +/* we send the total size of the vmstate firs
Re: [Qemu-devel] [PATCH v6 05/22] hw/acpi/aml-build: Add aml_interrupt() term
On Thu, 7 May 2015 16:51:31 +0100 Peter Maydell wrote: > On 7 May 2015 at 10:29, Shannon Zhao wrote: > > From: Shannon Zhao > > > > Add aml_interrupt() for describing device interrupt in resource template. > > These can be used to generating DSDT table for ACPI on ARM. > > > +/* Interrupt Number */ > > +build_append_byte(var->buf, extract32(irq, 0, 8)); /* bits[7:0] */ > > +build_append_byte(var->buf, extract32(irq, 8, 8)); /* bits[15:8] */ > > +build_append_byte(var->buf, extract32(irq, 16, 8)); /* bits[23:16] */ > > +build_append_byte(var->buf, extract32(irq, 24, 8)); /* bits[31:24] */ > > You used this twice in the previous patch and again here. Can > we factor it out so we can > build_append_uint32(var->buf, irq); I'd prefer to leave it as it is now, yes it's a bit of code duplication but when you compare to spec it's 1:1 match and easy to compare > > ? > > thanks > -- PMM >
Re: [Qemu-devel] [PATCH 0/3] scripts/qemu-gdb: Add event tracing support
Peter Maydell writes: > On 15 May 2015 at 08:58, Markus Armbruster wrote: >> Since you're touching qemu-gdb.py anyway, could you stick in a brief >> comment explaining how to put it to use? > > Good idea. It turns out the answer is just "source it from gdb", > but it took me a little while to find that out, so worth commenting. > I also have a patch which makes it do the 'ignore SIGUSR1' bit > by doing 'handle SIGUSR1 pass noprint nostop' for you. Here's how to load scripts/qemu-gdb.py automatically: * Apply the appended patch to turn it into a gdb init file That's what it is, after all. It's not a standalone Python program. * Tell gdb to trust it Add a line like add-auto-load-safe-path ~/work/qemu/scripts/qemu-gdb.py to your ~/.gdbinit * Link it into the directory where you run gdb --args qemu... * Verify it works: $ gdb [...] (gdb) help qemu Prefix for QEMU debug support commands List of qemu subcommands: qemu coroutine -- Display coroutine backtrace qemu mtree -- Display the memory tree hierarchy Type "help qemu" followed by qemu subcommand name for full documentation. Type "apropos word" to search for commands related to "word". Command name abbreviations are allowed if unambiguous. (gdb) If you know a better way to do this, please post it. diff --git a/scripts/qemu-gdb.py b/scripts/qemu-gdb.py index 6c7f4fb..ac3087c 100644 --- a/scripts/qemu-gdb.py +++ b/scripts/qemu-gdb.py @@ -1,5 +1,3 @@ -#!/usr/bin/python - # GDB debugging support # # Copyright 2012 Red Hat, Inc. and/or its affiliates @@ -13,7 +11,7 @@ # Contributions after 2012-01-13 are licensed under the terms of the # GNU GPL, version 2 or (at your option) any later version. - +python import gdb def isnull(ptr):
[Qemu-devel] [Bug 1455475] [NEW] Block Commit: [100 %]error: failed to pivot job for disk vda
Public bug reported: Hi, i´ve a Problem with committing a snapshot. The problem was discussed on the libvirt mailing list earlier this year. https://www.redhat.com/archives/libvirt-users/2015-January/msg00029.html Iam running gentoo and: Compiled against library: libvirt 1.2.14 Using library: libvirt 1.2.14 Using API: QEMU 1.2.14 Running hypervisor: QEMU 2.3.0 I´am running a Windows Server 2012 R2 VM with the latest quest driver (0.1.96) and QEMU quest Agent 0.12.1 installed. The domain is started with following command line: /usr/bin/qemu-system-x86_64 -name $DOMAIN -S -machine pc-i440fx-1.6,accel=kvm,usb=off -m 8192 -realtime mlock=off -smp 8,sockets=8,cores=1,threads=1 -uuid c96ef576-dbfc-49aa-9dd0-068886c4ef0e -no-user-config -nodefaults -chardev socket,id=charmonitor,path=/var/lib/libvirt/qemu/$DOMAIN.monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=control -rtc base=localtime -no-shutdown -boot strict=on -device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 -device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x6 -drive file=/var/lib/libvirt/images/$DOMAIN.qcow2,if=none,id=drive-virtio-disk0,format=qcow2 -device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x2,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1 -netdev tap,fd=25,id=hostnet0,vhost=on,vhostfd=26 -device virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:07:b4:0a,bus=pci.0,addr=0x3 -chardev pty,id=charserial0 -device isa-serial,chardev=charserial0,id=serial0 -chardev socket,id=charchannel0,path=/var/lib/libvirt/qemu/f16x86_64.agent,server,nowait -device virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,id=channel0,name=org.qemu.guest_agent.0 -device usb-tablet,id=input0 -vnc 127.0.0.1:6 -k de -device VGA,id=video0,vgamem_mb=16,bus=pci.0,addr=0x5 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4 -msg timestamp=on char device redirected to /dev/pts/2 (label charserial0) I´ve created the snapshot with: virsh # snapshot-create-as --domain $DOMAIN snap1 --diskspec vda,file=/var/lib/libvirt/images/snap1.qcow2 --quiesce --disk-only --atomic --no-metadata Domain snapshot snap1 created If i try to commit the snapshot i get this error: virsh # blockcommit $DOMAIN vda --active --verbose --pivot error: internal error: unable to execute QEMU command 'block-commit': Device 'drive-virtio-disk0' is busy: block device is in use by block job: commit virsh # blockjob $DOMAIN /var/lib/libvirt/images/snap1.qcow2 Block Commit: [100 %] virsh # $DOMAIN var/lib/libvirt/images/snap1.qcow2 --abort virsh # blockjob $DOMAIN /var/lib/libvirt/images/snap1.qcow2 No current block job for /var/lib/libvirt/images/snap1.qcow2 I´ve test this with qemu 2.1 and the problem does`nt exist. /usr/bin/qemu-system-x86_64 -name $DOMAIN -S -machine pc- i440fx-1.6,accel=kvm,usb=off -m 8192 -realtime mlock=off -smp 8,sockets=8,cores=1,threads=1 -uuid 30c49f37-6e62-49f6-a9df-ad2fef1fa312 -no-user-config -nodefaults -chardev socket,id=charmonitor,path=/var/lib/libvirt/qemu/$DOMAIN.monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=control -rtc base=localtime -no-shutdown -boot strict=on -device piix3-usb- uhci,id=usb,bus=pci.0,addr=0x1.0x2 -device virtio-serial-pci,id=virtio- serial0,bus=pci.0,addr=0x6 -drive file=/var/lib/libvirt/images/$DOMAIN.qcow2,if=none,id=drive-virtio- disk0,format=qcow2 -device virtio-blk- pci,scsi=off,bus=pci.0,addr=0x2,drive=drive-virtio-disk0,id=virtio- disk0,bootindex=1 -chardev pty,id=charserial0 -device isa- serial,chardev=charserial0,id=serial0 -chardev socket,id=charchannel0,path=/var/lib/libvirt/qemu/f16x86_64.agent,server,nowait -device virtserialport,bus=virtio- serial0.0,nr=1,chardev=charchannel0,id=channel0,name=org.qemu.guest_agent.0 -device usb-tablet,id=input0 -vnc 127.0.0.1:0 -k de -device VGA,id=video0,vgamem_mb=16,bus=pci.0,addr=0x5 -device virtio-balloon- pci,id=balloon0,bus=pci.0,addr=0x4 -msg timestamp=on Compiled against library: libvirt 1.2.11 Using library: libvirt 1.2.11 Using API: QEMU 1.2.11 Running hypervisor: QEMU 2.1.2 virsh # snapshot-create-as --domain windows.trohde-snapshot-test windows.trohde-snapshot-test_snap1 --diskspec vda,file=/var/lib/libvirt/images/windows.trohde-snapshot-test_snap1.qcow2 --quiesce --disk-only --atomic --no-metadata Domain snapshot windows.trohde-snapshot-test_snap1 created virsh # blockcommit $DOMAIN vda --active --verbose --pivot Block Commit: [100 %] Successfully pivoted Cheers Tim ** Affects: qemu Importance: Undecided Status: New ** Tags: blockcomit qemu snapshot -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1455475 Title: Block Commit: [100 %]error: failed to pivot job for disk vda Status in QEMU: New Bug description: Hi, i´ve a Problem with committing a snapshot. The problem was discussed on the libvirt mailing list earlier this year. https://www.redhat.com/archive
