Re: [Qemu-devel] [PATCH for-2.9 4/5] rbd: Peel off redundant RbdAuthMethod wrapper struct
Adding Daniel Berrange for QCryptoSecret expertise. Jeff Cody writes: > On Thu, Mar 23, 2017 at 04:56:30PM -0500, Eric Blake wrote: >> On 03/23/2017 04:43 PM, Eric Blake wrote: >> >> > We'd still have to allow libvirt's use of >> > ":key=...:auth_supported=cephx\\;none" on the command line, but from the >> > QMP side, we would support exactly one auth method, and libvirt will be >> > updated to match when it starts targetting blockdev-add instead of >> > legacy command line. >> > >> > If librados really needs 'cephx;none' any time cephx authorization is >> > requested, then even though the QMP only requests 'cephx', we can still >> > map it to 'cephx;none' in qemu - but I'm hoping that setting >> > auth_supported=cephx rather than auth_supported=cephx;none on the >> > librados side gives us what we (and libvirt) really want in the first >> > place. >> >> Or, if it becomes something that libvirt wants to allow users to tune in >> their XML (right now, users don't get a choice, they get either 'none' >> or 'cephx;none'), a forward-compatible solution under my QMP proposal >> would be to have qemu add a third enum state, "none", "cephx", and >> "cephx-no-fallback". >> >> On the other hand, if supporting multiple methods at once makes sense >> (say librados adds a 'cephy' method, and that users could legitimately >> use both 'cephx;cephy' and 'cephy;cephx' with different behaviors), then >> keeping auth as an array of instances of a simple flat union scales >> nicer than having to add a combinatorial explosion of branches to the >> flat union to cover every useful combination of auth_supported methods. >> Maybe I'm overthinking it. >> > > I spoke over email with Jason Dillaman (cc'ed), and this is what he told me > with regards to the authentication methods, and what cephx;none means: > > On Thu, Mar 23, 2017 at 06:04:30PM -0400, Jason Dillaman wrote: >> It's saying that the client is willing to connect to a server that >> uses cephx auth or a server that uses no auth. Looking at the code, >> the "auth_supported" configuration key is actually deprecated and >> "auth_client_required" should be used instead (which defaults to >> 'cephx, none' already). Since it's been deprecated since v0.55 and if >> you are cleaning things up, feel free to switch to the new one if >> possible. > > So from what Jason is saying, it seems like the conclusion we reached over > IRC is correct: it will attempt cephx but also fallback to no auth. So the client offers the server a list of authentication methods with credentials, and the server picks one it likes. Makes sense to me. Inluding "none" in the default less so, but that's of no concern to the QMP interface, so let's ignore it for now. > Also, since the preferred auth option may be named different depending on > ceph versions, I know think we probably should not tie the QAPI parameter to > the name of the older deprecated option. Yes. > I suggest that the 'auth_supported' parameter for QAPI be renamed to > 'auth_method'. If you don't like the array and the flexibility it provides > at the cost of complexity, I think a flat enum consisting of 3 values like > you mentioned is reasonable. Since the QAPI does not need to map to the > legacy commandline used by libvirt, I would suggest maybe naming them > slightly different, though: any, none, strict > > For 2.9, they could each map to 'auth_supported' like so: > > any: "cephx;none" > none:"none" > strict: "cephx" > > For 2.10, we could try to discover if 'auth_client_required' is supported or > not, and use either it or 'auth_supported' as appropriate (with the same > mappings as above). > > The reason I like "any" and "strict", is it gives consistent meanings to > options even if new auth methods are introduced. For a hypothetical "cephy" > method example: > > any: "cephy;cephx;none" > none:"none" > strict: "cephy;cephx" Two years later, an unfixable cryptograhic weakness in cephx is discovered. Some users really, really want to select only "cephy", but they can't. We react by pushing out a QEMU update adding method "stricter", which expands into "cephy". Libvirt then reacts and pushes out an update. And so forth, up the stack. Many moons later, users can actually select "cephy". Thanks, but no thanks. I think we should expose the current, recommended way to configure authentication, in a form that is suitable for QAPI/QMP, i.e. structured data as JSON objects, not strings you need to parse. If a future authentication method might need something else than a QCryptoSecret object, we need to take Eric's proposal and make this an array of unions, where the union tag is the method, and the variant members supply whatever data the method needs (none: nothing, cephx: a QCryptoSecret). Member @password-secret goes away. Else, we can make it an array of methods, and keep member @password-secret. We need to decide quickly to keep rbd fully supported in 2.9.
Re: [Qemu-devel] Guest application reading from pl011 without device driver
Hi, here are the patch files for char-pipe.c, char-win.c, char-win.h On 23 March 2017 at 18:31, Paolo Bonzini wrote: > > > On 23/03/2017 18:28, Jiahuan Zhang wrote: > > Hi, the method doesn't work for pipe. It still causes the same issue. > > The only difference is that the first byte in the UART fifo can be read > > by the guest app. > > So now my guest app can immediately read 17bytes when the host is > > sending data continuously, > > then guest app is unable to read. > > > > Now there is a solution-to-be. Set the qemu pipe chardev to wait for 1 > > millisecond before do a next ReadFile(), > > then the pl011 can deliver the complete large data by the 16-byte fifo > > continuously to the guest app. > > > > Is it rational? Or is this host -CPU-dependent? > > Post the patch, maybe you have a bug. > > Paolo > char-pipe.patch Description: Binary data char-win.h.patch Description: Binary data char-win.patch Description: Binary data
Re: [Qemu-devel] [PATCH v4 01/16] s390: cio: introduce cio_cancel_halt_clear
* Sebastian Ott [2017-03-23 12:51:40 +0100]: > On Fri, 17 Mar 2017, Dong Jia Shi wrote: > > For future code reuse purpose, this decouples the cio code with > > the ccw device specific parts from ccw_device_cancel_halt_clear, > > and makes a new common I/O interface named cio_cancel_halt_clear. > > > > Reviewed-by: Pierre Morel > > Signed-off-by: Dong Jia Shi > > Cc: Sebastian Ott > > Cc: Peter Oberparleiter > [...] > > +/** > > + * cio_cancel_halt_clear - Cancel running I/O by performing cancel, halt > > + * and clear ordinally if subchannel is valid. > > + * @sch: subchannel on which to perform the cancel_halt_clear operation > > + * @iretry: the number of the times remained to retry the next operation > > + * > > + * This should be called repeatedly since halt/clear are asynchronous > > + * operations. We do one try with cio_cancel, two tries with cio_halt, > ^ > three Ok. Nice catch! > > Acked-by: Sebastian Ott Thanks. -- Dong Jia Shi
[Qemu-devel] [Bug 1490611] Re: Using qemu >=2.2.1 to convert raw->VHD (fixed) adds extra padding to the result file, which Microsoft Azure rejects as invalid
Verified Proposed:
dd if=/dev/zero of=source-disk.img bs=1M count=4096
qemu-img convert -f raw -o subformat=fixed -O vpc source-disk.img
dest-disk-old.vhd
#upgrade
$qemu-img convert -f raw -o subformat=fixed -O vpc source-disk.img
dest-disk-new.vhd
$qemu-img convert -f raw -o subformat=fixed,force_size -O vpc source-disk.img
dest-disk-new-forced.vhd
#check alignment:
$ stat dest-disk-old.vhd dest-disk-new.vhd dest-disk-new-forced.vhd | awk '/^
*Size:/ {print ($2-512)/1024/1024}'
4096.48
4096.48
4096
That means:
1. without adding the new parm no behaviour change (good since it is SRU)
2. with the force_size parm the size is aligned
That is enough for verification, but it would be even greater if that
could also be tested on real azure.
** Tags removed: verification-needed
** Tags added: verification-done
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1490611
Title:
Using qemu >=2.2.1 to convert raw->VHD (fixed) adds extra padding to
the result file, which Microsoft Azure rejects as invalid
Status in QEMU:
Fix Released
Status in qemu package in Ubuntu:
Fix Released
Status in qemu source package in Xenial:
Fix Committed
Bug description:
[Impact]
* Starting with a raw disk image, using "qemu-img convert" to convert
from raw to VHD results in the output VHD file's virtual size being
aligned to the nearest 516096 bytes (16 heads x 63 sectors per head x
512 bytes per sector), instead of preserving the input file's size as
the output VHD's virtual disk size.
* Microsoft Azure requires that disk images (VHDs) submitted for
upload have virtual sizes aligned to a megabyte boundary. (Ex. 4096MB,
4097MB, 4098MB, etc. are OK, 4096.5MB is rejected with an error.) This
is reflected in Microsoft's documentation: https://azure.microsoft.com
/en-us/documentation/articles/virtual-machines-linux-create-upload-
vhd-generic/
* The fix for this bug is a backport from upstream.
http://git.qemu.org/?p=qemu.git;a=commitdiff;h=fb9245c2610932d33ce14
[Test Case]
* This is reproducible with the following set of commands (including
the Azure command line tools from https://github.com/Azure/azure-
xplat-cli). For the following example, I used qemu version 2.2.1:
$ dd if=/dev/zero of=source-disk.img bs=1M count=4096
$ stat source-disk.img
File: ‘source-disk.img’
Size: 4294967296 Blocks: 798656 IO Block: 4096 regular file
Device: fc01h/64513dInode: 13247963Links: 1
Access: (0644/-rw-r--r--) Uid: ( 1000/ smkent) Gid: ( 1000/ smkent)
Access: 2015-08-18 09:48:02.613988480 -0700
Modify: 2015-08-18 09:48:02.825985646 -0700
Change: 2015-08-18 09:48:02.825985646 -0700
Birth: -
$ qemu-img convert -f raw -o subformat=fixed -O vpc source-disk.img
dest-disk.vhd
$ stat dest-disk.vhd
File: ‘dest-disk.vhd’
Size: 4296499712 Blocks: 535216 IO Block: 4096 regular file
Device: fc01h/64513dInode: 13247964Links: 1
Access: (0644/-rw-r--r--) Uid: ( 1000/ smkent) Gid: ( 1000/ smkent)
Access: 2015-08-18 09:50:22.252077624 -0700
Modify: 2015-08-18 09:49:24.424868868 -0700
Change: 2015-08-18 09:49:24.424868868 -0700
Birth: -
$ azure vm image create testimage1 dest-disk.vhd -o linux -l "West US"
info:Executing command vm image create
+ Retrieving storage accounts
info:VHD size : 4097 MB
info:Uploading 4195800.5 KB
Requested:100.0% Completed:100.0% Running: 0 Time: 1m 0s Speed: 6744 KB/s
info:https://[redacted].blob.core.windows.net/vm-images/dest-disk.vhd was
uploaded successfully
error: The VHD
https://[redacted].blob.core.windows.net/vm-images/dest-disk.vhd has an
unsupported virtual size of 4296499200 bytes. The size must be a whole number
(in MBs).
info:Error information has been recorded to /home/smkent/.azure/azure.err
error: vm image create command failed
* A fixed qemu-img will not result in an error during azure image
creation. It will require passing -o force_size, which will leverage
the backported functionality.
[Regression Potential]
* The upstream fix introduces a qemu-img option (-o force_size) which
is unset by default. The regression potential is very low, as a
result.
...
I also ran the above commands using qemu 2.4.0, which resulted in the
same error as the conversion behavior is the same.
However, qemu 2.1.1 and earlier (including qemu 2.0.0 installed by
Ubuntu 14.04) does not pad the virtual disk size during conversion.
Using qemu-img convert from qemu versions <=2.1.1 results in a VHD
that is exactly the size of the raw input file plus 512 bytes (for the
VHD footer). Those qemu versions do not attempt to realign the disk.
As a result, Azure accepts VHD files created using those versions of
qemu-img convert for upload.
Is there a reason why newer qemu realigns the converted VHD file? It
Re: [Qemu-devel] [Bug 1675108] Re: Cocoa UI always crashes on startup
Brendan Shanks writes: > Tested on 10.12.3, it doesn't crash immediately (like before) but > crashes reliably once I send some keyboard input to qemu: > > $ i386-softmmu/qemu-system-i386 > ** > ERROR:/Users/pip/no_backup/qemu/translate-common.c:34:tcg_handle_interrupt: > assertion failed: (qemu_mutex_iothread_locked()) > Abort trap: 6 Can you test with the suggested patch I posted yesterday? If we keep the update under BQL this shouldn't fail. > > > Thread 0 Crashed:: Dispatch queue: com.apple.main-thread > 0 libsystem_kernel.dylib0x7fffa746edd6 __pthread_kill + 10 > 1 libsystem_pthread.dylib 0x7fffa755a787 pthread_kill + 90 > 2 libsystem_c.dylib 0x7fffa73d4420 abort + 129 > 3 libglib-2.0.0.dylib 0x0001076aec86 g_assertion_message > + 388 > 4 libglib-2.0.0.dylib 0x0001076aece4 > g_assertion_message_expr + 94 > 5 qemu-system-i386 0x0001066bb1ec tcg_handle_interrupt > + 156 (translate-common.c:50) > 6 qemu-system-i386 0x000106740dfc pic_irq_request + > 156 (pc.c:187) > 7 qemu-system-i386 0x00010673e5e4 gsi_handler + 36 > (pc.c:115) > 8 qemu-system-i386 0x00010685e97a kbd_update_kbd_irq + > 138 (pckbd.c:180) > 9 qemu-system-i386 0x00010694164a > qemu_input_event_send_impl + 938 (input.c:328) > 10 qemu-system-i386 0x00010694188f > qemu_input_event_send_key + 239 (input.c:359) > 11 qemu-system-i386 0x000106946a00 cocoa_refresh + 272 > (cocoa.m:1402) > 12 qemu-system-i386 0x00010693f6a8 gui_update + 104 > (console.c:1603) -- Alex Bennée
Re: [Qemu-devel] [RFC 1/4] [UNTESTED] xen: Don't force has_dynamic_sysbus on machine class
On 23/03/17 22:28, Eduardo Habkost wrote:
> The xen-backend devices created by the Xen code are not supposed
> to be treated as dynamic sysbus devices. This is an attempt to
> change that and see what happens, but I couldn't test it because
> I don't have a Xen host set up.
>
> If this patch breaks anything, this means we have a bug in
> foreach_dynamic_sysbus_device(), which is supposed to return only
> devices created using -device.
>
> The original code that sets has_dynamic_sysbus was added by
> commit 3a6c9172ac5951e6dac2b3f6cbce3cfccdec5894, but I don't see
> any comment explaining why it was necessary.
xen-backend devices are created via qmp commands when attaching new
pv-devices to a domain. They can be dynamically removed, too. Setting
has_dynamic_sysbus was necessary to support this feature.
So just removing it will break Xen.
NAK as a standalone patch.
Juergen
>
> Cc: Juergen Gross
> Cc: Stefano Stabellini
> Signed-off-by: Eduardo Habkost
> ---
> hw/xen/xen_backend.c | 11 ---
> 1 file changed, 11 deletions(-)
>
> diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c
> index 6c21c37d68..4607d6d3ee 100644
> --- a/hw/xen/xen_backend.c
> +++ b/hw/xen/xen_backend.c
> @@ -550,15 +550,6 @@ err:
> return -1;
> }
>
> -static void xen_set_dynamic_sysbus(void)
> -{
> -Object *machine = qdev_get_machine();
> -ObjectClass *oc = object_get_class(machine);
> -MachineClass *mc = MACHINE_CLASS(oc);
> -
> -mc->has_dynamic_sysbus = true;
> -}
> -
> int xen_be_register(const char *type, struct XenDevOps *ops)
> {
> char path[50];
> @@ -580,8 +571,6 @@ int xen_be_register(const char *type, struct XenDevOps
> *ops)
>
> void xen_be_register_common(void)
> {
> -xen_set_dynamic_sysbus();
> -
> xen_be_register("console", &xen_console_ops);
> xen_be_register("vkbd", &xen_kbdmouse_ops);
> xen_be_register("qdisk", &xen_blkdev_ops);
>
Re: [Qemu-devel] [PATCH for-2.9 3/5] rbd: Rewrite the code to extract list-valued options
Markus Armbruster writes:
> Kevin Wolf writes:
>
>> Am 23.03.2017 um 11:55 hat Markus Armbruster geschrieben:
>>> We have two list-values options:
>>>
>>> * "server" is a list of InetSocketAddress. We use members "host" and
>>> "port", and silently ignore the rest.
>>>
>>> * "auth-supported" is a list of RbdAuthMethod. We use its only member
>>> "auth".
>>>
>>> Since qemu_rbd_open() takes options as a flattened QDict, options has
>>> keys of the form server.%d.host, server.%d.port and
>>> auth-supported.%d.auth, where %d counts up from zero.
>>>
>>> qemu_rbd_array_opts() extracts these values as follows. First, it
>>> calls qdict_array_entries() to find the list's length. For each list
>>> element, it first formats the list's key prefix (e.g. "server.0."),
>>> then creates a new QDict holding the options with that key prefix,
>>> then converts that to a QemuOpts, so it can finally get the member
>>> values from there.
>>>
>>> If there's one surefire way to make code using QDict more awkward,
>>> it's creating more of them and mixing in QemuOpts for good measure.
>>>
>>> The conversion to QemuOpts abuses runtime_opts, as described in the
>>> commit before previous.
>>>
>>> Rewrite to simply get the values straight from the options QDict.
>>> This removes the abuse of runtime_opts, so clean it up.
>>>
>>> Signed-off-by: Markus Armbruster
>>
>>> @@ -577,91 +557,59 @@ static void qemu_rbd_complete_aio(RADOSCB *rcb)
>>> qemu_aio_unref(acb);
>>> }
>>>
>>> -#define RBD_MON_HOST 0
>>> -#define RBD_AUTH_SUPPORTED1
>>> -
>>> -static char *qemu_rbd_array_opts(QDict *options, const char *prefix, int
>>> type,
>>> - Error **errp)
>>> +static char *rbd_auth(QDict *options)
>>> {
>>> -int num_entries;
>>> -QemuOpts *opts = NULL;
>>> -QDict *sub_options;
>>> -const char *host;
>>> -const char *port;
>>> -char *str;
>>> -char *rados_str = NULL;
>>> -Error *local_err = NULL;
>>> +const char **vals = g_new(const char *, qdict_size(options));
>>> +char keybuf[32];
>>> +QObject *val;
>>> +char *rados_str;
>>> int i;
>>>
>>> -assert(type == RBD_MON_HOST || type == RBD_AUTH_SUPPORTED);
>>> -
>>> -num_entries = qdict_array_entries(options, prefix);
>>> +for (i = 0;; i++) {
>>> +sprintf(keybuf, "auth-supported.%d.auth", i);
>>> +val = qdict_get(options, keybuf);
>>> +if (!val) {
>>> +break;
>>> +}
>>>
>>> -if (num_entries < 0) {
>>> -error_setg(errp, "Parse error on RBD QDict array");
>>> -return NULL;
>>> +vals[i] = qstring_get_str(qobject_to_qstring(val));
>>> }
>>> +vals[i] = NULL;
>>
>> In case of doubt, i is one more than vals can hold. (It segfaulted for
>> me when options was empty because I passed only options that are removed
>> before this function is called.)
>
> Yes, the g_new() above needs one extra slot.
>
>> You also want to remove the options from the QDict, otherwise
>> bdrv_open_inherit() will complain that the options are unknown.
>
> Okay.
>
>>>
>>> -for (i = 0; i < num_entries; i++) {
>>> -char *strbuf = NULL;
>>> -const char *value;
>>> -char *rados_str_tmp;
>>> -
>>> -str = g_strdup_printf("%s%d.", prefix, i);
>>> -qdict_extract_subqdict(options, &sub_options, str);
>>> -g_free(str);
>>> -
>>> -opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
>>> -qemu_opts_absorb_qdict(opts, sub_options, &local_err);
>>> -QDECREF(sub_options);
>>> -if (local_err) {
>>> -error_propagate(errp, local_err);
>>> -g_free(rados_str);
>>> -rados_str = NULL;
>>> -goto exit;
>>> -}
>>> +rados_str = g_strjoinv(";", (char **)vals);
>>> +g_free(vals);
>>> +return rados_str;
>>> +}
>>>
>>> -if (type == RBD_MON_HOST) {
>>> -host = qemu_opt_get(opts, "host");
>>> -port = qemu_opt_get(opts, "port");
>>> +static char *rbd_mon_host(QDict *options)
>>> +{
>>> +const char **vals = g_new(const char *, qdict_size(options));
>>> +char keybuf[32];
>>> +QObject *val;
>>> +const char *host, *port;
>>> +char *rados_str;
>>> +int i;
>>>
>>> -value = host;
>>> -if (port) {
>>> -/* check for ipv6 */
>>> -if (strchr(host, ':')) {
>>> -strbuf = g_strdup_printf("[%s]:%s", host, port);
>>> -} else {
>>> -strbuf = g_strdup_printf("%s:%s", host, port);
>>> -}
>>> -value = strbuf;
>>> -} else if (strchr(host, ':')) {
>>> -strbuf = g_strdup_printf("[%s]", host);
>>> -value = strbuf;
>>> -}
>>> -} else {
>>> -value = qemu_opt_get(opts, "auth");
>>> +for (i = 0;; i++) {
>>> +sprintf(keybuf, "
Re: [Qemu-devel] [RFC 0/4] Replace has_dynamic_sysbus with device type whitelist
On 23/03/17 22:28, Eduardo Habkost wrote: > Summary > --- > > This series replaces the existing has_dynamic_sysbus flag (that > makes the machine accept every single sysbus device type on the > command-line) with a short whitelist. > > This will be helpful when implementing the new query-device-slots > command, because each machine type will include only the sysbus > devices it really supports, instead of including a catch-all > TYPE_SYS_BUS_DEVICE "slot". > > Most of the machines already have an internal whitelist > implemented using foreach_dynamic_sysbus_device(), so we just > encode the existing behavior inside the new MachineClass field. > > Caveat: q35 > --- > > The only problematic case is q35, that accepts all sysbus device > types. In this case, instead of adding TYPE_SYS_BUS_DEVICE to the > whitelist, I'm adding the existing list of existing sysbus device > types to keep compatibility. After that, we can gradually and > carefully remove unnecessary entries from the q35 whitelist. > > Issue: xen_set_dynamic_sysbus() hack > > > We still have an obstacle to get around: the callers of > qdev_set_id() outside qdev_device_add() are breaking > foreach_dynamic_sysbus_device(), and xen_set_dynamic_sysbus() is > a workaround for that bug. We now need to fix that bug to be able > to remove the xen_set_dynamic_sysbus() hack. This means the first > patch of this series will probably break Xen, until we fix that > bug. > > Eduardo Habkost (4): > [UNTESTED] xen: Don't force has_dynamic_sysbus on machine class > machine: Replace has_dynamic_sysbus with a whitelist > q35: Remove ioapic devices from sysbus whitelist > q35: Remove fw_cfg* from sysbus whitelist > > include/hw/arm/sysbus-fdt.h | 7 +++ > include/hw/boards.h | 3 ++- > hw/arm/sysbus-fdt.c | 10 ++ > hw/arm/virt.c | 4 +++- > hw/core/machine.c | 43 +-- > hw/i386/pc_q35.c| 25 - > hw/ppc/e500plat.c | 4 +++- > hw/ppc/spapr.c | 2 +- > hw/xen/xen_backend.c| 11 --- > 9 files changed, 79 insertions(+), 30 deletions(-) > This seems to break Xen. I got: qemu-system-i386: Option '-device xen-backend' cannot be handled by this machine when using qemu as Xen backend driver. Juergen
Re: [Qemu-devel] [PATCH 42/51] ram: Pass RAMBlock to bitmap_sync
Yang Hongyang wrote:
> On 2017/3/24 4:45, Juan Quintela wrote:
>> We change the meaning of start to be the offset from the beggining of
>> the block.
>>
>> @@ -701,7 +701,7 @@ static void migration_bitmap_sync(RAMState *rs)
>> qemu_mutex_lock(&rs->bitmap_mutex);
>> rcu_read_lock();
>> QLIST_FOREACH_RCU(block, &ram_list.blocks, next) {
>> -migration_bitmap_sync_range(rs, block->offset, block->used_length);
>> +migration_bitmap_sync_range(rs, block, 0, block->used_length);
>
> Since RAMBlock been passed to bitmap_sync, could we remove
> param 'block->used_length' either?
Hi
good catch.
I had that removed, and then realized that I want to synchronize parts
of the bitmap, not the whole one. That part of the series is still not
done.
Right now we do something like (I have simplified a lot of details):
while(true) {
foreach(block)
bitmap_sync(block)
foreach(page)
if(dirty(page))
page_send(page)
}
If you have several terabytes of RAM that is too ineficient, because
when we arrive to the page_send(page), it is possible that it is already
dirty again, and we have to send it twice. So, the idea is to change to
something like:
while(true) {
foreach(block)
bitmap_sync(block)
foreach(block)
foreach(64pages)
bitmap_sync(64pages)
foreach(page of the 64)
if (dirty)
page_send(page)
}
Where 64 is a magic number, I have to test what is the good value.
Basically it should be a multiple of sizeof(long) and a multiple/divisor
of host page size.
Reason of changing the for to be for each block, is that then we can
easily put bitmaps by hostpage size, instead of having to had it for
target page size.
Thanks for the review, Juan.
Later, Juan.
Re: [Qemu-devel] [RFC] Split migration bitmaps by ramblock
Yang Hongyang wrote: > Hi Juan, > > First of all, I like the refactor patchset about RAMState, it makes > things clean, great! Thanks. The whole idea of the series was to make testing changes easier. > On 2017/3/24 5:01, Juan Quintela wrote: >> Hi >> >> This series split the migration and unsent bitmaps by ramblock. This >> makes it easier to synchronize in small bits. This is on top of the >> RAMState and not-hotplug series. >> >> Why? >> >> reason 1: >> >> People have complained that by the time that we detect that a page is >> sent, it has already been marked dirty "again" inside kvm, so we are >> going to send it again. On top of this patch, my idea is, for words >> of the bitmap that have any bit set, just synchonize the bitmap before >> sending the pages. I have not looking into performance numbers yet, >> jsut asking for comments about how it is done. > > Here you said 'synchonize the bitmap before sending the pages', do you > mean synchronize the bitmap from kvm? If so, I doubt the performance... > because every synchronization will require a ioctl(). If not, the > synchronization of per block is useless. > > Currently, migration thread will synchronize the bitmap from kvm every > iter(migration_bitmap_sync()). The RAMBlock already has kind of per block > bitmap for this kind of sync. And the migration bitmap is used to put all > those per block bitmap together for data sending use. Hi For huge memory machines, we are doing it always in one go. bitmap_sync(1TB RAM) walk bitmap for 512MB of RAM, at that point, it is very probable that this page is again dirty in the KVM bitmap, so, we send it, but as it is dirty again, we would have to send it in the next pass. This sent is completely useless. So my idea is to split things in smaller chunks. As we have to do an ioctl, we wouldn't want to synchronize page by page, but perhaps 16MB at a time, 64MB, anything less than the whole amount of memory. Later, Juan.
Re: [Qemu-devel] [PATCH v2] xen-platform: separate unplugging of NVMe disks
> -Original Message-
> From: Stefano Stabellini [mailto:sstabell...@kernel.org]
> Sent: 24 March 2017 00:51
> To: Paul Durrant
> Cc: qemu-devel@nongnu.org; xen-de...@lists.xenproject.org; Stefano
> Stabellini ; Anthony Perard
>
> Subject: Re: [PATCH v2] xen-platform: separate unplugging of NVMe disks
>
> On Thu, 23 Mar 2017, Paul Durrant wrote:
> > Commit 090fa1c8 "add support for unplugging NVMe disks..." extended the
> > existing disk unplug flag to cover NVMe disks as well as IDE and SCSI.
> >
> > The recent thread on the xen-devel mailing list [1] has highlighted that
> > this is not desirable behaviour: PV frontends should be able to distinguish
> > NVMe disks from other types of disk and should have separate control
> over
> > whether they are unplugged.
> >
> > This patch defines a new bit in the unplug mask for this purpose and also
> > tidies up the definitions of, and improves the comments regarding, the
> > previously exiting bits in the protocol.
> >
> > [1] https://lists.xen.org/archives/html/xen-devel/2017-03/msg02924.html
> >
> > Signed-off-by: Paul Durrant
> > --
> > Cc: Stefano Stabellini
> > Cc: Anthony Perard
> >
> > NOTE: A companion patch will be submitted to xen-devel to align the
> > unplug protocol documentation once this patch is acked.
>
> The companion patch needs to be acked before this patch gets applied. In
> fact, I would prefer if the changeset of the Xen commit was added to
> this patch description.
>
> If you add that, then you can repost this with
>
> Reviewed-by: Stefano Stabellini
>
Ok. I'll cc you on the xen docs patch too.
Paul
>
> > v2:
> > - Fix the commit comment
> > ---
> > hw/i386/xen/xen_platform.c | 47
> ++
> > 1 file changed, 35 insertions(+), 12 deletions(-)
> >
> > diff --git a/hw/i386/xen/xen_platform.c b/hw/i386/xen/xen_platform.c
> > index 6010f35..983d532 100644
> > --- a/hw/i386/xen/xen_platform.c
> > +++ b/hw/i386/xen/xen_platform.c
> > @@ -87,10 +87,30 @@ static void log_writeb(PCIXenPlatformState *s, char
> val)
> > }
> > }
> >
> > -/* Xen Platform, Fixed IOPort */
> > -#define UNPLUG_ALL_DISKS 1
> > -#define UNPLUG_ALL_NICS 2
> > -#define UNPLUG_AUX_IDE_DISKS 4
> > +/*
> > + * Unplug device flags.
> > + *
> > + * The logic got a little confused at some point in the past but this is
> > + * what they do now.
> > + *
> > + * bit 0: Unplug all IDE and SCSI disks.
> > + * bit 1: Unplug all NICs.
> > + * bit 2: Unplug IDE disks except primary master. This is overridden if
> > + *bit 0 is also present in the mask.
> > + * bit 3: Unplug all NVMe disks.
> > + *
> > + */
> > +#define _UNPLUG_IDE_SCSI_DISKS 0
> > +#define UNPLUG_IDE_SCSI_DISKS (1u << _UNPLUG_IDE_SCSI_DISKS)
> > +
> > +#define _UNPLUG_ALL_NICS 1
> > +#define UNPLUG_ALL_NICS (1u << _UNPLUG_ALL_NICS)
> > +
> > +#define _UNPLUG_AUX_IDE_DISKS 2
> > +#define UNPLUG_AUX_IDE_DISKS (1u << _UNPLUG_AUX_IDE_DISKS)
> > +
> > +#define _UNPLUG_NVME_DISKS 3
> > +#define UNPLUG_NVME_DISKS (1u << _UNPLUG_NVME_DISKS)
> >
> > static void unplug_nic(PCIBus *b, PCIDevice *d, void *o)
> > {
> > @@ -111,7 +131,7 @@ static void unplug_disks(PCIBus *b, PCIDevice *d,
> void *opaque)
> > {
> > uint32_t flags = *(uint32_t *)opaque;
> > bool aux = (flags & UNPLUG_AUX_IDE_DISKS) &&
> > -!(flags & UNPLUG_ALL_DISKS);
> > +!(flags & UNPLUG_IDE_SCSI_DISKS);
> >
> > /* We have to ignore passthrough devices */
> > if (!strcmp(d->name, "xen-pci-passthrough")) {
> > @@ -124,12 +144,16 @@ static void unplug_disks(PCIBus *b, PCIDevice *d,
> void *opaque)
> > break;
> >
> > case PCI_CLASS_STORAGE_SCSI:
> > -case PCI_CLASS_STORAGE_EXPRESS:
> > if (!aux) {
> > object_unparent(OBJECT(d));
> > }
> > break;
> >
> > +case PCI_CLASS_STORAGE_EXPRESS:
> > +if (flags & UNPLUG_NVME_DISKS) {
> > +object_unparent(OBJECT(d));
> > +}
> > +
> > default:
> > break;
> > }
> > @@ -147,10 +171,9 @@ static void platform_fixed_ioport_writew(void
> *opaque, uint32_t addr, uint32_t v
> > switch (addr) {
> > case 0: {
> > PCIDevice *pci_dev = PCI_DEVICE(s);
> > -/* Unplug devices. Value is a bitmask of which devices to
> > - unplug, with bit 0 the disk devices, bit 1 the network
> > - devices, and bit 2 the non-primary-master IDE devices. */
> > -if (val & (UNPLUG_ALL_DISKS | UNPLUG_AUX_IDE_DISKS)) {
> > +/* Unplug devices. See comment above flag definitions */
> > +if (val & (UNPLUG_IDE_SCSI_DISKS | UNPLUG_AUX_IDE_DISKS |
> > + UNPLUG_NVME_DISKS)) {
> > DPRINTF("unplug disks\n");
> > pci_unplug_disks(pci_dev->bus, val);
> > }
> > @@ -338,14 +361,14 @@ static void xen_platform_ioport_writeb(void
> *opaque, hwaddr addr,
> > * If VMDP was to control both disk and LAN it would use 4.
> >
Re: [Qemu-devel] [PATCH 42/51] ram: Pass RAMBlock to bitmap_sync
Hi Juan,
On 2017/3/24 16:29, Juan Quintela wrote:
> Yang Hongyang wrote:
>> On 2017/3/24 4:45, Juan Quintela wrote:
>>> We change the meaning of start to be the offset from the beggining of
>>> the block.
>>>
>>> @@ -701,7 +701,7 @@ static void migration_bitmap_sync(RAMState *rs)
>>> qemu_mutex_lock(&rs->bitmap_mutex);
>>> rcu_read_lock();
>>> QLIST_FOREACH_RCU(block, &ram_list.blocks, next) {
>>> -migration_bitmap_sync_range(rs, block->offset, block->used_length);
>>> +migration_bitmap_sync_range(rs, block, 0, block->used_length);
>>
>> Since RAMBlock been passed to bitmap_sync, could we remove
>> param 'block->used_length' either?
>
> Hi
>
> good catch.
>
> I had that removed, and then realized that I want to synchronize parts
> of the bitmap, not the whole one. That part of the series is still not
> done.
>
> Right now we do something like (I have simplified a lot of details):
>
> while(true) {
> foreach(block)
> bitmap_sync(block)
> foreach(page)
> if(dirty(page))
>page_send(page)
> }
>
>
> If you have several terabytes of RAM that is too ineficient, because
> when we arrive to the page_send(page), it is possible that it is already
> dirty again, and we have to send it twice. So, the idea is to change to
> something like:
>
> while(true) {
> foreach(block)
> bitmap_sync(block)
Do you mean sync with KVM here?
> foreach(block)
> foreach(64pages)
> bitmap_sync(64pages)
Then here, we will sync with KVM too. For huge MEM,
it will generates lots of ioctl()...
Bitmap in KVM is per Memory region IIRC. KVM module currently
haven't the ability to sync parts of the bitmap. A sync have
to sync the whole mr. So if we want to do small sync, we might
need to modify KVM also, but that still won't solve the preblem
of increased ioctls.
> foreach(page of the 64)
>if (dirty)
> page_send(page)
> }
>
>
> Where 64 is a magic number, I have to test what is the good value.
> Basically it should be a multiple of sizeof(long) and a multiple/divisor
> of host page size.
>
> Reason of changing the for to be for each block, is that then we can
> easily put bitmaps by hostpage size, instead of having to had it for
> target page size.
>
> Thanks for the review, Juan.
>
> Later, Juan.
>
--
Thanks,
Yang
Re: [Qemu-devel] [RFC] Split migration bitmaps by ramblock
On 2017/3/24 16:34, Juan Quintela wrote: > Yang Hongyang wrote: >> Hi Juan, >> >> First of all, I like the refactor patchset about RAMState, it makes >> things clean, great! > > Thanks. > > The whole idea of the series was to make testing changes easier. > >> On 2017/3/24 5:01, Juan Quintela wrote: >>> Hi >>> >>> This series split the migration and unsent bitmaps by ramblock. This >>> makes it easier to synchronize in small bits. This is on top of the >>> RAMState and not-hotplug series. >>> >>> Why? >>> >>> reason 1: >>> >>> People have complained that by the time that we detect that a page is >>> sent, it has already been marked dirty "again" inside kvm, so we are >>> going to send it again. On top of this patch, my idea is, for words >>> of the bitmap that have any bit set, just synchonize the bitmap before >>> sending the pages. I have not looking into performance numbers yet, >>> jsut asking for comments about how it is done. >> >> Here you said 'synchonize the bitmap before sending the pages', do you >> mean synchronize the bitmap from kvm? If so, I doubt the performance... >> because every synchronization will require a ioctl(). If not, the >> synchronization of per block is useless. >> >> Currently, migration thread will synchronize the bitmap from kvm every >> iter(migration_bitmap_sync()). The RAMBlock already has kind of per block >> bitmap for this kind of sync. And the migration bitmap is used to put all >> those per block bitmap together for data sending use. > > Hi > For huge memory machines, we are doing it always in one go. > > > bitmap_sync(1TB RAM) > walk bitmap for 512MB of RAM, at that point, it is very probable that > this page is again dirty in the KVM bitmap, so, we send it, but as it is > dirty again, we would have to send it in the next pass. This sent is > completely useless. I got your point, the problem is KVM do not have the ability to sync in small chunks currently, even it has, it will generate lots of ioctls: KVM bitmap sync is per Memory Region IIRC, think we have a 1T mem guest, 16 MRs for example, currently, every iter we need to do 16 ioctls. But if we sync in small chunks, 64M for example, we might need to do 16384 ioctls in the worst case. eg:mem press(dirty rate) is very high. > > So my idea is to split things in smaller chunks. As we have to do an > ioctl, we wouldn't want to synchronize page by page, but perhaps 16MB at > a time, 64MB, anything less than the whole amount of memory. > > Later, Juan. > -- Thanks, Yang
[Qemu-devel] [PATCH] target/s390x/kvm: Fix problem when running with SELinux under z/VM
When running QEMU with KVM under z/VM, the memory for the guest
is allocated via legacy_s390_alloc() since the KVM_CAP_S390_COW
extension is not supported on z/VM. legacy_s390_alloc() then uses
mmap(... PROT_EXEC ...) for the guest memory - but this does not
work when running with SELinux enabled, mmap() fails and QEMU aborts
with the following error message:
cannot set up guest memory 's390.ram': Permission denied
Looking at the other allocator function qemu_anon_ram_alloc(), it
seems like PROT_EXEC is normally not needed for allocating the
guest RAM, and indeed, the guest also starts successfully under
z/VM when we remove the PROT_EXEC from the legacy_s390_alloc()
function. So let's get rid of that flag here to be able to run
with SELinux under z/VM, too.
Signed-off-by: Thomas Huth
---
target/s390x/kvm.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index ac47154..5167436 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -678,8 +678,7 @@ static void *legacy_s390_alloc(size_t size, uint64_t *align)
{
void *mem;
-mem = mmap((void *) 0x8ULL, size,
- PROT_EXEC|PROT_READ|PROT_WRITE,
+mem = mmap((void *) 0x8ULL, size, PROT_READ | PROT_WRITE,
MAP_SHARED | MAP_ANONYMOUS | MAP_FIXED, -1, 0);
return mem == MAP_FAILED ? NULL : mem;
}
--
1.8.3.1
[Qemu-devel] [Bug 1675549] Re: tcg softmmu i386 crashes on BE hardware
This is likely the same or at least a similar problem as reported in this bug here: https://bugs.launchpad.net/qemu/+bug/1675108 -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1675549 Title: tcg softmmu i386 crashes on BE hardware Status in QEMU: New Bug description: Hi, today i try to test qemu 2.9rc 1 with qemu-system-i386 if i set display as sdl and i push a key on keyboard qemu exit with an error translate-common.c:34:tcg_handle_interrupt: assertion failed: (qemu_mutex_iothread_locked()) This issue was not present on qemu 2.8.0 Test Machine PowerMac G5 Quad Fedora 25 Server PPC64 Qemu build with target-list=i386-softmuu --with-sdlabi=2.0 Ciao Luigi To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1675549/+subscriptions
Re: [Qemu-devel] [Qemu-ppc] Bug in qemu-system-ppc in Windows using the SDL2 GUI
On 24.03.2017 09:56, Mark Cave-Ayland wrote: > On 24/03/17 08:47, Howard Spoelstra wrote: > >> Hi, >> >> Running qemu-system-ppc.exe in windows with the SDL2 GUI results in a >> hangup. I bisected to this commit: >> >> 8bb93c6f99a42c2e0943bc904b283cd622d302c5 is the first bad commit >> commit 8bb93c6f99a42c2e0943bc904b283cd622d302c5 >> Author: Alex Bennée >> Date: Wed Mar 15 14:48:25 2017 + >> >> ui/console: ensure graphic updates don't race with TCG vCPUs >> >> Commit 8d04fb55.. >> >> tcg: drop global lock during TCG code execution >> >> ..broke the assumption that updates to the GUI couldn't happen at the >> same time as TCG vCPUs where running. As a result the TCG vCPU could >> still be updating a directly mapped frame-buffer while the display >> side was updating. This would cause artefacts to appear when the >> update code assumed that memory block hadn't changed. >> >> The simplest solution is to ensure the two things can't happen at the >> same time like the old BQL locking scheme. Here we use the solution >> introduced for MTTCG and schedule the update as async_safe_work when >> we know no vCPUs can be running. >> >> Reported-by: Mark Cave-Ayland >> Signed-off-by: Alex Bennée >> Message-id: 20170315144825.3108-1-alex.ben...@linaro.org >> Cc: BALATON Zoltan >> Cc: Gerd Hoffmann >> Cc: Paolo Bonzini >> Signed-off-by: Alex Bennée >> >> [ kraxel: updated comment clarifying the display adapters are buggy >> and this is a temporary workaround ] >> >> Signed-off-by: Gerd Hoffmann > > Hi Howard, > > Alex posted another patch yesterday to resolve a similar issue: > https://lists.nongnu.org/archive/html/qemu-devel/2017-03/msg04474.html. > > Does applying the above patch help at all? Luigi Burdo reported a similar issue here: https://bugs.launchpad.net/qemu/+bug/1675549 Luigi, could you please try Alex' patch to see whether it fixes your crash? Thanks, Thomas
Re: [Qemu-devel] [PULL 5/7] pci: introduce a bus master container
On 16/03/17 05:01, Michael S. Tsirkin wrote:
> From: Jason Wang
>
> 96a8821d2141 ("virtio: unbreak virtio-pci with IOMMU after caching ring
> translations") tries to make IOMMU works with virtio memory region
> cache, but it requires IOMMU to be created before any virtio
> devices. This is sub optimal, fixing this by introduce a bus master
> container to make sure address space can be initialized during device
> registering, and then we can safely set alias and make
> bus_master_enable_region as its subregion during bus master
> initialization.
>
> Cc: Paolo Bonzini
> Signed-off-by: Jason Wang
> Reviewed-by: Paolo Bonzini
> Reviewed-by: Michael S. Tsirkin
> Signed-off-by: Michael S. Tsirkin
I am not sure why exactly (friday 20:00, gotta go :) ) but this broke PCI
hot uplug on my setup which is ppc64, pseries guest, vfio-pci device.
I run QEMU with:
-device nec-usb-xhci,id=nec-usb-xhci0 \
-device "vfio-pci,id=vfio0001_01_00_2,host=0001:01:00.2"
Then:
{ "execute": "device_del", "arguments": {"id": "vfio0001_01_00_2"} }
all good. Then repeat after 30s:
{ 'error': { 'class': 'DeviceNotFound',
'desc': "Device 'vfio0001_01_00_2' not found"}}
which is expected, and then, after a minute or so:
{ 'error': { 'class': 'GenericError',
'desc': 'vfio error: 0001:01:00.2: device is already '
'attached'}}
And I can definitely tell that vfio_exitfn() has been called but
vfio_instance_finalize() has not so something is holding vfio-pci device.
Any quick ideas why or I debug on Monday? Thanks!
btw I tried sha1 0832970119.
> ---
> include/hw/pci/pci.h | 1 +
> hw/pci/pci.c | 9 +++--
> 2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index 9349acb..713ede0 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -284,6 +284,7 @@ struct PCIDevice {
> char name[64];
> PCIIORegion io_regions[PCI_NUM_REGIONS];
> AddressSpace bus_master_as;
> +MemoryRegion bus_master_container_region;
> MemoryRegion bus_master_enable_region;
>
> /* do not access the following fields */
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 273f1e4..ad46390 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -88,8 +88,8 @@ static void pci_init_bus_master(PCIDevice *pci_dev)
> OBJECT(pci_dev), "bus master",
> dma_as->root, 0,
> memory_region_size(dma_as->root));
> memory_region_set_enabled(&pci_dev->bus_master_enable_region, false);
> -address_space_init(&pci_dev->bus_master_as,
> - &pci_dev->bus_master_enable_region, pci_dev->name);
> +memory_region_add_subregion(&pci_dev->bus_master_container_region, 0,
> +&pci_dev->bus_master_enable_region);
> }
>
> static void pcibus_machine_done(Notifier *notifier, void *data)
> @@ -995,6 +995,11 @@ static PCIDevice *do_pci_register_device(PCIDevice
> *pci_dev, PCIBus *bus,
> pci_dev->devfn = devfn;
> pci_dev->requester_id_cache = pci_req_id_cache_get(pci_dev);
>
> +memory_region_init(&pci_dev->bus_master_container_region,
> OBJECT(pci_dev),
> + "bus master container", UINT64_MAX);
> +address_space_init(&pci_dev->bus_master_as,
> + &pci_dev->bus_master_container_region, pci_dev->name);
> +
> if (qdev_hotplug) {
> pci_init_bus_master(pci_dev);
> }
>
--
Alexey
Re: [Qemu-devel] [PATCH] target/s390x/kvm: Fix problem when running with SELinux under z/VM
On Fri, 24 Mar 2017 10:26:55 +0100
Thomas Huth wrote:
> When running QEMU with KVM under z/VM, the memory for the guest
> is allocated via legacy_s390_alloc() since the KVM_CAP_S390_COW
> extension is not supported on z/VM. legacy_s390_alloc() then uses
> mmap(... PROT_EXEC ...) for the guest memory - but this does not
> work when running with SELinux enabled, mmap() fails and QEMU aborts
> with the following error message:
>
> cannot set up guest memory 's390.ram': Permission denied
>
> Looking at the other allocator function qemu_anon_ram_alloc(), it
> seems like PROT_EXEC is normally not needed for allocating the
> guest RAM, and indeed, the guest also starts successfully under
> z/VM when we remove the PROT_EXEC from the legacy_s390_alloc()
> function. So let's get rid of that flag here to be able to run
> with SELinux under z/VM, too.
The root cause of this is lack of ESOP in the host.
>
> Signed-off-by: Thomas Huth
> ---
> target/s390x/kvm.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index ac47154..5167436 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -678,8 +678,7 @@ static void *legacy_s390_alloc(size_t size, uint64_t
> *align)
> {
> void *mem;
>
> -mem = mmap((void *) 0x8ULL, size,
> - PROT_EXEC|PROT_READ|PROT_WRITE,
> +mem = mmap((void *) 0x8ULL, size, PROT_READ | PROT_WRITE,
> MAP_SHARED | MAP_ANONYMOUS | MAP_FIXED, -1, 0);
> return mem == MAP_FAILED ? NULL : mem;
> }
Wouldn't it be better to adapt the SELinux rules?
Re: [Qemu-devel] [PATCH] target/s390x/kvm: Fix problem when running with SELinux under z/VM
On 03/24/2017 10:26 AM, Thomas Huth wrote:
> When running QEMU with KVM under z/VM, the memory for the guest
> is allocated via legacy_s390_alloc() since the KVM_CAP_S390_COW
> extension is not supported on z/VM. legacy_s390_alloc() then uses
> mmap(... PROT_EXEC ...) for the guest memory - but this does not
> work when running with SELinux enabled, mmap() fails and QEMU aborts
> with the following error message:
>
> cannot set up guest memory 's390.ram': Permission denied
>
> Looking at the other allocator function qemu_anon_ram_alloc(), it
> seems like PROT_EXEC is normally not needed for allocating the
> guest RAM, and indeed, the guest also starts successfully under
> z/VM when we remove the PROT_EXEC from the legacy_s390_alloc()
> function. So let's get rid of that flag here to be able to run
> with SELinux under z/VM, too.
Older z/VM versions do not provide the enhanced suppression on protection
facility, which would result in guest failures as soon as the kernel
starts dirty pages tracking by write protecting the pages via the page
table. Some kernel release back (last time I checked) the PROT_EXEC was
necessary to prevent the dirty pages tracking from taking place. So this
patch would break KVM in that case.
Newer z/VMs (e.g. 6.3) do provide ESOP. SO the question is,
why is KVM_CAP_S390_COW not set?
>
> Signed-off-by: Thomas Huth
> ---
> target/s390x/kvm.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index ac47154..5167436 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -678,8 +678,7 @@ static void *legacy_s390_alloc(size_t size, uint64_t
> *align)
> {
> void *mem;
>
> -mem = mmap((void *) 0x8ULL, size,
> - PROT_EXEC|PROT_READ|PROT_WRITE,
> +mem = mmap((void *) 0x8ULL, size, PROT_READ | PROT_WRITE,
> MAP_SHARED | MAP_ANONYMOUS | MAP_FIXED, -1, 0);
> return mem == MAP_FAILED ? NULL : mem;
> }
>
Re: [Qemu-devel] [PATCH v2] qemu-ga: add guest-get-osinfo command
> On Mar 23, 2017, at 3:58 PM, Eric Blake wrote:
>
> On 03/16/2017 09:50 AM, Vinzenz 'evilissimo' Feenstra wrote:
>> From: Vinzenz Feenstra
>>
>> Add a new 'guest-get-osinfo' command for reporting basic information of
>> the guest operating system (hereafter just 'OS'). This information
>> includes the type of the OS, the version, and the architecture.
>> Additionally reported would be a name, distribution type and kernel
>> version where applicable.
>>
>> Here an example for a Fedora 25 VM:
>>
>> $ virsh -c qemu:system qemu-agent-command F25 \
>>'{ "execute": "guest-get-osinfo" }'
>> {"return":{"arch":"x86_64","codename":"Server Edition","version":"25",
>> "kernel":"4.8.6-300.fc25.x86_64","type":"linux","distribution":"Fedora"}}
>>
>> And an example for a Windows 2012 R2 VM:
>>
>> $ virsh -c qemu:system qemu-agent-command Win2k12R2 \
>>'{ "execute": "guest-get-osinfo" }'
>> {"return":{"arch":"x86_64","codename":"Win 2012 R2",
>> "version":"6.3","kernel":"","type":"windows","distribution":""}}
>>
>> Signed-off-by: Vinzenz Feenstra
>> ---
>
> Let's make sure we get the interface right, before I even bother looking
> at the code.
Sure
>
>> +++ b/qga/qapi-schema.json
>> @@ -1042,3 +1042,43 @@
>> 'data':{ 'path': 'str', '*arg': ['str'], '*env': ['str'],
>>'*input-data': 'str', '*capture-output': 'bool' },
>> 'returns': 'GuestExec' }
>> +
>> +##
>> +# @GuestOSType:
>> +#
>> +# @linux:Indicator for linux distributions
>> +# @windows: Indicator for windows versions
>> +# @other:Indicator for any other operating system that is not yet
>> +#explicitly supported
>> +#
>> +# Since: 2.10
>> +##
>> +{ 'enum': 'GuestOSType', 'data': ['linux', 'windows', 'other'] }
>> +
>> +##
>> +# @GuestOSInfo:
>> +#
>> +# @version: OS version, e.g. 25 for FC25 etc.
>> +# @distribution: Fedora, Ubuntu, Debian, CentOS...
>> +# @codename: Code name of the OS. e.g. Ubuntu has Xenial Xerus etc.
>> +# @arch: Architecture of the OS e.g. x86, x86_64, ppc64, aarch64...
>> +# @type: Specifies the type of the OS.
>> +# @kernel: Linux kernel version (Might be used by other OS types too).
>> +#May be empty.
>
> Why would it be empty, instead of just omitted?
That’s a relict of v1 where this was still the case and all fields were
supposed to be there all the time.
>
>> +# Since: 2.10
>> +##
>> +{ 'struct': 'GuestOSInfo',
>> + 'data': { '*version': 'str', '*distribution': 'str', '*codename': 'str',
>> +'*arch': 'str', 'type': 'GuestOSType', '*kernel': 'str'} }
>
> So everything except 'type' is optional? Is there anything a client can
> actually rely on being always present?
>
> uname() is probably the lowest-common-denominator interface for
> describing a system (in that it is implemented on more OSs than anything
> else, and Windows information can be mapped to uname concepts). Let's
> compare:
>
> POSIX says:
> http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/sys_utsname.h.html#tag_13_70
>
>
>
>The header shall define the structure utsname which
> shall include at least the following members:
>
>char sysname[] Name of this implementation of the operating system.
>char nodename[] Name of this node within the communications
> network to which this node is attached, if any.
>char release[] Current release level of this implementation.
>char version[] Current version level of this release.
>char machine[] Name of the hardware type on which the system is
> running.
>
> So you both have version; your 'arch' maps somewhat to machine, your
> 'type' maps somewhat to sysname, your 'kernel' probably matches release,
> and then you are adding 'distribution' and 'codename' which don't appear
> in uname output. Is it worth naming your members after the uname()
> names, for less confusion?
OK So how about something like this:
{‘struct’: ‘GuestWindowsVersionInfo’,
‘data’: {
‘version’: ‘str’,
‘name’: ‘str’,
} }
{‘struct’: ‘GuestLinuxOSRelease’,
‘data’: {
‘id’: ‘str’,
‘name’: ‘str'
‘*version’: ‘str’,
‘*variant’: ‘str’
‘*codename’: ‘str’
} }
{‘enum’: ‘GuestOSVersionInfo’,
‘data’: {‘linux’: ‘ GuestLinuxOSRelease’,
‘win’: ‘ GuestWindowsVersionInfo’
}}
{ 'struct': 'GuestOSInfo’,
'data': {
‘machine': 'str’,
’sysname': 'GuestOSType’,
‘release': ‘str’,
‘*version_info’: ‘GuestOSVersionInfo’,
} }
>
>> +
>> +##
>> +# @guest-get-osinfo:
>> +#
>> +# Retrieve guest operating system information
>> +#
>> +# Returns: operating system information on success
>> +#
>> +# Since 2.10
>> +##
>> +{ 'command': 'guest-get-osinfo',
>> + 'returns': 'GuestOSInfo' }
>>
>
> --
> Eric Blake eblake redhat com+1-919-301-3266
> Libvirt virtualization library http://libvirt
Re: [Qemu-devel] [PATCH 1/2] migration: Disable hotplug/unplug during migration
On 23.03.2017 21:50, Juan Quintela wrote:
> Until we have reviewed what can/can't be hotplug during migration,
> disable it. We can enable it later for the things that we know that
> work. For instance, memory hotplug during postcopy don't work
> currently.
>
> Signed-off-by: Juan Quintela
> ---
> hw/core/qdev.c | 5 +
> qdev-monitor.c | 7 ++-
> 2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 1e7fb33..8c4a3f3 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -277,6 +277,11 @@ void qdev_unplug(DeviceState *dev, Error **errp)
> HotplugHandler *hotplug_ctrl;
> HotplugHandlerClass *hdc;
>
> +if (!migration_is_idle()) {
> +error_setg(errp, "device_add not allowed while migrating");
> +return;
> +}
> +
> if (dev->parent_bus && !qbus_is_hotpluggable(dev->parent_bus)) {
> error_setg(errp, QERR_BUS_NO_HOTPLUG, dev->parent_bus->name);
> return;
> diff --git a/qdev-monitor.c b/qdev-monitor.c
> index 5f2fcdf..e0622b4 100644
> --- a/qdev-monitor.c
> +++ b/qdev-monitor.c
> @@ -29,7 +29,7 @@
> #include "qemu/error-report.h"
> #include "qemu/help_option.h"
> #include "sysemu/block-backend.h"
> -
> +#include "migration/migration.h"
> /*
> * Aliases were a bad idea from the start. Let's keep them
> * from spreading further.
Maybe better keep an empty line between the include statement and the
comment?
Thomas
> @@ -566,6 +566,11 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error
> **errp)
> BusState *bus = NULL;
> Error *err = NULL;
>
> +if (!migration_is_idle()) {
> +error_setg(errp, "device_add not allowed while migrating");
> +return NULL;
> +}
> +
> driver = qemu_opt_get(opts, "driver");
> if (!driver) {
> error_setg(errp, QERR_MISSING_PARAMETER, "driver");
>
Re: [Qemu-devel] [PATCH 1/2] migration: Disable hotplug/unplug during migration
Thomas Huth wrote:
> On 23.03.2017 21:50, Juan Quintela wrote:
>> Until we have reviewed what can/can't be hotplug during migration,
>> disable it. We can enable it later for the things that we know that
>> work. For instance, memory hotplug during postcopy don't work
>> currently.
>>
>> Signed-off-by: Juan Quintela
>> ---
>> hw/core/qdev.c | 5 +
>> qdev-monitor.c | 7 ++-
>> 2 files changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
>> index 1e7fb33..8c4a3f3 100644
>> --- a/hw/core/qdev.c
>> +++ b/hw/core/qdev.c
>> @@ -277,6 +277,11 @@ void qdev_unplug(DeviceState *dev, Error **errp)
>> HotplugHandler *hotplug_ctrl;
>> HotplugHandlerClass *hdc;
>>
>> +if (!migration_is_idle()) {
>> +error_setg(errp, "device_add not allowed while migrating");
>> +return;
>> +}
>> +
>> if (dev->parent_bus && !qbus_is_hotpluggable(dev->parent_bus)) {
>> error_setg(errp, QERR_BUS_NO_HOTPLUG, dev->parent_bus->name);
>> return;
>> diff --git a/qdev-monitor.c b/qdev-monitor.c
>> index 5f2fcdf..e0622b4 100644
>> --- a/qdev-monitor.c
>> +++ b/qdev-monitor.c
>> @@ -29,7 +29,7 @@
>> #include "qemu/error-report.h"
>> #include "qemu/help_option.h"
>> #include "sysemu/block-backend.h"
>> -
>> +#include "migration/migration.h"
>> /*
>> * Aliases were a bad idea from the start. Let's keep them
>> * from spreading further.
>
> Maybe better keep an empty line between the include statement and the
> comment?
Oops, sure.
>
> Thomas
>
>
>> @@ -566,6 +566,11 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error
>> **errp)
>> BusState *bus = NULL;
>> Error *err = NULL;
>>
>> +if (!migration_is_idle()) {
>> +error_setg(errp, "device_add not allowed while migrating");
>> +return NULL;
>> +}
>> +
>> driver = qemu_opt_get(opts, "driver");
>> if (!driver) {
>> error_setg(errp, QERR_MISSING_PARAMETER, "driver");
>>
Re: [Qemu-devel] [PATCH] target/s390x/kvm: Fix problem when running with SELinux under z/VM
On 24.03.2017 10:38, Cornelia Huck wrote:
> On Fri, 24 Mar 2017 10:26:55 +0100
> Thomas Huth wrote:
[...]
>> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
>> index ac47154..5167436 100644
>> --- a/target/s390x/kvm.c
>> +++ b/target/s390x/kvm.c
>> @@ -678,8 +678,7 @@ static void *legacy_s390_alloc(size_t size, uint64_t
>> *align)
>> {
>> void *mem;
>>
>> -mem = mmap((void *) 0x8ULL, size,
>> - PROT_EXEC|PROT_READ|PROT_WRITE,
>> +mem = mmap((void *) 0x8ULL, size, PROT_READ | PROT_WRITE,
>> MAP_SHARED | MAP_ANONYMOUS | MAP_FIXED, -1, 0);
>> return mem == MAP_FAILED ? NULL : mem;
>> }
>
> Wouldn't it be better to adapt the SELinux rules?
I don't think that we want to change the default behavior of SELinux
here, since this is a security feature. Fortunately, there is already a
SELinux configuration variable available which can be used as a workaround:
setsebool virt_use_execmem 1
But still, it would be nicer, if things worked out of the box instead...
Thomas
Re: [Qemu-devel] [PATCH 01/51] ram: Update all functions comments
Hi, Juan,
Got several nitpicks below... (along with some questions)
On Thu, Mar 23, 2017 at 09:44:54PM +0100, Juan Quintela wrote:
[...]
> static void xbzrle_cache_zero_page(ram_addr_t current_addr)
> {
> @@ -459,8 +474,8 @@ static void xbzrle_cache_zero_page(ram_addr_t
> current_addr)
> * -1 means that xbzrle would be longer than normal
> *
> * @f: QEMUFile where to send the data
> - * @current_data:
> - * @current_addr:
> + * @current_data: contents of the page
Since current_data is a double pointer, so... maybe "pointer to the
address of page content"?
Btw, a question not related to this series... Why here in
save_xbzrle_page() we need to update *current_data to be the newly
created page cache? I see that we have:
/* update *current_data when the page has been
inserted into cache */
*current_data = get_cached_data(XBZRLE.cache, current_addr);
What would be the difference if we just use the old pointer in
RAMBlock.host?
[...]
> @@ -1157,11 +1186,12 @@ static bool get_queued_page(MigrationState *ms,
> PageSearchStatus *pss,
> }
>
> /**
> - * flush_page_queue: Flush any remaining pages in the ram request queue
> - *it should be empty at the end anyway, but in error cases there may be
> - *some left.
> + * flush_page_queue: flush any remaining pages in the ram request queue
Here the comment says (just like mentioned in function name) that we
will "flush any remaining pages in the ram request queue", however in
the implementation, we should be only freeing everything in
src_page_requests. The problem is "flush" let me think about "flushing
the rest of the pages to the other side"... while it's not.
Would it be nice we just rename the function into something else, like
migration_page_queue_free()? We can tune the comments correspondingly
as well.
[...]
> -/*
> - * Helper for postcopy_chunk_hostpages; it's called twice to cleanup
> - * the two bitmaps, that are similar, but one is inverted.
> +/**
> + * postcopy_chuck_hostpages_pass: canocalize bitmap in hostpages
^ should be n? ^^ canonicalize?
> *
> - * We search for runs of target-pages that don't start or end on a
> - * host page boundary;
> - * unsent_pass=true: Cleans up partially unsent host pages by searching
> - * the unsentmap
> - * unsent_pass=false: Cleans up partially dirty host pages by searching
> - * the main migration bitmap
> + * Helper for postcopy_chunk_hostpages; it's called twice to
> + * canonicalize the two bitmaps, that are similar, but one is
> + * inverted.
> *
> + * Postcopy requires that all target pages in a hostpage are dirty or
> + * clean, not a mix. This function canonicalizes the bitmaps.
> + *
> + * @ms: current migration state
> + * @unsent_pass: if true we need to canonicalize partially unsent host pages
> + * otherwise we need to canonicalize partially dirty host pages
> + * @block: block that contains the page we want to canonicalize
> + * @pds: state for postcopy
> */
> static void postcopy_chunk_hostpages_pass(MigrationState *ms, bool
> unsent_pass,
>RAMBlock *block,
[...]
> +/**
> + * ram_save_setup: iterative stage for migration
^^ should be ram_save_iterate()?
> + *
> + * Returns zero to indicate success and negative for error
> + *
> + * @f: QEMUFile where to send the data
> + * @opaque: RAMState pointer
> + */
> static int ram_save_iterate(QEMUFile *f, void *opaque)
> {
> int ret;
> @@ -2091,7 +2169,16 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
> return done;
> }
[...]
> -/*
> - * Allocate data structures etc needed by incoming migration with
> postcopy-ram
> - * postcopy-ram's similarly names postcopy_ram_incoming_init does the work
> +/**
> + * ram_postococpy_incoming_init: allocate postcopy data structures
> + *
> + * Returns 0 for success and negative if there was one error
> + *
> + * @mis: current migration incoming state
> + *
> + * Allocate data structures etc needed by incoming migration with
> + * postcopy-ram postcopy-ram's similarly names
> + * postcopy_ram_incoming_init does the work
This sentence is slightly hard to understand... But I think the
function name explained itself enough though. :)
Thanks,
-- peterx
Re: [Qemu-devel] Guest application reading from pl011 without device driver
Hi, now it is working with the following char-pipe_new.c. See the patch. Doing the "if fifo has space" polling only when there are data on the pipe, and next reading from pipe. The sub_thread does these three things, and the main thread only does writing to UART fifo when the subthread signalled its handler. It is working in my case to transfer large data. Rational? Please check. On 24 March 2017 at 08:24, Jiahuan Zhang wrote: > Hi, here are the patch files for char-pipe.c, char-win.c, char-win.h > > > On 23 March 2017 at 18:31, Paolo Bonzini wrote: > >> >> >> On 23/03/2017 18:28, Jiahuan Zhang wrote: >> > Hi, the method doesn't work for pipe. It still causes the same issue. >> > The only difference is that the first byte in the UART fifo can be read >> > by the guest app. >> > So now my guest app can immediately read 17bytes when the host is >> > sending data continuously, >> > then guest app is unable to read. >> > >> > Now there is a solution-to-be. Set the qemu pipe chardev to wait for 1 >> > millisecond before do a next ReadFile(), >> > then the pl011 can deliver the complete large data by the 16-byte fifo >> > continuously to the guest app. >> > >> > Is it rational? Or is this host -CPU-dependent? >> >> Post the patch, maybe you have a bug. >> >> Paolo >> > > char-pipe_new.patch Description: Binary data
Re: [Qemu-devel] [PATCH] target/s390x/kvm: Fix problem when running with SELinux under z/VM
On 24.03.2017 10:39, Christian Borntraeger wrote: > On 03/24/2017 10:26 AM, Thomas Huth wrote: >> When running QEMU with KVM under z/VM, the memory for the guest >> is allocated via legacy_s390_alloc() since the KVM_CAP_S390_COW >> extension is not supported on z/VM. legacy_s390_alloc() then uses >> mmap(... PROT_EXEC ...) for the guest memory - but this does not >> work when running with SELinux enabled, mmap() fails and QEMU aborts >> with the following error message: >> >> cannot set up guest memory 's390.ram': Permission denied >> >> Looking at the other allocator function qemu_anon_ram_alloc(), it >> seems like PROT_EXEC is normally not needed for allocating the >> guest RAM, and indeed, the guest also starts successfully under >> z/VM when we remove the PROT_EXEC from the legacy_s390_alloc() >> function. So let's get rid of that flag here to be able to run >> with SELinux under z/VM, too. > > Older z/VM versions do not provide the enhanced suppression on protection > facility, which would result in guest failures as soon as the kernel > starts dirty pages tracking by write protecting the pages via the page > table. Some kernel release back (last time I checked) the PROT_EXEC was > necessary to prevent the dirty pages tracking from taking place. So this > patch would break KVM in that case. OK, then please ignore it. > Newer z/VMs (e.g. 6.3) do provide ESOP. SO the question is, > why is KVM_CAP_S390_COW not set? I'll check whether MACHINE_HAS_ESOP is correctly set in my kernel... Thomas
Re: [Qemu-devel] [PATCH 42/51] ram: Pass RAMBlock to bitmap_sync
Yang Hongyang wrote:
> Hi Juan,
>
> On 2017/3/24 16:29, Juan Quintela wrote:
>> Yang Hongyang wrote:
>>> On 2017/3/24 4:45, Juan Quintela wrote:
We change the meaning of start to be the offset from the beggining of
the block.
@@ -701,7 +701,7 @@ static void migration_bitmap_sync(RAMState *rs)
qemu_mutex_lock(&rs->bitmap_mutex);
rcu_read_lock();
QLIST_FOREACH_RCU(block, &ram_list.blocks, next) {
-migration_bitmap_sync_range(rs, block->offset,
block->used_length);
+migration_bitmap_sync_range(rs, block, 0, block->used_length);
>>>
>>> Since RAMBlock been passed to bitmap_sync, could we remove
>>> param 'block->used_length' either?
>>
>> Hi
>>
>> good catch.
>>
>> I had that removed, and then realized that I want to synchronize parts
>> of the bitmap, not the whole one. That part of the series is still not
>> done.
>>
>> Right now we do something like (I have simplified a lot of details):
>>
>> while(true) {
>> foreach(block)
>> bitmap_sync(block)
>> foreach(page)
>> if(dirty(page))
>>page_send(page)
>> }
>>
>>
>> If you have several terabytes of RAM that is too ineficient, because
>> when we arrive to the page_send(page), it is possible that it is already
>> dirty again, and we have to send it twice. So, the idea is to change to
>> something like:
>>
>> while(true) {
>> foreach(block)
>> bitmap_sync(block)
>
> Do you mean sync with KVM here?
>
>> foreach(block)
>> foreach(64pages)
>> bitmap_sync(64pages)
>
> Then here, we will sync with KVM too. For huge MEM,
> it will generates lots of ioctl()...
> Bitmap in KVM is per Memory region IIRC. KVM module currently
> haven't the ability to sync parts of the bitmap. A sync have
> to sync the whole mr. So if we want to do small sync, we might
> need to modify KVM also, but that still won't solve the preblem
> of increased ioctls.
And why I remembered incorrectly that we could sync part of the bitmaps.
Yes, we could have more ioctls, but less pages written twice, it is a
tradeoff, at some point it makes sense to change it.
Problem is that now this is going to be more difficult that I thought to
test for it.
Thanks, Juan.
Re: [Qemu-devel] [Qemu-arm] [patch 1/1]about armv8's prefetch decode
On 24 March 2017 at 06:14, Wangjintang wrote:
> Hi Pranith,
>
> Thanks for your reply. patch as below, new added code default is off,
> please review.
> The major thinking is about translate Armv8's prefetch instruction into
> intermediate code, at the same time don't effect the rm/rn register.
>
>
> diff --git a/translate-a64.c b/translate-a64.c
> index 814f30f..86da8ee 100644
> --- a/translate-a64.c
> +++ b/translate-a64.c
> @@ -2061,7 +2061,11 @@ static void disas_ld_lit(DisasContext *s, uint32_t
> insn)
> } else {
> if (opc == 3) {
> /* PRFM (literal) : prefetch */
> +#ifdef TCG_AARCH64_PREFETCH_TRANSLATE
> +;
> +#else
> return;
> +#endif
> }
No, these changes look wrong. PRFM instructions do not need to
do anything and should definitely not be emitting any intermediate
code. In particular if you let execution fall through and try
do_gpr_ld() then it will really do a load, which might cause
an exception -- this is specifically forbidden for PRFM.
Architecturally the ARM ARM says "it is valid for the PE to
treat any or all prefetch instructions as a NOP", which is
what QEMU does.
The existing code is correct. In general you should not
expect to be able to deduce the guest instructions from
the intermediate code representation.
thanks
-- PMM
Re: [Qemu-devel] [RFC 1/4] [UNTESTED] xen: Don't force has_dynamic_sysbus on machine class
On 24 March 2017 at 08:23, Juergen Gross wrote: > On 23/03/17 22:28, Eduardo Habkost wrote: >> The xen-backend devices created by the Xen code are not supposed >> to be treated as dynamic sysbus devices. This is an attempt to >> change that and see what happens, but I couldn't test it because >> I don't have a Xen host set up. >> >> If this patch breaks anything, this means we have a bug in >> foreach_dynamic_sysbus_device(), which is supposed to return only >> devices created using -device. >> >> The original code that sets has_dynamic_sysbus was added by >> commit 3a6c9172ac5951e6dac2b3f6cbce3cfccdec5894, but I don't see >> any comment explaining why it was necessary. > > xen-backend devices are created via qmp commands when attaching new > pv-devices to a domain. They can be dynamically removed, too. Setting > has_dynamic_sysbus was necessary to support this feature. This seems like it ought to be handled by marking the xen-backend devices as being ok-to-dynamically-create somehow, not by marking the machine as supporting dynamic-sysbus (which it doesn't). Maybe we don't have the necessary support code to do that though? thanks -- PMM
Re: [Qemu-devel] [PATCH 17/51] ram: Move xbzrle_bytes into RAMState
* Juan Quintela (quint...@redhat.com) wrote:
> Signed-off-by: Juan Quintela
Reviewed-by: Dr. David Alan Gilbert
> ---
> migration/ram.c | 8 +---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/migration/ram.c b/migration/ram.c
> index 690ca8f..721fd66 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -172,6 +172,8 @@ struct RAMState {
> uint64_t norm_pages;
> /* Iterations since start */
> uint64_t iterations;
> +/* xbzrle transmitted bytes */
> +uint64_t xbzrle_bytes;
> };
> typedef struct RAMState RAMState;
>
> @@ -179,7 +181,6 @@ static RAMState ram_state;
>
> /* accounting for migration statistics */
> typedef struct AccountingInfo {
> -uint64_t xbzrle_bytes;
> uint64_t xbzrle_pages;
> uint64_t xbzrle_cache_miss;
> double xbzrle_cache_miss_rate;
> @@ -205,7 +206,7 @@ uint64_t norm_mig_pages_transferred(void)
>
> uint64_t xbzrle_mig_bytes_transferred(void)
> {
> -return acct_info.xbzrle_bytes;
> +return ram_state.xbzrle_bytes;
> }
>
> uint64_t xbzrle_mig_pages_transferred(void)
> @@ -544,7 +545,7 @@ static int save_xbzrle_page(RAMState *rs, QEMUFile *f,
> uint8_t **current_data,
> qemu_put_buffer(f, XBZRLE.encoded_buf, encoded_len);
> bytes_xbzrle += encoded_len + 1 + 2;
> acct_info.xbzrle_pages++;
> -acct_info.xbzrle_bytes += bytes_xbzrle;
> +rs->xbzrle_bytes += bytes_xbzrle;
> *bytes_transferred += bytes_xbzrle;
>
> return 1;
> @@ -1995,6 +1996,7 @@ static int ram_save_init_globals(RAMState *rs)
> rs->zero_pages = 0;
> rs->norm_pages = 0;
> rs->iterations = 0;
> +rs->xbzrle_bytes = 0;
> migration_bitmap_sync_init(rs);
> qemu_mutex_init(&migration_bitmap_mutex);
>
> --
> 2.9.3
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH 18/51] ram: Move xbzrle_pages into RAMState
* Juan Quintela (quint...@redhat.com) wrote:
> Signed-off-by: Juan Quintela
> ---
> migration/ram.c | 8 +---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/migration/ram.c b/migration/ram.c
> index 721fd66..b4e647a 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -174,6 +174,8 @@ struct RAMState {
> uint64_t iterations;
> /* xbzrle transmitted bytes */
> uint64_t xbzrle_bytes;
> +/* xbzrle transmmited pages */
> +uint64_t xbzrle_pages;
Yes, it might be useful to comment that the bytes are compressed bytes
while the pages are raw so it's not just a ratio.
Reviewed-by: Dr. David Alan Gilbert
> };
> typedef struct RAMState RAMState;
>
> @@ -181,7 +183,6 @@ static RAMState ram_state;
>
> /* accounting for migration statistics */
> typedef struct AccountingInfo {
> -uint64_t xbzrle_pages;
> uint64_t xbzrle_cache_miss;
> double xbzrle_cache_miss_rate;
> uint64_t xbzrle_overflows;
> @@ -211,7 +212,7 @@ uint64_t xbzrle_mig_bytes_transferred(void)
>
> uint64_t xbzrle_mig_pages_transferred(void)
> {
> -return acct_info.xbzrle_pages;
> +return ram_state.xbzrle_pages;
> }
>
> uint64_t xbzrle_mig_pages_cache_miss(void)
> @@ -544,7 +545,7 @@ static int save_xbzrle_page(RAMState *rs, QEMUFile *f,
> uint8_t **current_data,
> qemu_put_be16(f, encoded_len);
> qemu_put_buffer(f, XBZRLE.encoded_buf, encoded_len);
> bytes_xbzrle += encoded_len + 1 + 2;
> -acct_info.xbzrle_pages++;
> +rs->xbzrle_pages++;
> rs->xbzrle_bytes += bytes_xbzrle;
> *bytes_transferred += bytes_xbzrle;
>
> @@ -1997,6 +1998,7 @@ static int ram_save_init_globals(RAMState *rs)
> rs->norm_pages = 0;
> rs->iterations = 0;
> rs->xbzrle_bytes = 0;
> +rs->xbzrle_pages = 0;
> migration_bitmap_sync_init(rs);
> qemu_mutex_init(&migration_bitmap_mutex);
>
> --
> 2.9.3
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH v3 13/34] tcg: Add atomic helpers
Richard Henderson writes:
> On 09/12/2016 06:47 AM, Alex Bennée wrote:
>>> > +/* Notice an IO access, or a notdirty page. */
>>> > +if (unlikely(tlb_addr & ~TARGET_PAGE_MASK)) {
>>> > +/* There's really nothing that can be done to
>>> > + support this apart from stop-the-world. */
>>> > +goto stop_the_world;
>> We are also triggering on TLB_NOTDIRTY here in the case where a
>> conditional write is the first write to a page. I don't know if a
>> stop_the_world is required at this point but we will need to ensure we
>> clear bits as notdirty_mem_write() does.
>>
>
> You're quite right that we could probably special-case TLB_NOTDIRTY here such
> that (1) we needn't leave the cpu loop, and (2) needn't utilize the actual
> "write" part of notdirty_mem_write; just set the bits then fall through to the
> actual atomic instruction below.
I do hit this case with ppc64, where I see that its the first write to
the page and it exits from this every time, causing the kernel to print
soft-lockups.
Can we add the special case here for NOTDIRTY and set the page as dirty
and return successfully?
Regards
Nikunj
Re: [Qemu-devel] [PATCH 19/51] ram: Move xbzrle_cache_miss into RAMState
* Juan Quintela (quint...@redhat.com) wrote:
> Signed-off-by: Juan Quintela
Reviewed-by: Dr. David Alan Gilbert
> ---
> migration/ram.c | 12 +++-
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/migration/ram.c b/migration/ram.c
> index b4e647a..cc19406 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -176,6 +176,8 @@ struct RAMState {
> uint64_t xbzrle_bytes;
> /* xbzrle transmmited pages */
> uint64_t xbzrle_pages;
> +/* xbzrle number of cache miss */
> +uint64_t xbzrle_cache_miss;
> };
> typedef struct RAMState RAMState;
>
> @@ -183,7 +185,6 @@ static RAMState ram_state;
>
> /* accounting for migration statistics */
> typedef struct AccountingInfo {
> -uint64_t xbzrle_cache_miss;
> double xbzrle_cache_miss_rate;
> uint64_t xbzrle_overflows;
> } AccountingInfo;
> @@ -217,7 +218,7 @@ uint64_t xbzrle_mig_pages_transferred(void)
>
> uint64_t xbzrle_mig_pages_cache_miss(void)
> {
> -return acct_info.xbzrle_cache_miss;
> +return ram_state.xbzrle_cache_miss;
> }
>
> double xbzrle_mig_cache_miss_rate(void)
> @@ -497,7 +498,7 @@ static int save_xbzrle_page(RAMState *rs, QEMUFile *f,
> uint8_t **current_data,
> uint8_t *prev_cached_page;
>
> if (!cache_is_cached(XBZRLE.cache, current_addr, rs->bitmap_sync_count))
> {
> -acct_info.xbzrle_cache_miss++;
> +rs->xbzrle_cache_miss++;
> if (!last_stage) {
> if (cache_insert(XBZRLE.cache, current_addr, *current_data,
> rs->bitmap_sync_count) == -1) {
> @@ -698,12 +699,12 @@ static void migration_bitmap_sync(RAMState *rs)
> if (migrate_use_xbzrle()) {
> if (rs->iterations_prev != rs->iterations) {
> acct_info.xbzrle_cache_miss_rate =
> - (double)(acct_info.xbzrle_cache_miss -
> + (double)(rs->xbzrle_cache_miss -
> rs->xbzrle_cache_miss_prev) /
> (rs->iterations - rs->iterations_prev);
> }
> rs->iterations_prev = rs->iterations;
> -rs->xbzrle_cache_miss_prev = acct_info.xbzrle_cache_miss;
> +rs->xbzrle_cache_miss_prev = rs->xbzrle_cache_miss;
> }
> s->dirty_pages_rate = rs->num_dirty_pages_period * 1000
> / (end_time - rs->start_time);
> @@ -1999,6 +2000,7 @@ static int ram_save_init_globals(RAMState *rs)
> rs->iterations = 0;
> rs->xbzrle_bytes = 0;
> rs->xbzrle_pages = 0;
> +rs->xbzrle_cache_miss = 0;
> migration_bitmap_sync_init(rs);
> qemu_mutex_init(&migration_bitmap_mutex);
>
> --
> 2.9.3
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH 20/51] ram: Move xbzrle_cache_miss_rate into RAMState
* Juan Quintela (quint...@redhat.com) wrote:
> Signed-off-by: Juan Quintela
Reviewed-by: Dr. David Alan Gilbert
> ---
> migration/ram.c | 8 +---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/migration/ram.c b/migration/ram.c
> index cc19406..c398ff9 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -178,6 +178,8 @@ struct RAMState {
> uint64_t xbzrle_pages;
> /* xbzrle number of cache miss */
> uint64_t xbzrle_cache_miss;
> +/* xbzrle miss rate */
> +double xbzrle_cache_miss_rate;
> };
> typedef struct RAMState RAMState;
>
> @@ -185,7 +187,6 @@ static RAMState ram_state;
>
> /* accounting for migration statistics */
> typedef struct AccountingInfo {
> -double xbzrle_cache_miss_rate;
> uint64_t xbzrle_overflows;
> } AccountingInfo;
>
> @@ -223,7 +224,7 @@ uint64_t xbzrle_mig_pages_cache_miss(void)
>
> double xbzrle_mig_cache_miss_rate(void)
> {
> -return acct_info.xbzrle_cache_miss_rate;
> +return ram_state.xbzrle_cache_miss_rate;
> }
>
> uint64_t xbzrle_mig_pages_overflow(void)
> @@ -698,7 +699,7 @@ static void migration_bitmap_sync(RAMState *rs)
>
> if (migrate_use_xbzrle()) {
> if (rs->iterations_prev != rs->iterations) {
> -acct_info.xbzrle_cache_miss_rate =
> +rs->xbzrle_cache_miss_rate =
> (double)(rs->xbzrle_cache_miss -
> rs->xbzrle_cache_miss_prev) /
> (rs->iterations - rs->iterations_prev);
> @@ -2001,6 +2002,7 @@ static int ram_save_init_globals(RAMState *rs)
> rs->xbzrle_bytes = 0;
> rs->xbzrle_pages = 0;
> rs->xbzrle_cache_miss = 0;
> +rs->xbzrle_cache_miss_rate = 0;
> migration_bitmap_sync_init(rs);
> qemu_mutex_init(&migration_bitmap_mutex);
>
> --
> 2.9.3
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH 21/51] ram: Move xbzrle_overflows into RAMState
* Juan Quintela (quint...@redhat.com) wrote:
> Once there, remove the now unused AccountingInfo struct and var.
Reviewed-by: Dr. David Alan Gilbert
> Signed-off-by: Juan Quintela
> ---
> migration/ram.c | 21 +
> 1 file changed, 5 insertions(+), 16 deletions(-)
>
> diff --git a/migration/ram.c b/migration/ram.c
> index c398ff9..3292eb0 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -180,23 +180,13 @@ struct RAMState {
> uint64_t xbzrle_cache_miss;
> /* xbzrle miss rate */
> double xbzrle_cache_miss_rate;
> +/* xbzrle number of overflows */
> +uint64_t xbzrle_overflows;
> };
> typedef struct RAMState RAMState;
>
> static RAMState ram_state;
>
> -/* accounting for migration statistics */
> -typedef struct AccountingInfo {
> -uint64_t xbzrle_overflows;
> -} AccountingInfo;
> -
> -static AccountingInfo acct_info;
> -
> -static void acct_clear(void)
> -{
> -memset(&acct_info, 0, sizeof(acct_info));
> -}
> -
> uint64_t dup_mig_pages_transferred(void)
> {
> return ram_state.zero_pages;
> @@ -229,7 +219,7 @@ double xbzrle_mig_cache_miss_rate(void)
>
> uint64_t xbzrle_mig_pages_overflow(void)
> {
> -return acct_info.xbzrle_overflows;
> +return ram_state.xbzrle_overflows;
> }
>
> static QemuMutex migration_bitmap_mutex;
> @@ -527,7 +517,7 @@ static int save_xbzrle_page(RAMState *rs, QEMUFile *f,
> uint8_t **current_data,
> return 0;
> } else if (encoded_len == -1) {
> trace_save_xbzrle_page_overflow();
> -acct_info.xbzrle_overflows++;
> +rs->xbzrle_overflows++;
> /* update data in the cache */
> if (!last_stage) {
> memcpy(prev_cached_page, *current_data, TARGET_PAGE_SIZE);
> @@ -2003,6 +1993,7 @@ static int ram_save_init_globals(RAMState *rs)
> rs->xbzrle_pages = 0;
> rs->xbzrle_cache_miss = 0;
> rs->xbzrle_cache_miss_rate = 0;
> +rs->xbzrle_overflows = 0;
> migration_bitmap_sync_init(rs);
> qemu_mutex_init(&migration_bitmap_mutex);
>
> @@ -2033,8 +2024,6 @@ static int ram_save_init_globals(RAMState *rs)
> XBZRLE.encoded_buf = NULL;
> return -1;
> }
> -
> -acct_clear();
> }
>
> /* For memory_global_dirty_log_start below. */
> --
> 2.9.3
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [RFC 1/4] [UNTESTED] xen: Don't force has_dynamic_sysbus on machine class
On 24/03/17 11:09, Peter Maydell wrote: > On 24 March 2017 at 08:23, Juergen Gross wrote: >> On 23/03/17 22:28, Eduardo Habkost wrote: >>> The xen-backend devices created by the Xen code are not supposed >>> to be treated as dynamic sysbus devices. This is an attempt to >>> change that and see what happens, but I couldn't test it because >>> I don't have a Xen host set up. >>> >>> If this patch breaks anything, this means we have a bug in >>> foreach_dynamic_sysbus_device(), which is supposed to return only >>> devices created using -device. >>> >>> The original code that sets has_dynamic_sysbus was added by >>> commit 3a6c9172ac5951e6dac2b3f6cbce3cfccdec5894, but I don't see >>> any comment explaining why it was necessary. >> >> xen-backend devices are created via qmp commands when attaching new >> pv-devices to a domain. They can be dynamically removed, too. Setting >> has_dynamic_sysbus was necessary to support this feature. > > This seems like it ought to be handled by marking the xen-backend > devices as being ok-to-dynamically-create somehow, not by marking > the machine as supporting dynamic-sysbus (which it doesn't). > Maybe we don't have the necessary support code to do that though? When writing the patches I couldn't find a way to do it differently. OTOH I'm not so deep in qemu internals I'd be able to add the needed support. I'd be happy to test any patch, though. Juergen
Re: [Qemu-devel] [RFC 1/4] [UNTESTED] xen: Don't force has_dynamic_sysbus on machine class
On 24.03.2017 11:09, Peter Maydell wrote: > On 24 March 2017 at 08:23, Juergen Gross wrote: >> On 23/03/17 22:28, Eduardo Habkost wrote: >>> The xen-backend devices created by the Xen code are not supposed >>> to be treated as dynamic sysbus devices. This is an attempt to >>> change that and see what happens, but I couldn't test it because >>> I don't have a Xen host set up. >>> >>> If this patch breaks anything, this means we have a bug in >>> foreach_dynamic_sysbus_device(), which is supposed to return only >>> devices created using -device. >>> >>> The original code that sets has_dynamic_sysbus was added by >>> commit 3a6c9172ac5951e6dac2b3f6cbce3cfccdec5894, but I don't see >>> any comment explaining why it was necessary. >> >> xen-backend devices are created via qmp commands when attaching new >> pv-devices to a domain. They can be dynamically removed, too. Setting >> has_dynamic_sysbus was necessary to support this feature. > > This seems like it ought to be handled by marking the xen-backend > devices as being ok-to-dynamically-create somehow, not by marking > the machine as supporting dynamic-sysbus (which it doesn't). > Maybe we don't have the necessary support code to do that though? Do the xen devices have to be sysbus devices? If no, I think you could change them to be of type TYPE_DEVICE nowadays - TYPE_DEVICE can be instantiated with "-device" nowadays, too. That's what we do with the spapr-rng device for example (see hw/ppc/spapr_rng.c). Thomas
[Qemu-devel] guest-exec of qemu-guest-agent fails on Windows guest
Hi everyone,
We are trying to use guest-exec[0] command of qemu-guest-agent. However,
discovered that whatever command we try to run, it ends up with the
following error on the host side:
> error: Guest agent is not responding: Guest agent not available for now
An example of what we run on the host side:
> virsh qemu-agent-command --domain armin_1 '{ "execute": "guest-exec",
"arguments": { "path": "C:/Windows/System32/ipconfig.exe",
"capture-output": false } }'
Because Windows was complaining about `gspawn-win64-helper-console.exe`
whenever we ran a command on host, we tried to run it directly from cli and
it failed on the following assertion:
> g_assert (argc >= ARG_COUNT); // [1]
This picture[2] might be a bit more useful as it includes the output of
`qemu-ga` in verbose mode.
Any idea how we can solve it and/or what we are doing wrong?
Thanks in advance.
[0]
https://github.com/qemu/qemu/blob/fbddc2e5608eb655493253d080598375db61a748/qga/qapi-schema.json#L1024
[1]
https://github.com/GNOME/glib/blob/8edcf67b0221efa3c2ada67c44eff29939b1704d/glib/gspawn-win32-helper.c#L208
[2] https://ibin.co/3GTVo4WXfR7a.png
---
Armin ranjbar
Re: [Qemu-devel] [Qemu-ppc] [Bug 1674925] Re: Qemu PPC64 kvm no display if --device virtio-gpu-pci is selected
Hello Luigi, I have some difficulty sorting out what is going on and what could be considered a regression :/ you are reporting many issues at the same time with a home made kernel. Could you please use the kernel shipped with the distro to start with ? > yes usually with 2.8 i boot the VM without issue on G5 Quad with the > option > -M pseries from 2.1 to 2.5 with kvm-pr enabled. > i did the tests and with all pseries now on 2.9 i have the same issue. > example: > qemu-system-ppc64 --enable-kvm -cpu 970fx_v2.0 -m 1024 -M pseries-2.1 > qemu-system-ppc64: KVM and IRQ_XICS capability must be present for in-kernel > XICS This error message is because your host kernel lacks in-kernel XICS, but you are saying that was not an issue with QEMU-2.8. Correct ? If so, that is a first thing to look at. May be the changes I made introduced some regression in that case. Here is the command line I used on a 17.04 host : qemu-system-ppc64 -M pseries-2.[1-8],accel=kvm,kvm-type=PR -cpu 970fx_v2.0 -m 1024 -nographic I get this message with QEMU 2.9, for all pseries qemu-system-ppc64: Unable to find sPAPR CPU Core definition with QEMU 2.8, the guest starts for pseries-2.[1-6] and fails the same way for pseries-2.[7-8]. I tried with '-cpu 970_v2.2' and there are no issues but that is a defined sPAPR CPU Core. Did we introduce a regression in compatibility in QEMU 2.9 ? or was it bogus before ? That needs a little digging. I did not work on that part. Thanks, C. > but no issue if i run with -M mac99 before 2.9 was not possible use it on > qemu-system-ppc64 > It means it will no possible anymore in future release of qemu use open > firmware on powermacs > any moore? > >>I admit the message is not very clear, but the host kernel is >>using a dev config. > > Im so sorry, i learn English by my self reading ml and on irc > chatting is too difficult where no one speak English around. > >>> On Qoriq if i pass the -cpu e500 (normal thing) all is working right qemu >>> 2.9rc1 >>> all boot and so and so. >>but you must be changing the machine right ? > not on Qoriq because it is book3e and is not so flexible like the G5 Quad who > is book3s machine. > i can run qemu kvm only with emb hardware on Qoriq > >>>On G5 the -cpu variable dont fix the issue. >>with which machine ? > On PowerMac G5 Quad 970MP it have similar hardware configuration of IBM > intellistation power 285 > >> >I can build a new kernel release , usually mine dont have xics enabled >> >because G5 >>> dont have that feature, if needed i can enable it for testing.** >>Yes that would be interesting. > > I will do ASAP just the time to build it . > > Thank you really much for your time and patience. > Luigi > --
Re: [Qemu-devel] [PATCH for-2.9 v3 0/2] virtio-scsi: Fix assertion failure on dataplane handlers
On 17/03/2017 07:14, Fam Zheng wrote: > v3: Don't forget the virtio_scsi_release in virtio_scsi_handle_event_vq. [Ed > Swierk] > v2: Use virtio_scsi_acquire/release. [Paolo] > > Fam Zheng (2): > virtio-scsi: Make virtio_scsi_acquire/release public > virtio-scsi: Fix acquire/release in dataplane handlers > > hw/scsi/virtio-scsi-dataplane.c | 20 > hw/scsi/virtio-scsi.c | 41 > ++--- > include/hw/virtio/virtio-scsi.h | 14 ++ > 3 files changed, 44 insertions(+), 31 deletions(-) > Queued, thanks. Paolo
Re: [Qemu-devel] [PATCH] tcg/i386: Check the size of instruction being translated
Queued, thanks.
Paolo
On 23/03/2017 18:58, Pranith Kumar wrote:
> Sending again since I messed by pbonzini's email.
>
> This fixes the bug: 'user-to-root privesc inside VM via bad translation
> caching' reported by Jann Horn here:
> https://bugs.chromium.org/p/project-zero/issues/detail?id=1122
>
> CC: Richard Henderson
> CC: Peter Maydell
> CC: Paolo Bonzini
> Reported-by: Jann Horn
> Signed-off-by: Pranith Kumar
> ---
> target/i386/translate.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/target/i386/translate.c b/target/i386/translate.c
> index 72c1b03a2a..1d1372fb43 100644
> --- a/target/i386/translate.c
> +++ b/target/i386/translate.c
> @@ -4418,6 +4418,13 @@ static target_ulong disas_insn(CPUX86State *env,
> DisasContext *s,
> s->vex_l = 0;
> s->vex_v = 0;
> next_byte:
> +/* x86 has an upper limit of 15 bytes for an instruction. Since we
> + * do not want to decode and generate IR for an illegal
> + * instruction, the following check limits the instruction size to
> + * 25 bytes: 14 prefix + 1 opc + 6 (modrm+sib+ofs) + 4 imm */
> +if (s->pc - pc_start > 14) {
> +goto illegal_op;
> +}
> b = cpu_ldub_code(env, s->pc);
> s->pc++;
> /* Collect prefixes. */
>
Re: [Qemu-devel] [PATCH] mem-prealloc: fix sysconf(_SC_NPROCESSORS_ONLN) failure case.
On 21/03/2017 07:50, Jitendra Kolhe wrote:
> This was spotted by Coverity, in case where sysconf(_SC_NPROCESSORS_ONLN)
> fails and returns -1. This results in memset_num_threads getting set to -1.
> Which we then pass to g_new0().
> The patch replaces MAX_MEM_PREALLOC_THREAD_COUNT macro with a function call
> get_memset_num_threads() to handle sysconf() failure gracefully. In case
> sysconf() fails, we fall back to single threaded.
>
> (Spotted by Coverity, CID 1372465.)
>
> Signed-off-by: Jitendra Kolhe
> ---
> util/oslib-posix.c | 16 ++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index 3fe6089..4d9189e 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -55,7 +55,7 @@
> #include "qemu/error-report.h"
> #endif
>
> -#define MAX_MEM_PREALLOC_THREAD_COUNT (MIN(sysconf(_SC_NPROCESSORS_ONLN),
> 16))
> +#define MAX_MEM_PREALLOC_THREAD_COUNT 16
>
> struct MemsetThread {
> char *addr;
> @@ -381,6 +381,18 @@ static void *do_touch_pages(void *arg)
> return NULL;
> }
>
> +static inline int get_memset_num_threads(int smp_cpus)
> +{
> +long host_procs = sysconf(_SC_NPROCESSORS_ONLN);
> +int ret = 1;
> +
> +if (host_procs > 0) {
> +ret = MIN(MIN(host_procs, MAX_MEM_PREALLOC_THREAD_COUNT), smp_cpus);
> +}
> +/* In case sysconf() fails, we fall back to single threaded */
> +return ret;
> +}
> +
> static bool touch_all_pages(char *area, size_t hpagesize, size_t numpages,
> int smp_cpus)
> {
> @@ -389,7 +401,7 @@ static bool touch_all_pages(char *area, size_t hpagesize,
> size_t numpages,
> int i = 0;
>
> memset_thread_failed = false;
> -memset_num_threads = MIN(smp_cpus, MAX_MEM_PREALLOC_THREAD_COUNT);
> +memset_num_threads = get_memset_num_threads(smp_cpus);
> memset_thread = g_new0(MemsetThread, memset_num_threads);
> numpages_per_thread = (numpages / memset_num_threads);
> size_per_thread = (hpagesize * numpages_per_thread);
>
Queued, thanks.
Paolo
Re: [Qemu-devel] q35 and sysbus devices
On 03/22/2017 10:46 PM, Laszlo Ersek wrote: On 03/22/17 21:31, Eduardo Habkost wrote: Hi, I am investigating the current status of has_dynamic_sysbus and sysbus device support on each of QEMU's machine types. The good news is that almost all has_dynamic_sysbus=1 machines have their own internal (often short) whitelist of supported sysbus device types, and automatically reject unsupported devices. ...except for q35. q35 currently accepts all sys-bus-device subtypes on "-device", and today this includes the following 23 devices: * allwinner-ahci * amd-iommu * cfi.pflash01 * esp * fw_cfg_io * fw_cfg_mem * generic-sdhci * hpet * intel-iommu * ioapic * isabus-bridge * kvmclock * kvm-ioapic * kvmvapic * SUNW,fdtwo * sysbus-ahci * sysbus-fdc * sysbus-ohci * unimplemented-device * virtio-mmio * xen-backend * xen-sysdev My question is: do all those devices really make sense to be used with "-device" on q35? I think fw_cfg_io and fw_cfg_mem should be board-only devices (no -device switch). Regarding cfi.pflash01, I think originally it would have been nice to specify pflash chips with the modern (non-legacy) syntax, that is, separate -drive if=none,file=... backend options combined with -device cfi.pflash01,drive=... frontend options. However, that ship has sailed, even libvirt uses -drive if=pflash for these, and given the purpose we use pflash chips for, on Q35, I don't see much benefit in exposing cfi.pflash01 with a naked -device *now*. Re: virtio-mmio, I don't think that should be available on Q35 at all. I can't comment on the rest. Hi Eduardo, Thanks for finding these problems. We should ping all maintainers of the above devices, the best way to do it is to add the "cannot_instantiate_with_device_add_yet = true" and ask maintainers to agree (or not) on that. Thanks, Marcel Thanks Laszlo Should we make q35 validate dynamic sysbus devices against a whitelist, like the other has_dynamic_sysbus machines? I'm asking this because I will resume work on the "query-device-slots" command, which will report supported sysbus devices too. And I don't want the new command to report any devices that it shouldn't.
Re: [Qemu-devel] [PATCH 35/51] ram: Add QEMUFile to RAMState
* Juan Quintela (quint...@redhat.com) wrote:
> Signed-off-by: Juan Quintela
> ---
> migration/ram.c | 17 ++---
> 1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/migration/ram.c b/migration/ram.c
> index c0d6841..7667e73 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -165,6 +165,8 @@ struct RAMSrcPageRequest {
>
> /* State of RAM for migration */
> struct RAMState {
> +/* QEMUFile used for this migration */
> +QEMUFile *f;
Yes, I guess you're hoping that becomes 'for this RAMBlock' eventually?
Reviewed-by: Dr. David Alan Gilbert
> /* Last block that we have visited searching for dirty pages */
> RAMBlock *last_seen_block;
> /* Last block from where we have sent data */
> @@ -524,14 +526,13 @@ static void xbzrle_cache_zero_page(RAMState *rs,
> ram_addr_t current_addr)
> * -1 means that xbzrle would be longer than normal
> *
> * @rs: current RAM state
> - * @f: QEMUFile where to send the data
> * @current_data: contents of the page
> * @current_addr: addr of the page
> * @block: block that contains the page we want to send
> * @offset: offset inside the block for the page
> * @last_stage: if we are at the completion stage
> */
> -static int save_xbzrle_page(RAMState *rs, QEMUFile *f, uint8_t
> **current_data,
> +static int save_xbzrle_page(RAMState *rs, uint8_t **current_data,
> ram_addr_t current_addr, RAMBlock *block,
> ram_addr_t offset, bool last_stage)
> {
> @@ -582,10 +583,11 @@ static int save_xbzrle_page(RAMState *rs, QEMUFile *f,
> uint8_t **current_data,
> }
>
> /* Send XBZRLE based compressed page */
> -bytes_xbzrle = save_page_header(f, block, offset | RAM_SAVE_FLAG_XBZRLE);
> -qemu_put_byte(f, ENCODING_FLAG_XBZRLE);
> -qemu_put_be16(f, encoded_len);
> -qemu_put_buffer(f, XBZRLE.encoded_buf, encoded_len);
> +bytes_xbzrle = save_page_header(rs->f, block,
> +offset | RAM_SAVE_FLAG_XBZRLE);
> +qemu_put_byte(rs->f, ENCODING_FLAG_XBZRLE);
> +qemu_put_be16(rs->f, encoded_len);
> +qemu_put_buffer(rs->f, XBZRLE.encoded_buf, encoded_len);
> bytes_xbzrle += encoded_len + 1 + 2;
> rs->xbzrle_pages++;
> rs->xbzrle_bytes += bytes_xbzrle;
> @@ -849,7 +851,7 @@ static int ram_save_page(RAMState *rs, MigrationState
> *ms, QEMUFile *f,
> ram_release_pages(ms, block->idstr, pss->offset, pages);
> } else if (!rs->ram_bulk_stage &&
> !migration_in_postcopy(ms) && migrate_use_xbzrle()) {
> -pages = save_xbzrle_page(rs, f, &p, current_addr, block,
> +pages = save_xbzrle_page(rs, &p, current_addr, block,
> offset, last_stage);
> if (!last_stage) {
> /* Can't send this cached data async, since the cache page
> @@ -2087,6 +2089,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
> return -1;
> }
> }
> +rs->f = f;
>
> rcu_read_lock();
>
> --
> 2.9.3
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
[Qemu-devel] qemu-system-s390x tests/boot-serial-test intermittent failure
Hi; qemu-system-s390x seems to have an intermittent failure at
the moment -- it's been causing our Travis builds to flap. I actually
caught it doing this on one of my local test builds (which happens
to be aarch64 but I don't think that matters, since Travis is doing
x86 builds):
while QTEST_QEMU_BINARY=s390x-softmmu/qemu-system-s390x
QTEST_QEMU_IMG=qemu-img MALLOC_PERTURB_=${MALLOC_PERTURB_:-$((RANDOM %
255 + 1))} gtester -k --verbose -m=quick tests/boot-serial-test ; do
true; done
TEST: tests/boot-serial-test... (pid=1122)
/s390x/boot-serial/s390-ccw-virtio: OK
PASS: tests/boot-serial-test
TEST: tests/boot-serial-test... (pid=1135)
/s390x/boot-serial/s390-ccw-virtio: OK
[skip lots more successes]
TEST: tests/boot-serial-test... (pid=1582)
/s390x/boot-serial/s390-ccw-virtio:
Broken pipe
FAIL
GTester: last random seed: R02Se94f36f305f2edd8391a22749ec91143
(pid=1635)
FAIL: tests/boot-serial-test
Any ideas?
thanks
-- PMM
Re: [Qemu-devel] [PATCH v3 13/34] tcg: Add atomic helpers
Nikunj A Dadhania writes:
> Richard Henderson writes:
>
>> On 09/12/2016 06:47 AM, Alex Bennée wrote:
> +/* Notice an IO access, or a notdirty page. */
> +if (unlikely(tlb_addr & ~TARGET_PAGE_MASK)) {
> +/* There's really nothing that can be done to
> + support this apart from stop-the-world. */
> +goto stop_the_world;
>>> We are also triggering on TLB_NOTDIRTY here in the case where a
>>> conditional write is the first write to a page. I don't know if a
>>> stop_the_world is required at this point but we will need to ensure we
>>> clear bits as notdirty_mem_write() does.
>>>
>>
>> You're quite right that we could probably special-case TLB_NOTDIRTY here such
>> that (1) we needn't leave the cpu loop, and (2) needn't utilize the actual
>> "write" part of notdirty_mem_write; just set the bits then fall through to
>> the
>> actual atomic instruction below.
>
> I do hit this case with ppc64, where I see that its the first write to
> the page and it exits from this every time, causing the kernel to print
> soft-lockups.
>
> Can we add the special case here for NOTDIRTY and set the page as dirty
> and return successfully?
Does the atomic step fall-back not work for you?
--
Alex Bennée
Re: [Qemu-devel] [RFC 1/4] [UNTESTED] xen: Don't force has_dynamic_sysbus on machine class
On Fri, Mar 24, 2017 at 11:24:31AM +0100, Juergen Gross wrote:
> On 24/03/17 11:09, Peter Maydell wrote:
> > On 24 March 2017 at 08:23, Juergen Gross wrote:
> >> On 23/03/17 22:28, Eduardo Habkost wrote:
> >>> The xen-backend devices created by the Xen code are not supposed
> >>> to be treated as dynamic sysbus devices. This is an attempt to
> >>> change that and see what happens, but I couldn't test it because
> >>> I don't have a Xen host set up.
> >>>
> >>> If this patch breaks anything, this means we have a bug in
> >>> foreach_dynamic_sysbus_device(), which is supposed to return only
> >>> devices created using -device.
> >>>
> >>> The original code that sets has_dynamic_sysbus was added by
> >>> commit 3a6c9172ac5951e6dac2b3f6cbce3cfccdec5894, but I don't see
> >>> any comment explaining why it was necessary.
> >>
> >> xen-backend devices are created via qmp commands when attaching new
> >> pv-devices to a domain. They can be dynamically removed, too. Setting
> >> has_dynamic_sysbus was necessary to support this feature.
> >
> > This seems like it ought to be handled by marking the xen-backend
> > devices as being ok-to-dynamically-create somehow, not by marking
> > the machine as supporting dynamic-sysbus (which it doesn't).
> > Maybe we don't have the necessary support code to do that though?
>
> When writing the patches I couldn't find a way to do it differently.
> OTOH I'm not so deep in qemu internals I'd be able to add the needed
> support.
>
> I'd be happy to test any patch, though.
If xen-backend devices are created via QMP commands, then
has_dynamic_sysbus is (currently) really needed, although I would
have preferred to set it on all x86 machines instead of changing
MachineClass::has_dynamic_sysbus outside class_init.
But with the new whitelist implemented by this series, we could
simply include xen-backend in the whitelist for the machines that
can be used with Xen, and get rid of xen_set_dynamic_sysbus().
I assume plugging/unplugging xen-backend devices apply to both
xen{pv,fv} and pc,accel=xen, right? Do we need to make it work
with "-machine none,accel=xen" and "-machine isapc" too?
--
Eduardo
Re: [Qemu-devel] qemu-system-s390x tests/boot-serial-test intermittent failure
On 03/24/2017 11:57 AM, Peter Maydell wrote:
> Hi; qemu-system-s390x seems to have an intermittent failure at
> the moment -- it's been causing our Travis builds to flap. I actually
> caught it doing this on one of my local test builds (which happens
> to be aarch64 but I don't think that matters, since Travis is doing
> x86 builds):
>
> while QTEST_QEMU_BINARY=s390x-softmmu/qemu-system-s390x
> QTEST_QEMU_IMG=qemu-img MALLOC_PERTURB_=${MALLOC_PERTURB_:-$((RANDOM %
> 255 + 1))} gtester -k --verbose -m=quick tests/boot-serial-test ; do
> true; done
> TEST: tests/boot-serial-test... (pid=1122)
> /s390x/boot-serial/s390-ccw-virtio: OK
> PASS: tests/boot-serial-test
> TEST: tests/boot-serial-test... (pid=1135)
> /s390x/boot-serial/s390-ccw-virtio: OK
> [skip lots more successes]
> TEST: tests/boot-serial-test... (pid=1582)
> /s390x/boot-serial/s390-ccw-virtio:
> Broken pipe
> FAIL
> GTester: last random seed: R02Se94f36f305f2edd8391a22749ec91143
> (pid=1635)
> FAIL: tests/boot-serial-test
>
> Any ideas?
> thanks
Adding Thomas who did the s390 version.
One idea. Maybe qemu exits before the other side is ready.
Does reverting
commit 864111f422babcf8ce837fb47f7f9e1948446f22
Author: Christian Borntraeger
AuthorDate: Tue Oct 18 09:29:54 2016 +0200
Commit: Paolo Bonzini
CommitDate: Wed Nov 2 09:28:56 2016 +0100
vl: exit qemu on guest panic if -no-shutdown is not set
help?
If yes, does
diff --git a/tests/boot-serial-test.c b/tests/boot-serial-test.c
index 57edf6a..11f48b0 100644
--- a/tests/boot-serial-test.c
+++ b/tests/boot-serial-test.c
@@ -79,8 +79,8 @@ static void test_machine(const void *data)
g_assert(fd != -1);
args = g_strdup_printf("-M %s,accel=tcg -chardev file,id=serial0,path=%s"
- " -serial chardev:serial0 %s", test->machine,
- tmpname, test->extra);
+ " -no-shutdown -serial chardev:serial0 %s",
+ test->machine, tmpname, test->extra);
qtest_start(args);
unlink(tmpname);
also help?
Re: [Qemu-devel] [PATCH 02/51] ram: rename block_name to rbname
* Juan Quintela (quint...@redhat.com) wrote:
> So all places are consisten on the nambing of a block name parameter.
>
> Signed-off-by: Juan Quintela
Reviewed-by: Dr. David Alan Gilbert
> ---
> migration/ram.c | 17 -
> 1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/migration/ram.c b/migration/ram.c
> index 76f1fc4..21047c5 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -743,14 +743,14 @@ static int save_zero_page(QEMUFile *f, RAMBlock *block,
> ram_addr_t offset,
> return pages;
> }
>
> -static void ram_release_pages(MigrationState *ms, const char *block_name,
> +static void ram_release_pages(MigrationState *ms, const char *rbname,
>uint64_t offset, int pages)
> {
> if (!migrate_release_ram() || !migration_in_postcopy(ms)) {
> return;
> }
>
> -ram_discard_range(NULL, block_name, offset, pages << TARGET_PAGE_BITS);
> +ram_discard_range(NULL, rbname, offset, pages << TARGET_PAGE_BITS);
> }
>
> /**
> @@ -1942,25 +1942,24 @@ int ram_postcopy_send_discard_bitmap(MigrationState
> *ms)
> * Returns zero on success
> *
> * @mis: current migration incoming state
> - * @block_name: Name of the RAMBLock of the request. NULL means the
> - * same that last one.
> + * @rbname: name of the RAMBLock of the request. NULL means the
> + * same that last one.
> * @start: RAMBlock starting page
> * @length: RAMBlock size
> */
> int ram_discard_range(MigrationIncomingState *mis,
> - const char *block_name,
> + const char *rbname,
>uint64_t start, size_t length)
> {
> int ret = -1;
>
> -trace_ram_discard_range(block_name, start, length);
> +trace_ram_discard_range(rbname, start, length);
>
> rcu_read_lock();
> -RAMBlock *rb = qemu_ram_block_by_name(block_name);
> +RAMBlock *rb = qemu_ram_block_by_name(rbname);
>
> if (!rb) {
> -error_report("ram_discard_range: Failed to find block '%s'",
> - block_name);
> +error_report("ram_discard_range: Failed to find block '%s'", rbname);
> goto err;
> }
>
> --
> 2.9.3
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH 35/51] ram: Add QEMUFile to RAMState
"Dr. David Alan Gilbert" wrote:
> * Juan Quintela (quint...@redhat.com) wrote:
>> Signed-off-by: Juan Quintela
>> ---
>> migration/ram.c | 17 ++---
>> 1 file changed, 10 insertions(+), 7 deletions(-)
>>
>> diff --git a/migration/ram.c b/migration/ram.c
>> index c0d6841..7667e73 100644
>> --- a/migration/ram.c
>> +++ b/migration/ram.c
>> @@ -165,6 +165,8 @@ struct RAMSrcPageRequest {
>>
>> /* State of RAM for migration */
>> struct RAMState {
>> +/* QEMUFile used for this migration */
>> +QEMUFile *f;
>
> Yes, I guess you're hoping that becomes 'for this RAMBlock' eventually?
For this ramblock or for this migration. For some reason that I can't
yet fully understand people continues asking about starting a new migration
before the previous one has finished. No, I haven't a good explanation
for why that could be a good idea.
Later, Juan.
>
> Reviewed-by: Dr. David Alan Gilbert
>
>> /* Last block that we have visited searching for dirty pages */
>> RAMBlock *last_seen_block;
>> /* Last block from where we have sent data */
>> @@ -524,14 +526,13 @@ static void xbzrle_cache_zero_page(RAMState *rs,
>> ram_addr_t current_addr)
>> * -1 means that xbzrle would be longer than normal
>> *
>> * @rs: current RAM state
>> - * @f: QEMUFile where to send the data
>> * @current_data: contents of the page
>> * @current_addr: addr of the page
>> * @block: block that contains the page we want to send
>> * @offset: offset inside the block for the page
>> * @last_stage: if we are at the completion stage
>> */
>> -static int save_xbzrle_page(RAMState *rs, QEMUFile *f, uint8_t
>> **current_data,
>> +static int save_xbzrle_page(RAMState *rs, uint8_t **current_data,
>> ram_addr_t current_addr, RAMBlock *block,
>> ram_addr_t offset, bool last_stage)
>> {
>> @@ -582,10 +583,11 @@ static int save_xbzrle_page(RAMState *rs, QEMUFile *f,
>> uint8_t **current_data,
>> }
>>
>> /* Send XBZRLE based compressed page */
>> -bytes_xbzrle = save_page_header(f, block, offset |
>> RAM_SAVE_FLAG_XBZRLE);
>> -qemu_put_byte(f, ENCODING_FLAG_XBZRLE);
>> -qemu_put_be16(f, encoded_len);
>> -qemu_put_buffer(f, XBZRLE.encoded_buf, encoded_len);
>> +bytes_xbzrle = save_page_header(rs->f, block,
>> +offset | RAM_SAVE_FLAG_XBZRLE);
>> +qemu_put_byte(rs->f, ENCODING_FLAG_XBZRLE);
>> +qemu_put_be16(rs->f, encoded_len);
>> +qemu_put_buffer(rs->f, XBZRLE.encoded_buf, encoded_len);
>> bytes_xbzrle += encoded_len + 1 + 2;
>> rs->xbzrle_pages++;
>> rs->xbzrle_bytes += bytes_xbzrle;
>> @@ -849,7 +851,7 @@ static int ram_save_page(RAMState *rs, MigrationState
>> *ms, QEMUFile *f,
>> ram_release_pages(ms, block->idstr, pss->offset, pages);
>> } else if (!rs->ram_bulk_stage &&
>> !migration_in_postcopy(ms) && migrate_use_xbzrle()) {
>> -pages = save_xbzrle_page(rs, f, &p, current_addr, block,
>> +pages = save_xbzrle_page(rs, &p, current_addr, block,
>> offset, last_stage);
>> if (!last_stage) {
>> /* Can't send this cached data async, since the cache page
>> @@ -2087,6 +2089,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
>> return -1;
>> }
>> }
>> +rs->f = f;
>>
>> rcu_read_lock();
>>
>> --
>> 2.9.3
>>
> --
> Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH for-2.9] disas/microblaze: Remove unused REG_PC define
On 23 March 2017 at 19:41, Edgar E. Iglesias wrote: > On Thu, Mar 23, 2017 at 12:42:41PM +, Peter Maydell wrote: >> The REG_PC define in disas/microblaze.c clashes with a define in >> the Linux SPARC system headers: >> >> /home/pm215/qemu/disas/microblaze.c:162:0: error: "REG_PC" redefined >> [-Werror] >> #define REG_PC 32 /* PC */ >> >> In file included from /usr/include/signal.h:326:0, >> from /home/pm215/qemu/include/qemu/osdep.h:86, >> from /home/pm215/qemu/disas/microblaze.c:36: >> /usr/include/sparc64-linux-gnu/sys/ucontext.h:96:0: note: this is the >> location of the previous definition >> #define REG_PC (1) >> >> Since the code doesn't actually use the REG_PC define >> anywhere, the simplest fix is just to remove it. > > > Reviewed-by: Edgar E. Iglesias Thanks; applied to master. -- PMM
Re: [Qemu-devel] qemu-system-s390x tests/boot-serial-test intermittent failure
On 24.03.2017 12:11, Christian Borntraeger wrote:
> On 03/24/2017 11:57 AM, Peter Maydell wrote:
>> Hi; qemu-system-s390x seems to have an intermittent failure at
>> the moment -- it's been causing our Travis builds to flap. I actually
>> caught it doing this on one of my local test builds (which happens
>> to be aarch64 but I don't think that matters, since Travis is doing
>> x86 builds):
>>
>> while QTEST_QEMU_BINARY=s390x-softmmu/qemu-system-s390x
>> QTEST_QEMU_IMG=qemu-img MALLOC_PERTURB_=${MALLOC_PERTURB_:-$((RANDOM %
>> 255 + 1))} gtester -k --verbose -m=quick tests/boot-serial-test ; do
>> true; done
>> TEST: tests/boot-serial-test... (pid=1122)
>> /s390x/boot-serial/s390-ccw-virtio: OK
>> PASS: tests/boot-serial-test
>> TEST: tests/boot-serial-test... (pid=1135)
>> /s390x/boot-serial/s390-ccw-virtio: OK
>> [skip lots more successes]
>> TEST: tests/boot-serial-test... (pid=1582)
>> /s390x/boot-serial/s390-ccw-virtio:
>> Broken pipe
>> FAIL
>> GTester: last random seed: R02Se94f36f305f2edd8391a22749ec91143
>> (pid=1635)
>> FAIL: tests/boot-serial-test
>>
>> Any ideas?
I was not able to reproduce this issue so far (on my x86 laptop since I
don't have an aarch64 host) ... can you reproduce it by running the test
directly, too, e.g. something like:
QTEST_QEMU_BINARY=s390x-softmmu/qemu-system-s390x tests/boot-serial-test
?
> Adding Thomas who did the s390 version.
>
> One idea. Maybe qemu exits before the other side is ready.
Or could this be a timeout issue again? Is the host very loaded?
(however, I don't really believe that this could be the issue here,
since the test is rather fast)
Thomas
Re: [Qemu-devel] qemu-system-s390x tests/boot-serial-test intermittent failure
On 24 March 2017 at 11:26, Thomas Huth wrote: > I was not able to reproduce this issue so far (on my x86 laptop since I > don't have an aarch64 host) ... can you reproduce it by running the test > directly, too, e.g. something like: > > QTEST_QEMU_BINARY=s390x-softmmu/qemu-system-s390x tests/boot-serial-test > > ? Yes, you can. >> Adding Thomas who did the s390 version. >> >> One idea. Maybe qemu exits before the other side is ready. > > Or could this be a timeout issue again? Is the host very loaded? > (however, I don't really believe that this could be the issue here, > since the test is rather fast) The timeout is 60 seconds, right? When the test fails it fails immediately, not after hanging for 60s. thanks -- PMM
Re: [Qemu-devel] q35 and sysbus devices
On 22.03.2017 21:31, Eduardo Habkost wrote: > Hi, > > I am investigating the current status of has_dynamic_sysbus and > sysbus device support on each of QEMU's machine types. The good > news is that almost all has_dynamic_sysbus=1 machines have their > own internal (often short) whitelist of supported sysbus device > types, and automatically reject unsupported devices. > > ...except for q35. > > q35 currently accepts all sys-bus-device subtypes on "-device", > and today this includes the following 23 devices: > > * allwinner-ahci > * amd-iommu > * cfi.pflash01 > * esp > * fw_cfg_io > * fw_cfg_mem > * generic-sdhci > * hpet > * intel-iommu > * ioapic > * isabus-bridge > * kvmclock > * kvm-ioapic > * kvmvapic > * SUNW,fdtwo I think that SUNW,fdtwo device only makes sense on the sun4m machine, so the code should likely be extracted from fdc.c into a separate file, which should then only be compiled if CONFIG_SUN4M is set. Thomas > * sysbus-ahci > * sysbus-fdc > * sysbus-ohci > * unimplemented-device > * virtio-mmio > * xen-backend > * xen-sysdev > > My question is: do all those devices really make sense to be used > with "-device" on q35? Should we make q35 validate dynamic sysbus > devices against a whitelist, like the other has_dynamic_sysbus > machines? > > I'm asking this because I will resume work on the > "query-device-slots" command, which will report supported sysbus > devices too. And I don't want the new command to report any > devices that it shouldn't. >
Re: [Qemu-devel] [PATCH 01/51] ram: Update all functions comments
Peter Xu wrote:
> Hi, Juan,
>
> Got several nitpicks below... (along with some questions)
>
> On Thu, Mar 23, 2017 at 09:44:54PM +0100, Juan Quintela wrote:
>
> [...]
>
>> static void xbzrle_cache_zero_page(ram_addr_t current_addr)
>> {
>> @@ -459,8 +474,8 @@ static void xbzrle_cache_zero_page(ram_addr_t
>> current_addr)
>> * -1 means that xbzrle would be longer than normal
>> *
>> * @f: QEMUFile where to send the data
>> - * @current_data:
>> - * @current_addr:
>> + * @current_data: contents of the page
>
> Since current_data is a double pointer, so... maybe "pointer to the
> address of page content"?
ok. changed.
> Btw, a question not related to this series... Why here in
> save_xbzrle_page() we need to update *current_data to be the newly
> created page cache? I see that we have:
>
> /* update *current_data when the page has been
>inserted into cache */
> *current_data = get_cached_data(XBZRLE.cache, current_addr);
>
> What would be the difference if we just use the old pointer in
> RAMBlock.host?
Its contents could have been changed since we inserted it into the
cache. Then we could end having "memory corruption" during transfer.
> [...]
>
>> @@ -1157,11 +1186,12 @@ static bool get_queued_page(MigrationState
>> *ms, PageSearchStatus *pss,
>> }
>>
>> /**
>> - * flush_page_queue: Flush any remaining pages in the ram request queue
>> - *it should be empty at the end anyway, but in error cases there may be
>> - *some left.
>> + * flush_page_queue: flush any remaining pages in the ram request queue
>
> Here the comment says (just like mentioned in function name) that we
> will "flush any remaining pages in the ram request queue", however in
> the implementation, we should be only freeing everything in
> src_page_requests. The problem is "flush" let me think about "flushing
> the rest of the pages to the other side"... while it's not.
>
> Would it be nice we just rename the function into something else, like
> migration_page_queue_free()? We can tune the comments correspondingly
> as well.
I will let this one to dave to answer O:-)
I agree than previous name is not perfect, but not sure that the new one
is mucth better either.
migration_drop_page_queue()?
>
> [...]
>
>> -/*
>> - * Helper for postcopy_chunk_hostpages; it's called twice to cleanup
>> - * the two bitmaps, that are similar, but one is inverted.
>> +/**
>> + * postcopy_chuck_hostpages_pass: canocalize bitmap in hostpages
> ^ should be n? ^^ canonicalize?
Fixed.
>> - * We search for runs of target-pages that don't start or end on a
>> - * host page boundary;
>> - * unsent_pass=true: Cleans up partially unsent host pages by searching
>> - * the unsentmap
>> - * unsent_pass=false: Cleans up partially dirty host pages by searching
>> - * the main migration bitmap
>> + * Helper for postcopy_chunk_hostpages; it's called twice to
>> + * canonicalize the two bitmaps, that are similar, but one is
>> + * inverted.
>> *
>> + * Postcopy requires that all target pages in a hostpage are dirty or
>> + * clean, not a mix. This function canonicalizes the bitmaps.
>> + *
>> + * @ms: current migration state
>> + * @unsent_pass: if true we need to canonicalize partially unsent host pages
>> + * otherwise we need to canonicalize partially dirty host
>> pages
>> + * @block: block that contains the page we want to canonicalize
>> + * @pds: state for postcopy
>> */
>> static void postcopy_chunk_hostpages_pass(MigrationState *ms, bool
>> unsent_pass,
>>RAMBlock *block,
>
> [...]
>
>> +/**
>> + * ram_save_setup: iterative stage for migration
> ^^ should be ram_save_iterate()?
fixed. Too much copy and paste.
>
>> + *
>> + * Returns zero to indicate success and negative for error
>> + *
>> + * @f: QEMUFile where to send the data
>> + * @opaque: RAMState pointer
>> + */
>> static int ram_save_iterate(QEMUFile *f, void *opaque)
>> {
>> int ret;
>> @@ -2091,7 +2169,16 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
>> return done;
>> }
>
> [...]
>
>> -/*
>> - * Allocate data structures etc needed by incoming migration with
>> postcopy-ram
>> - * postcopy-ram's similarly names postcopy_ram_incoming_init does the work
>> +/**
>> + * ram_postococpy_incoming_init: allocate postcopy data structures
>> + *
>> + * Returns 0 for success and negative if there was one error
>> + *
>> + * @mis: current migration incoming state
>> + *
>> + * Allocate data structures etc needed by incoming migration with
>> + * postcopy-ram postcopy-ram's similarly names
>> + * postcopy_ram_incoming_init does the work
>
> This sentence is slightly hard to understand... But I think the
> function name explained itself enough though. :)
I didn't want to remove Dave comments at this point, jusnt doing the
formating8 and put them consintent. I agree that this file comments
could
Re: [Qemu-devel] qemu-system-s390x tests/boot-serial-test intermittent failure
On 24/03/2017 12:11, Christian Borntraeger wrote:
> One idea. Maybe qemu exits before the other side is ready.
> Does reverting
>
> commit 864111f422babcf8ce837fb47f7f9e1948446f22
> Author: Christian Borntraeger
> AuthorDate: Tue Oct 18 09:29:54 2016 +0200
> Commit: Paolo Bonzini
> CommitDate: Wed Nov 2 09:28:56 2016 +0100
>
> vl: exit qemu on guest panic if -no-shutdown is not set
>
> help?
Didn't test, but this:
> If yes, does
> diff --git a/tests/boot-serial-test.c b/tests/boot-serial-test.c
> index 57edf6a..11f48b0 100644
> --- a/tests/boot-serial-test.c
> +++ b/tests/boot-serial-test.c
> @@ -79,8 +79,8 @@ static void test_machine(const void *data)
> g_assert(fd != -1);
>
> args = g_strdup_printf("-M %s,accel=tcg -chardev file,id=serial0,path=%s"
> - " -serial chardev:serial0 %s", test->machine,
> - tmpname, test->extra);
> + " -no-shutdown -serial chardev:serial0 %s",
> + test->machine, tmpname, test->extra);
>
> qtest_start(args);
> unlink(tmpname);
>
> also help?
seems to help (survives about 1 minute, while usually it fails in a few
seconds).
Paolo
Re: [Qemu-devel] qemu-system-s390x tests/boot-serial-test intermittent failure
On 24 March 2017 at 11:11, Christian Borntraeger wrote:
> One idea. Maybe qemu exits before the other side is ready.
> Does reverting
>
> commit 864111f422babcf8ce837fb47f7f9e1948446f22
> Author: Christian Borntraeger
> AuthorDate: Tue Oct 18 09:29:54 2016 +0200
> Commit: Paolo Bonzini
> CommitDate: Wed Nov 2 09:28:56 2016 +0100
>
> vl: exit qemu on guest panic if -no-shutdown is not set
>
> help?
I didn't test this...
> If yes, does
> diff --git a/tests/boot-serial-test.c b/tests/boot-serial-test.c
> index 57edf6a..11f48b0 100644
> --- a/tests/boot-serial-test.c
> +++ b/tests/boot-serial-test.c
> @@ -79,8 +79,8 @@ static void test_machine(const void *data)
> g_assert(fd != -1);
>
> args = g_strdup_printf("-M %s,accel=tcg -chardev file,id=serial0,path=%s"
> - " -serial chardev:serial0 %s", test->machine,
> - tmpname, test->extra);
> + " -no-shutdown -serial chardev:serial0 %s",
> + test->machine, tmpname, test->extra);
...but this doesn't help.
thanks
-- PMM
Re: [Qemu-devel] .PO files modified under build
* James Hanley (jhan...@dgtlrift.com) wrote: > I have a git clone of qemu, and I build out of qemu-build... essentially > the layout looks like: > ./ > ./qemu/ -> clone of qemu > ./qemu-build/ > ./Makefile > > The contents of the top level Makefile contain the following rules: > rwildcard=$(wildcard $1$2) $(foreach d,$(wildcard $1*),$(call > rwildcard,$d/,$2)) > files := $(call rwildcard,qemu/,*.[ch]) > > .PHONY: clean_orphan_artifacts > clean_orphan_artifacts: $(note update) > $(Q)\ > $(GIT_VERBOSE) ; \ > cd qemu && \ > git clean -f -d -X > $(call trace,GIT CLEAN) > > qemu-build: qemu $(files) clean_orphan_artifacts > $(Q)\ > mkdir -p $(@) && \ > cd $(@) && \ > ../qemu/configure \ > --prefix=$(current_path)/local \ > --target-list=arm-softmmu \ > --disable-werror&& \ > $(MAKE) && \ > $(MAKE) install \ > $(call trace,BUILD) > > It seems that somehow the .po files are being modified under the qemu repo > even though the entire build is out of ./qemu-build. > > The two questions are: >Should the .po files be modified under the qemu path when building out > of another (ie qemu-build)? >Should the .po files continue to be versioned or placed in .gitignore if > they are (apparently) generated? It's a known problem we've had for ages, and IMHO really needs fixing. Dave -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [Qemu-ppc] [Bug 1674925] Re: Qemu PPC64 kvm no display if --device virtio-gpu-pci is selected
Hi Cèdric, first of all thanks for your relpy. >I have some difficulty sorting out what is going on and what >could be considered a regression :/ you are reporting many >issues at the same time with a home made kernel. >Could you please use the kernel shipped with the distro to >start with ? I can do it and report. > yes usually with 2.8 i boot the VM without issue on G5 Quad with the option > -M pseries from 2.1 to 2.5 with kvm-pr enabled. > i did the tests and with all pseries now on 2.9 i have the same issue. > example: > qemu-system-ppc64 --enable-kvm -cpu 970fx_v2.0 -m 1024 -M pseries-2.1 > qemu-system-ppc64: KVM and IRQ_XICS capability must be present for in-kernel > XICS >This error message is because your host kernel lacks in-kernel XICS, >but you are saying that was not an issue with QEMU-2.8. Correct ? Exactly i have the same on Qoriq too. >Here is the command line I used on a 17.04 host : >qemu-system-ppc64 -M pseries-2.[1-8],accel=kvm,kvm-type=PR -cpu 970fx_v2.0 -m >1024 -nographic I will try your same command line and see what will exit on me. I cant use qemu on 17.04 host because there is no more support for PPC32, PPC64 dead line is 16.10 and last working version of qemu on PPC is 2.6.1 i dint try 2.9 there if needed i can do i have ubuntu mate 17.04 installed too. >Did we introduce a regression in compatibility in QEMU 2.9 ? or Im facing many issue on this last update im try to help how i can before all come upstream. i like really much qemu. i can help in testing on PPC64 Be if need with my hw. >was it bogus before ? That needs a little digging. I did not work >on that part. dont worry you did much Thanks Luigi -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1674925 Title: Qemu PPC64 kvm no display if --device virtio-gpu-pci is selected Status in QEMU: New Bug description: Hi, i did many tests on qemu 2.8 on my BE machines and i found an issue that i think was need to be reported Test Machines BE 970MP if i setup qemu with qemu-system-ppc64 -M 1024 --display sdl(or gtk),gl=on --device virtio- gpu-pci,virgl --enable-kvm and so and so result is doubled window one is vga other is virtio-gpu-pci without any start of the VM . pratically i dont have any output of openbios and on the virtual serial output the same issue i found is if i select: qemu-system-ppc64 -M 1024 --display gtk(or sdl) --device virtio-gpu-pci --enable-kvm and so and so i had been try to change all the -M types of all kind of pseries without any positive result. Ciao Luigi To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1674925/+subscriptions
Re: [Qemu-devel] [PATCH for-2.10 15/23] QMP: include CpuInstanceProperties into query_cpus output output
On Thu, 23 Mar 2017 08:19:24 -0500
Eric Blake wrote:
> On 03/22/2017 08:32 AM, Igor Mammedov wrote:
> > if board supports CpuInstanceProperties, report them for
> > each CPU thread listed. Main motivation for this is to
> > provide these properties introspection via QMP interface
> > for using in test cases to verify numa node to cpu mapping,
> > which includes not only boards that support cpu hotplug
> > and have this info in query-hotpluggable-cpus (pc/spapr)
> > but also for boards that don't not support hotpluggable-cpus
> > but support numa mapping (virt-arm).
> >
> > Signed-off-by: Igor Mammedov
> > ---
>
> > @@ -1860,6 +1863,12 @@ CpuInfoList *qmp_query_cpus(Error **errp)
> > #else
> > info->value->arch = CPU_INFO_ARCH_OTHER;
> > #endif
> > +if ((info->value->has_props = !!mc->cpu_index_to_instance_props))
> > {
>
> checkpatch.pl doesn't flag that? We generally try to avoid side-effects
> inside conditionals.
it does, fixed in v2 branch
(lazy me skipped checkpatch since QMP test case been added,
I've also fixed another checkpatch error in the next patch)
> > +CpuInstanceProperties *props;
> > +props = g_malloc0(sizeof(*props));
> > +*props = mc->cpu_index_to_instance_props(ms, cpu->cpu_index);
> > +info->value->props = props;
>
> Why two spaces after =?
fixed
>
> With those cleaned up,
> Reviewed-by: Eric Blake
Thanks!
Re: [Qemu-devel] [RFC 1/4] [UNTESTED] xen: Don't force has_dynamic_sysbus on machine class
On 24/03/17 12:10, Eduardo Habkost wrote:
> On Fri, Mar 24, 2017 at 11:24:31AM +0100, Juergen Gross wrote:
>> On 24/03/17 11:09, Peter Maydell wrote:
>>> On 24 March 2017 at 08:23, Juergen Gross wrote:
On 23/03/17 22:28, Eduardo Habkost wrote:
> The xen-backend devices created by the Xen code are not supposed
> to be treated as dynamic sysbus devices. This is an attempt to
> change that and see what happens, but I couldn't test it because
> I don't have a Xen host set up.
>
> If this patch breaks anything, this means we have a bug in
> foreach_dynamic_sysbus_device(), which is supposed to return only
> devices created using -device.
>
> The original code that sets has_dynamic_sysbus was added by
> commit 3a6c9172ac5951e6dac2b3f6cbce3cfccdec5894, but I don't see
> any comment explaining why it was necessary.
xen-backend devices are created via qmp commands when attaching new
pv-devices to a domain. They can be dynamically removed, too. Setting
has_dynamic_sysbus was necessary to support this feature.
>>>
>>> This seems like it ought to be handled by marking the xen-backend
>>> devices as being ok-to-dynamically-create somehow, not by marking
>>> the machine as supporting dynamic-sysbus (which it doesn't).
>>> Maybe we don't have the necessary support code to do that though?
>>
>> When writing the patches I couldn't find a way to do it differently.
>> OTOH I'm not so deep in qemu internals I'd be able to add the needed
>> support.
>>
>> I'd be happy to test any patch, though.
>
> If xen-backend devices are created via QMP commands, then
> has_dynamic_sysbus is (currently) really needed, although I would
> have preferred to set it on all x86 machines instead of changing
> MachineClass::has_dynamic_sysbus outside class_init.
>
> But with the new whitelist implemented by this series, we could
> simply include xen-backend in the whitelist for the machines that
> can be used with Xen, and get rid of xen_set_dynamic_sysbus().
>
> I assume plugging/unplugging xen-backend devices apply to both
> xen{pv,fv} and pc,accel=xen, right? Do we need to make it work
> with "-machine none,accel=xen" and "-machine isapc" too?
AFAIK -xenpv, -xenfv and -pc,accel=xen are the only machine types
to support. Wouldn't it make sense to do the whitelisting in
xen_be_register_common() in spite of setting has_dynamic_sysbus?
Juergen
[Qemu-devel] Making QMP 'block-job-cancel' transactionable
While debugging some other issue, I happened to stumble across an old libvirt commit[*] that adds support for pivot (whether QEMU should switch to a target copy or not) operation as a result of issuing QMP 'block-job-cancel' to a 'drive-mirror' (in libvirt parlance, "block copy"). In the libvirt commit message[*] Eric Blake writes: "[...] There may be potential improvements to the snapshot code to exploit block copy over multiple disks all at one point in time. And, if 'block-job-cancel' were made part of 'transaction', you could copy multiple disks at the same point in time without pausing the domain. [...]" I realize that 'block-job-cancel' is currently not part of the @TransactionAction. Is it worthwhile to do so? Given the current behavior of QMP 'drive-mirror': - Upon 'block-job-complete', synchronization will end, and live QEMU pivots to the target (i.e. the copy) - Upon 'block-job-cancel', a point-in-time (at the time of cancel) copy gets created, and live QEMU will _not_ pivot. I realize that it is not possible to perform a "block copy" of multiple disks at the same point in time without pausing QEMU. From a brief chat with Stefan Hajnoczi on IRC, he does confirm that there's no current API for doing it atomically across multiple disks. Since Stefan asked if a bug exists for adding 'transaction' support to 'block-job-cancel', thought I might bring it up here first. [*] https://libvirt.org/git/?p=libvirt.git;a=commit;h=eaba79d -- blockjob: support pivot operation on cancel -- /kashyap
Re: [Qemu-devel] [PATCH for-2.9 4/5] rbd: Peel off redundant RbdAuthMethod wrapper struct
On Fri, Mar 24, 2017 at 08:05:49AM +0100, Markus Armbruster wrote: > Adding Daniel Berrange for QCryptoSecret expertise. > > Jeff Cody writes: > > > On Thu, Mar 23, 2017 at 04:56:30PM -0500, Eric Blake wrote: > >> On 03/23/2017 04:43 PM, Eric Blake wrote: > >> > >> > We'd still have to allow libvirt's use of > >> > ":key=...:auth_supported=cephx\\;none" on the command line, but from the > >> > QMP side, we would support exactly one auth method, and libvirt will be > >> > updated to match when it starts targetting blockdev-add instead of > >> > legacy command line. > >> > > >> > If librados really needs 'cephx;none' any time cephx authorization is > >> > requested, then even though the QMP only requests 'cephx', we can still > >> > map it to 'cephx;none' in qemu - but I'm hoping that setting > >> > auth_supported=cephx rather than auth_supported=cephx;none on the > >> > librados side gives us what we (and libvirt) really want in the first > >> > place. > >> > >> Or, if it becomes something that libvirt wants to allow users to tune in > >> their XML (right now, users don't get a choice, they get either 'none' > >> or 'cephx;none'), a forward-compatible solution under my QMP proposal > >> would be to have qemu add a third enum state, "none", "cephx", and > >> "cephx-no-fallback". > >> > >> On the other hand, if supporting multiple methods at once makes sense > >> (say librados adds a 'cephy' method, and that users could legitimately > >> use both 'cephx;cephy' and 'cephy;cephx' with different behaviors), then > >> keeping auth as an array of instances of a simple flat union scales > >> nicer than having to add a combinatorial explosion of branches to the > >> flat union to cover every useful combination of auth_supported methods. > >> Maybe I'm overthinking it. > >> > > > > I spoke over email with Jason Dillaman (cc'ed), and this is what he told me > > with regards to the authentication methods, and what cephx;none means: > > > > On Thu, Mar 23, 2017 at 06:04:30PM -0400, Jason Dillaman wrote: > >> It's saying that the client is willing to connect to a server that > >> uses cephx auth or a server that uses no auth. Looking at the code, > >> the "auth_supported" configuration key is actually deprecated and > >> "auth_client_required" should be used instead (which defaults to > >> 'cephx, none' already). Since it's been deprecated since v0.55 and if > >> you are cleaning things up, feel free to switch to the new one if > >> possible. > > > > So from what Jason is saying, it seems like the conclusion we reached over > > IRC is correct: it will attempt cephx but also fallback to no auth. > > So the client offers the server a list of authentication methods with > credentials, and the server picks one it likes. Makes sense to me. > > Inluding "none" in the default less so, but that's of no concern to the > QMP interface, so let's ignore it for now. > > > Also, since the preferred auth option may be named different depending on > > ceph versions, I know think we probably should not tie the QAPI parameter to > > the name of the older deprecated option. > > Yes. > > > I suggest that the 'auth_supported' parameter for QAPI be renamed to > > 'auth_method'. If you don't like the array and the flexibility it provides > > at the cost of complexity, I think a flat enum consisting of 3 values like > > you mentioned is reasonable. Since the QAPI does not need to map to the > > legacy commandline used by libvirt, I would suggest maybe naming them > > slightly different, though: any, none, strict > > > > For 2.9, they could each map to 'auth_supported' like so: > > > > any: "cephx;none" > > none:"none" > > strict: "cephx" > > > > For 2.10, we could try to discover if 'auth_client_required' is supported or > > not, and use either it or 'auth_supported' as appropriate (with the same > > mappings as above). > > > > The reason I like "any" and "strict", is it gives consistent meanings to > > options even if new auth methods are introduced. For a hypothetical "cephy" > > method example: > > > > any: "cephy;cephx;none" > > none:"none" > > strict: "cephy;cephx" > > Two years later, an unfixable cryptograhic weakness in cephx is > discovered. Some users really, really want to select only "cephy", but > they can't. We react by pushing out a QEMU update adding method > "stricter", which expands into "cephy". Libvirt then reacts and pushes > out an update. And so forth, up the stack. Many moons later, users can > actually select "cephy". Thanks, but no thanks. > Good point; it could be dealt with, but only clumsily. > I think we should expose the current, recommended way to configure > authentication, in a form that is suitable for QAPI/QMP, i.e. structured > data as JSON objects, not strings you need to parse. > > If a future authentication method might need something else than a > QCryptoSecret object, we need to take Eric's proposal and make this an > array of
Re: [Qemu-devel] [PATCH 0/3] script for crash-testing -device
On 03/23/2017 05:43 PM, Thomas Huth wrote: On 22.03.2017 20:13, Eduardo Habkost wrote: On Wed, Mar 22, 2017 at 01:00:49PM -0300, Eduardo Habkost wrote: This series adds scripts/device-crashtest.py, that can be used to crash-test -device with multiple machine/accel/device combinations. The script found a few crashes on some machines/devices. A dump of existing cases can be seen here: https://gist.github.com/ehabkost/503b0af0375f0d98d3e84017e8ca54eb The script contains a whitelist that can also be useful as documentation of existing ways -device can fail or crash. Note that the script takes a few hours to run on the default mode (testing all accel/machine/device combinations), but the "-r N" option can be used to make it only test N random samples. Wow, impressive script, that must have been a lot of work 'til you've got it in a usable shape with that huge whitelist! +1 Great work Eduardo, thanks! Something I forgot to mention: I would like to run some subset of these tests on "make check", but I don't know how we could choose that subset. We could run, e.g., 100 random samples, but I am not sure we really want to make "make check" non-deterministic. Maybe limit the tests to the devices that have a high chance to work on different machines? ... that means primarily PCI, ISA and USB devices, I guess. Is hard to maintain that list, it will miss new devices and so on. We should have a "nightly" run or something, but still, maintaining the white list of known errors is still problematic. Thanks, Marcel Thomas
[Qemu-devel] [PATCH] ppc_booke: drop useless assignment
From: KONRAD Frederic The tb_env variable is set two lines above. So just drop the double assignment. Signed-off-by: KONRAD Frederic --- hw/ppc/ppc_booke.c | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/ppc/ppc_booke.c b/hw/ppc/ppc_booke.c index 60baffa..23bcf1b 100644 --- a/hw/ppc/ppc_booke.c +++ b/hw/ppc/ppc_booke.c @@ -282,7 +282,6 @@ void store_booke_tcr(CPUPPCState *env, target_ulong val) ppc_tb_t *tb_env = env->tb_env; booke_timer_t *booke_timer = tb_env->opaque; -tb_env = env->tb_env; env->spr[SPR_BOOKE_TCR] = val; kvmppc_set_tcr(cpu); -- 1.8.3.1
Re: [Qemu-devel] qemu-system-s390x tests/boot-serial-test intermittent failure
On 24 March 2017 at 11:50, Peter Maydell wrote:
> On 24 March 2017 at 11:11, Christian Borntraeger
> wrote:
>> diff --git a/tests/boot-serial-test.c b/tests/boot-serial-test.c
>> index 57edf6a..11f48b0 100644
>> --- a/tests/boot-serial-test.c
>> +++ b/tests/boot-serial-test.c
>> @@ -79,8 +79,8 @@ static void test_machine(const void *data)
>> g_assert(fd != -1);
>>
>> args = g_strdup_printf("-M %s,accel=tcg -chardev
>> file,id=serial0,path=%s"
>> - " -serial chardev:serial0 %s", test->machine,
>> - tmpname, test->extra);
>> + " -no-shutdown -serial chardev:serial0 %s",
>> + test->machine, tmpname, test->extra);
>
> ...but this doesn't help.
I think I must have got the testing wrong here somehow, because
trying it again, it does seem to cause the failures to stop.
So I guess this must be the bug...
thanks
-- PMM
Re: [Qemu-devel] qemu-system-s390x tests/boot-serial-test intermittent failure
On 03/24/2017 01:56 PM, Peter Maydell wrote:
> On 24 March 2017 at 11:50, Peter Maydell wrote:
>> On 24 March 2017 at 11:11, Christian Borntraeger
>> wrote:
>>> diff --git a/tests/boot-serial-test.c b/tests/boot-serial-test.c
>>> index 57edf6a..11f48b0 100644
>>> --- a/tests/boot-serial-test.c
>>> +++ b/tests/boot-serial-test.c
>>> @@ -79,8 +79,8 @@ static void test_machine(const void *data)
>>> g_assert(fd != -1);
>>>
>>> args = g_strdup_printf("-M %s,accel=tcg -chardev
>>> file,id=serial0,path=%s"
>>> - " -serial chardev:serial0 %s", test->machine,
>>> - tmpname, test->extra);
>>> + " -no-shutdown -serial chardev:serial0 %s",
>>> + test->machine, tmpname, test->extra);
>>
>> ...but this doesn't help.
>
> I think I must have got the testing wrong here somehow, because
> trying it again, it does seem to cause the failures to stop.
> So I guess this must be the bug...
Will send a proper patch.
[Qemu-devel] [PATCH] boot-serial-test: use -no-shutdown
a qemu with an empty s390 guest will exit very quickly. This races
against the testsuite reading from the console pipe leading to
intermittent test suite failures. Using -no-shutdown will keep
the guest running.
Fixes: 864111f422ba (vl: exit qemu on guest panic if -no-shutdown is not set)
Reported-by: Peter Maydell
Signed-off-by: Christian Borntraeger
---
tests/boot-serial-test.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/boot-serial-test.c b/tests/boot-serial-test.c
index 57edf6a..11f48b0 100644
--- a/tests/boot-serial-test.c
+++ b/tests/boot-serial-test.c
@@ -79,8 +79,8 @@ static void test_machine(const void *data)
g_assert(fd != -1);
args = g_strdup_printf("-M %s,accel=tcg -chardev file,id=serial0,path=%s"
- " -serial chardev:serial0 %s", test->machine,
- tmpname, test->extra);
+ " -no-shutdown -serial chardev:serial0 %s",
+ test->machine, tmpname, test->extra);
qtest_start(args);
unlink(tmpname);
--
2.7.4
Re: [Qemu-devel] [PATCH for-2.10 22/23] numa: add '-numa cpu, ...' option for property based node mapping
On Thu, 23 Mar 2017 08:23:32 -0500
Eric Blake wrote:
...
> >
> > +@samp{cpu} option is new alternative to @samp{cpus} option
>
> s/is/is a/
>
> > +uses @samp{socket-id|core-id|thread-id} properties to assign
>
> s/uses/which uses/
>
> > +CPU objects to a @var{node} using topology layout properties of CPU.
> > +Set of properties is machine specific, and depends on used machine
>
> s/Set/The set/
>
Fixed in v2 branch
Re: [Qemu-devel] [PATCH for-2.9 3/5] rbd: Rewrite the code to extract list-valued options
On 03/24/2017 03:25 AM, Markus Armbruster wrote:
-value = host;
-if (port) {
+host = qstring_get_str(qobject_to_qstring(val));
+sprintf(keybuf, "server.%d.port", i);
+port = qdict_get_str(options, keybuf);
>>>
>>> This segfaults if the port option isn't given.
>>
>> @port is mandatory in BlockdevOptionsRbd. If it's not there here, the
>> options must have bypassed QAPI. That would be bad news. Can you
>> explain how it can happen?
>
> Answering myself, please correct mistakes.
>
> There are two ways to create @options:
>
> 1. -blockdev and blockdev-add
>
>These create @options with a QAPI visitor from the command line
>option argument or QMP arguments, respectively. This checks them
>against the QAPI schema. Missing @port is rejected.
>
> 2. -drive and drive_add
>
>These appear to create @options manually, without checking against
>the QAPI schema.
>
>Crash reproducer: -drive driver=rbd,server.0.host=s0
Yep - and that's why the old code was checking 'if (port)'.
>
> In other words, we have *two* specifications for @options: the QAPI
> schema, and the union of all the QemuOptsList that apply. In case 1, we
> check against both (I think). In case 2, we only check against the
> latter.
>
> I understand how we got into this state, but it's not a good state to be
> in. We need to have our options defined in one way and one way only.
>
> For 2.9, we cope with missing @port.
Maybe for 2.10 we would make port optional rather than mandatory - but
that's a LOT of code to audit to add in appropriate 'has_port=true' so
it's too late to do for 2.9. Or even better, maybe for 2.10 we finally
implement parameter defaults so that we don't need a has_port field (if
port is omitted, it gets assigned the default value). Except that I
don't know if all the existing users of InetSocketAddress will want the
same default. Maybe that argues that we want some way to declare a QAPI
subtype that changes the default of a field otherwise inherited from the
base class (so port is mandatory in the base class, but each instance
that wants a default port creates a new subclass with a new default value).
>
> Post 2.9, we should either finish the QAPIfication of block
> configuration we started with blockdev-add, or back it out, i.e. make
> the QAPI schema accept anything, and rely on the QemuOpts-based
> checking.
>
> I want us to finish QAPIfication.
I'd like to finish the QAPIfication as well. I'm fine if port is
mandatory for QMP and -blockdev-add for 2.9, where only -drive gets away
with the default (but it DOES mean that we have to make sure that
QemuOpts created by -drive is augmented with the default before we pass
it through QAPI validation), so that back-compat of -drive omitting port
doesn't break.
>>> Of course, this also changes the behaviour so that additional options in
>>> server.* and auth-supported.* aren't silently ignored any more, but we
>>> complain that they are unknown. I consider this a bonus bug fix, but it
>>> should probably be spelt out in the commit message.
>>
>> Good point.
>
> Note to self: this applies to -drive / drive_add, but not to -blockdev /
> blockdev_add, because the QAPI schema kicks in there. Example:
> -drive driver=rbd,server.0.host=s0,server.0.port=p0,server.0.foo=bar
Yep - another instance where -drive parsing is slightly different than
-blockdev-add parsing. However, while tightening the parse to require
port is not back-compat friendly to -drive, I think that tightening the
parse to reject garbage given to -drive is perfectly acceptable.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [Qemu-arm] [patch 1/1]about armv8's prefetch decode
Hi Pranith,
Thanks for your reply. patch as below, new added code default is off,
please review.
The major thinking is about translate Armv8's prefetch instruction into
intermediate code, at the same time don't effect the rm/rn register.
diff --git a/translate-a64.c b/translate-a64.c
index 814f30f..86da8ee 100644
--- a/translate-a64.c
+++ b/translate-a64.c
@@ -2061,7 +2061,11 @@ static void disas_ld_lit(DisasContext *s, uint32_t insn)
} else {
if (opc == 3) {
/* PRFM (literal) : prefetch */
+#ifdef TCG_AARCH64_PREFETCH_TRANSLATE
+;
+#else
return;
+#endif
}
size = 2 + extract32(opc, 0, 1);
is_signed = extract32(opc, 1, 1);
@@ -2075,9 +2079,19 @@ static void disas_ld_lit(DisasContext *s, uint32_t insn)
} else {
/* Only unsigned 32bit loads target 32bit registers. */
bool iss_sf = opc != 0;
-
-do_gpr_ld(s, tcg_rt, tcg_addr, size, is_signed, false,
- true, rt, iss_sf, false);
+#ifdef TCG_AARCH64_PREFETCH_TRANSLATE
+if (opc == 3) {
+TCGv_i64 v = tcg_temp_new_i64();
+do_gpr_ld(s, v, tcg_addr, size, is_signed, false,
+ true, rt, iss_sf, false);
+tcg_temp_free_i64(v);
+} else {
+#endif
+do_gpr_ld(s, tcg_rt, tcg_addr, size, is_signed, false,
+ true, rt, iss_sf, false);
+#ifdef TCG_AARCH64_PREFETCH_TRANSLATE
+}
+#endif
}
tcg_temp_free_i64(tcg_addr);
}
@@ -2283,7 +2297,11 @@ static void disas_ldst_reg_imm9(DisasContext *s,
uint32_t insn,
unallocated_encoding(s);
return;
}
+#ifdef TCG_AARCH64_PREFETCH_TRANSLATE
+rn = 31;
+#else
return;
+#endif
}
if (opc == 3 && size > 1) {
unallocated_encoding(s);
@@ -2334,9 +2352,21 @@ static void disas_ldst_reg_imm9(DisasContext *s,
uint32_t insn,
do_gpr_st_memidx(s, tcg_rt, tcg_addr, size, memidx,
iss_valid, rt, iss_sf, false);
} else {
-do_gpr_ld_memidx(s, tcg_rt, tcg_addr, size,
- is_signed, is_extended, memidx,
- iss_valid, rt, iss_sf, false);
+#ifdef TCG_AARCH64_PREFETCH_TRANSLATE
+if (size == 3 && opc == 2) {
+TCGv_i64 v = tcg_temp_new_i64();
+do_gpr_ld_memidx(s, v, tcg_addr, size,
+ is_signed, is_extended, memidx,
+ iss_valid, rt, iss_sf, false);
+tcg_temp_free_i64(v);
+ } else {
+ #endif
+ do_gpr_ld_memidx(s, tcg_rt, tcg_addr, size,
+ is_signed, is_extended, memidx,
+ iss_valid, rt, iss_sf, false);
+ #ifdef TCG_AARCH64_PREFETCH_TRANSLATE
+ }
+ #endif
}
}
@@ -2383,6 +2413,9 @@ static void disas_ldst_reg_roffset(DisasContext *s,
uint32_t insn,
bool is_signed = false;
bool is_store = false;
bool is_extended = false;
+#ifdef TCG_AARCH64_PREFETCH_TRANSLATE
+TCGv_i64 v = tcg_temp_new_i64();
+#endif
TCGv_i64 tcg_rm;
TCGv_i64 tcg_addr;
@@ -2405,7 +2438,11 @@ static void disas_ldst_reg_roffset(DisasContext *s,
uint32_t insn,
} else {
if (size == 3 && opc == 2) {
/* PRFM - prefetch */
+#ifdef TCG_AARCH64_PREFETCH_TRANSLATE
+rn = 31;
+#else
return;
+#endif
}
if (opc == 3 && size > 1) {
unallocated_encoding(s);
@@ -2422,9 +2459,17 @@ static void disas_ldst_reg_roffset(DisasContext *s,
uint32_t insn,
tcg_addr = read_cpu_reg_sp(s, rn, 1);
tcg_rm = read_cpu_reg(s, rm, 1);
-ext_and_shift_reg(tcg_rm, tcg_rm, opt, shift ? size : 0);
-
-tcg_gen_add_i64(tcg_addr, tcg_addr, tcg_rm);
+#ifdef TCG_AARCH64_PREFETCH_TRANSLATE
+if ((!is_vector) &&(size == 3 && opc == 2)) {
+ext_and_shift_reg(v, tcg_rm, opt, shift ? size : 0);
+tcg_gen_add_i64(tcg_addr, tcg_addr, v);
+} else {
+#endif
+ext_and_shift_reg(tcg_rm, tcg_rm, opt, shift ? size : 0);
+tcg_gen_add_i64(tcg_addr, tcg_addr, tcg_rm);
+#ifdef TCG_AARCH64_PREFETCH_TRANSLATE
+}
+#endif
if (is_vector) {
if (is_store) {
@@ -2439,11 +2484,25 @@ static void disas_ldst_reg_roffset(DisasContext *s,
uint32_t insn,
do_gpr_st(s, tcg_rt, tcg_addr, size,
true, rt, iss_sf, false);
} else {
-do_gpr_ld(s, tcg_rt, tcg_addr, size,
- is_signed, is_extended,
- true, rt, iss_sf, false);
+#ifdef TCG_AARCH64_PREFETCH_TRANSLATE
+
Re: [Qemu-devel] [PATCH] Dead code removal: removing code for unsupported DEPTH.
On Thu, Mar 16, 2017 at 2:12 PM, Gerd Hoffmann wrote:
>> > -#define DEPTH 8
>> > -#include "cirrus_vga_rop2.h"
>> > -
>> > -#define DEPTH 16
>> > -#include "cirrus_vga_rop2.h"
>> > -
>> > -#define DEPTH 24
>> > -#include "cirrus_vga_rop2.h"
>> > -
>> > #define DEPTH 32
>> > #include "cirrus_vga_rop2.h"
>
> No. It isn't *that* simple. The cirrus blitter operates on the guest
> framebuffer, which can have any format the guest wishes it to have.
> This is *not* about rendering into a 32bpp DisplaySurface.
Gerd, please reply if this is wrong, but here is a summary of the task:
Some emulated graphics cards use qemu_console_surface() and that
surface is always in 32 bits per pixel format. Therefore, code for
dealing with other pixel formats can be removed.
When looking for emulated graphics cards where this cleanup is
possible, keep in mind that some of the code operates on the guest's
frame buffer, whose pixel format is not under QEMU's control.
Therefore we cannot assume that 32bpp is always used and this cleanup
doesn't apply there. Limit yourself to code that uses
qemu_console_surface() exclusively to access the surface and you
should be fine. Other code is likely to need support for additional
pixel formats.
One example is hw/display/milkymist-vgafb.c:
static void vgafb_update_display(void *opaque)
{
DisplaySurface *surface = qemu_console_surface(s->con);
...
switch (surface_bits_per_pixel(surface)) {
case 0:
return;
case 8:
fn = draw_line_8;
break;
case 15:
fn = draw_line_15;
dest_width *= 2;
break;
case 16:
fn = draw_line_16;
dest_width *= 2;
break;
case 24:
fn = draw_line_24;
dest_width *= 3;
break;
case 32:
fn = draw_line_32;
dest_width *= 4;
break;
All cases except for 32 are dead code (they will never execute). The
hw/display/milkymist-vgafb_template.h file can be deleted.
draw_line_32() can be macro expanded and moved into
hw/display/milkymist-vgafb.c.
Stefan
[Qemu-devel] [PATCH v3] xen-platform: separate unplugging of NVMe disks
Commit 090fa1c8 "add support for unplugging NVMe disks..." extended the
existing disk unplug flag to cover NVMe disks as well as IDE and SCSI.
The recent thread on the xen-devel mailing list [1] has highlighted that
this is not desirable behaviour: PV frontends should be able to distinguish
NVMe disks from other types of disk and should have separate control over
whether they are unplugged.
This patch defines a new bit in the unplug mask for this purpose (see Xen
commit [2]) and also tidies up the definitions of, and improves the
comments regarding, the previously exiting bits in the protocol.
[1] https://lists.xen.org/archives/html/xen-devel/2017-03/msg02924.html
[2] http://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=1096aa02
Signed-off-by: Paul Durrant
Reviewed-by: Stefano Stabellini
--
Cc: Anthony Perard
v3:
- Updated to reference Xen documentation patch
v2:
- Fix the commit comment
---
hw/i386/xen/xen_platform.c | 47 ++
1 file changed, 35 insertions(+), 12 deletions(-)
diff --git a/hw/i386/xen/xen_platform.c b/hw/i386/xen/xen_platform.c
index 6010f35..983d532 100644
--- a/hw/i386/xen/xen_platform.c
+++ b/hw/i386/xen/xen_platform.c
@@ -87,10 +87,30 @@ static void log_writeb(PCIXenPlatformState *s, char val)
}
}
-/* Xen Platform, Fixed IOPort */
-#define UNPLUG_ALL_DISKS 1
-#define UNPLUG_ALL_NICS 2
-#define UNPLUG_AUX_IDE_DISKS 4
+/*
+ * Unplug device flags.
+ *
+ * The logic got a little confused at some point in the past but this is
+ * what they do now.
+ *
+ * bit 0: Unplug all IDE and SCSI disks.
+ * bit 1: Unplug all NICs.
+ * bit 2: Unplug IDE disks except primary master. This is overridden if
+ *bit 0 is also present in the mask.
+ * bit 3: Unplug all NVMe disks.
+ *
+ */
+#define _UNPLUG_IDE_SCSI_DISKS 0
+#define UNPLUG_IDE_SCSI_DISKS (1u << _UNPLUG_IDE_SCSI_DISKS)
+
+#define _UNPLUG_ALL_NICS 1
+#define UNPLUG_ALL_NICS (1u << _UNPLUG_ALL_NICS)
+
+#define _UNPLUG_AUX_IDE_DISKS 2
+#define UNPLUG_AUX_IDE_DISKS (1u << _UNPLUG_AUX_IDE_DISKS)
+
+#define _UNPLUG_NVME_DISKS 3
+#define UNPLUG_NVME_DISKS (1u << _UNPLUG_NVME_DISKS)
static void unplug_nic(PCIBus *b, PCIDevice *d, void *o)
{
@@ -111,7 +131,7 @@ static void unplug_disks(PCIBus *b, PCIDevice *d, void
*opaque)
{
uint32_t flags = *(uint32_t *)opaque;
bool aux = (flags & UNPLUG_AUX_IDE_DISKS) &&
-!(flags & UNPLUG_ALL_DISKS);
+!(flags & UNPLUG_IDE_SCSI_DISKS);
/* We have to ignore passthrough devices */
if (!strcmp(d->name, "xen-pci-passthrough")) {
@@ -124,12 +144,16 @@ static void unplug_disks(PCIBus *b, PCIDevice *d, void
*opaque)
break;
case PCI_CLASS_STORAGE_SCSI:
-case PCI_CLASS_STORAGE_EXPRESS:
if (!aux) {
object_unparent(OBJECT(d));
}
break;
+case PCI_CLASS_STORAGE_EXPRESS:
+if (flags & UNPLUG_NVME_DISKS) {
+object_unparent(OBJECT(d));
+}
+
default:
break;
}
@@ -147,10 +171,9 @@ static void platform_fixed_ioport_writew(void *opaque,
uint32_t addr, uint32_t v
switch (addr) {
case 0: {
PCIDevice *pci_dev = PCI_DEVICE(s);
-/* Unplug devices. Value is a bitmask of which devices to
- unplug, with bit 0 the disk devices, bit 1 the network
- devices, and bit 2 the non-primary-master IDE devices. */
-if (val & (UNPLUG_ALL_DISKS | UNPLUG_AUX_IDE_DISKS)) {
+/* Unplug devices. See comment above flag definitions */
+if (val & (UNPLUG_IDE_SCSI_DISKS | UNPLUG_AUX_IDE_DISKS |
+ UNPLUG_NVME_DISKS)) {
DPRINTF("unplug disks\n");
pci_unplug_disks(pci_dev->bus, val);
}
@@ -338,14 +361,14 @@ static void xen_platform_ioport_writeb(void *opaque,
hwaddr addr,
* If VMDP was to control both disk and LAN it would use 4.
* If it controlled just disk or just LAN, it would use 8 below.
*/
-pci_unplug_disks(pci_dev->bus, UNPLUG_ALL_DISKS);
+pci_unplug_disks(pci_dev->bus, UNPLUG_IDE_SCSI_DISKS);
pci_unplug_nics(pci_dev->bus);
}
break;
case 8:
switch (val) {
case 1:
-pci_unplug_disks(pci_dev->bus, UNPLUG_ALL_DISKS);
+pci_unplug_disks(pci_dev->bus, UNPLUG_IDE_SCSI_DISKS);
break;
case 2:
pci_unplug_nics(pci_dev->bus);
--
2.1.4
Re: [Qemu-devel] [PATCH] Dead code removal: removing code for unsupported DEPTH.
On 24 March 2017 at 13:36, Stefan Hajnoczi wrote: > Gerd, please reply if this is wrong, but here is a summary of the task: > > Some emulated graphics cards use qemu_console_surface() and that > surface is always in 32 bits per pixel format. Therefore, code for > dealing with other pixel formats can be removed. > > When looking for emulated graphics cards where this cleanup is > possible, keep in mind that some of the code operates on the guest's > frame buffer, whose pixel format is not under QEMU's control. > Therefore we cannot assume that 32bpp is always used and this cleanup > doesn't apply there. Limit yourself to code that uses > qemu_console_surface() exclusively to access the surface and you > should be fine. Other code is likely to need support for additional > pixel formats. Also worth pointing at an example of a patchset that does this for some other device, eg Gerd's recent pl110 set, as a guide to what's getting removed. thanks -- PMM
Re: [Qemu-devel] q35 and sysbus devices
On Fri, Mar 24, 2017 at 01:49:16PM +0300, Marcel Apfelbaum wrote: > On 03/22/2017 10:46 PM, Laszlo Ersek wrote: > > On 03/22/17 21:31, Eduardo Habkost wrote: > > > Hi, > > > > > > I am investigating the current status of has_dynamic_sysbus and > > > sysbus device support on each of QEMU's machine types. The good > > > news is that almost all has_dynamic_sysbus=1 machines have their > > > own internal (often short) whitelist of supported sysbus device > > > types, and automatically reject unsupported devices. > > > > > > ...except for q35. > > > > > > q35 currently accepts all sys-bus-device subtypes on "-device", > > > and today this includes the following 23 devices: > > > > > > * allwinner-ahci > > > * amd-iommu > > > * cfi.pflash01 > > > * esp > > > * fw_cfg_io > > > * fw_cfg_mem > > > * generic-sdhci > > > * hpet > > > * intel-iommu > > > * ioapic > > > * isabus-bridge > > > * kvmclock > > > * kvm-ioapic > > > * kvmvapic > > > * SUNW,fdtwo > > > * sysbus-ahci > > > * sysbus-fdc > > > * sysbus-ohci > > > * unimplemented-device > > > * virtio-mmio > > > * xen-backend > > > * xen-sysdev > > > > > > My question is: do all those devices really make sense to be used > > > with "-device" on q35? > > > > I think fw_cfg_io and fw_cfg_mem should be board-only devices (no > > -device switch). > > > > Regarding cfi.pflash01, I think originally it would have been nice to > > specify pflash chips with the modern (non-legacy) syntax, that is, > > separate -drive if=none,file=... backend options combined with -device > > cfi.pflash01,drive=... frontend options. However, that ship has sailed, > > even libvirt uses -drive if=pflash for these, and given the purpose we > > use pflash chips for, on Q35, I don't see much benefit in exposing > > cfi.pflash01 with a naked -device *now*. > > > > Re: virtio-mmio, I don't think that should be available on Q35 at all. > > > > I can't comment on the rest. > > > > Hi Eduardo, > Thanks for finding these problems. > > We should ping all maintainers of the above devices, the best way to do it > is to add the "cannot_instantiate_with_device_add_yet = true" and ask > maintainers > to agree (or not) on that. If I understand it correctly, cannot_instantiate_with_device_add_yet is supposed to be temporary. And it applies to all machines, with no exceptions. The problem with today's mechanism is that we have no way to make a machine accept one type of sysbus device without making it start accepting every other sysbus devices. If we thought all !cannot_instantiate_with_device_add_yet sysbus devices were already safe, we wouldn't have has_dynamic_sysbus in the first place, would we? -- Eduardo
Re: [Qemu-devel] [PATCH for-2.9 4/5] rbd: Peel off redundant RbdAuthMethod wrapper struct
On 03/24/2017 07:42 AM, Jeff Cody wrote:
> Agree. My preference is to leave it as an array of methods, so long as that
> is tenable to libvirt.
The -drive syntax should remain unchanged (that's an absolute must for
libvirt). But the QMP syntax being an array of methods sounds best to
me, and I think password-secret should be part of the array. So my vote
would be for:
{ "driver": "rbd", "image": "foo",
"auth": [ { "type": "cephx", "password-secret": "sec0" },
{ "type": "none" } ],
"pool": "bar" }
It makes mapping -drive arguments into QMP form a bit trickier, but the
QMP form is then easily extensible if we add another auth method down
the road.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [RFC 1/4] [UNTESTED] xen: Don't force has_dynamic_sysbus on machine class
On Fri, Mar 24, 2017 at 01:27:31PM +0100, Juergen Gross wrote:
> On 24/03/17 12:10, Eduardo Habkost wrote:
> > On Fri, Mar 24, 2017 at 11:24:31AM +0100, Juergen Gross wrote:
> >> On 24/03/17 11:09, Peter Maydell wrote:
> >>> On 24 March 2017 at 08:23, Juergen Gross wrote:
> On 23/03/17 22:28, Eduardo Habkost wrote:
> > The xen-backend devices created by the Xen code are not supposed
> > to be treated as dynamic sysbus devices. This is an attempt to
> > change that and see what happens, but I couldn't test it because
> > I don't have a Xen host set up.
> >
> > If this patch breaks anything, this means we have a bug in
> > foreach_dynamic_sysbus_device(), which is supposed to return only
> > devices created using -device.
> >
> > The original code that sets has_dynamic_sysbus was added by
> > commit 3a6c9172ac5951e6dac2b3f6cbce3cfccdec5894, but I don't see
> > any comment explaining why it was necessary.
>
> xen-backend devices are created via qmp commands when attaching new
> pv-devices to a domain. They can be dynamically removed, too. Setting
> has_dynamic_sysbus was necessary to support this feature.
> >>>
> >>> This seems like it ought to be handled by marking the xen-backend
> >>> devices as being ok-to-dynamically-create somehow, not by marking
> >>> the machine as supporting dynamic-sysbus (which it doesn't).
> >>> Maybe we don't have the necessary support code to do that though?
> >>
> >> When writing the patches I couldn't find a way to do it differently.
> >> OTOH I'm not so deep in qemu internals I'd be able to add the needed
> >> support.
> >>
> >> I'd be happy to test any patch, though.
> >
> > If xen-backend devices are created via QMP commands, then
> > has_dynamic_sysbus is (currently) really needed, although I would
> > have preferred to set it on all x86 machines instead of changing
> > MachineClass::has_dynamic_sysbus outside class_init.
> >
> > But with the new whitelist implemented by this series, we could
> > simply include xen-backend in the whitelist for the machines that
> > can be used with Xen, and get rid of xen_set_dynamic_sysbus().
> >
> > I assume plugging/unplugging xen-backend devices apply to both
> > xen{pv,fv} and pc,accel=xen, right? Do we need to make it work
> > with "-machine none,accel=xen" and "-machine isapc" too?
>
> AFAIK -xenpv, -xenfv and -pc,accel=xen are the only machine types
> to support. Wouldn't it make sense to do the whitelisting in
> xen_be_register_common() in spite of setting has_dynamic_sysbus?
It would, but that would mean we would make the whitelisting
mechanism more complex: in addition to the static
per-machine-class whitelist, we would need a runtime whitelist.
This would make the interface for querying available/supported
device types more complex and messier, and I would like to avoid
that.
--
Eduardo
Re: [Qemu-devel] [RFC 1/4] [UNTESTED] xen: Don't force has_dynamic_sysbus on machine class
On 24/03/17 14:50, Eduardo Habkost wrote:
> On Fri, Mar 24, 2017 at 01:27:31PM +0100, Juergen Gross wrote:
>> On 24/03/17 12:10, Eduardo Habkost wrote:
>>> On Fri, Mar 24, 2017 at 11:24:31AM +0100, Juergen Gross wrote:
On 24/03/17 11:09, Peter Maydell wrote:
> On 24 March 2017 at 08:23, Juergen Gross wrote:
>> On 23/03/17 22:28, Eduardo Habkost wrote:
>>> The xen-backend devices created by the Xen code are not supposed
>>> to be treated as dynamic sysbus devices. This is an attempt to
>>> change that and see what happens, but I couldn't test it because
>>> I don't have a Xen host set up.
>>>
>>> If this patch breaks anything, this means we have a bug in
>>> foreach_dynamic_sysbus_device(), which is supposed to return only
>>> devices created using -device.
>>>
>>> The original code that sets has_dynamic_sysbus was added by
>>> commit 3a6c9172ac5951e6dac2b3f6cbce3cfccdec5894, but I don't see
>>> any comment explaining why it was necessary.
>>
>> xen-backend devices are created via qmp commands when attaching new
>> pv-devices to a domain. They can be dynamically removed, too. Setting
>> has_dynamic_sysbus was necessary to support this feature.
>
> This seems like it ought to be handled by marking the xen-backend
> devices as being ok-to-dynamically-create somehow, not by marking
> the machine as supporting dynamic-sysbus (which it doesn't).
> Maybe we don't have the necessary support code to do that though?
When writing the patches I couldn't find a way to do it differently.
OTOH I'm not so deep in qemu internals I'd be able to add the needed
support.
I'd be happy to test any patch, though.
>>>
>>> If xen-backend devices are created via QMP commands, then
>>> has_dynamic_sysbus is (currently) really needed, although I would
>>> have preferred to set it on all x86 machines instead of changing
>>> MachineClass::has_dynamic_sysbus outside class_init.
>>>
>>> But with the new whitelist implemented by this series, we could
>>> simply include xen-backend in the whitelist for the machines that
>>> can be used with Xen, and get rid of xen_set_dynamic_sysbus().
>>>
>>> I assume plugging/unplugging xen-backend devices apply to both
>>> xen{pv,fv} and pc,accel=xen, right? Do we need to make it work
>>> with "-machine none,accel=xen" and "-machine isapc" too?
>>
>> AFAIK -xenpv, -xenfv and -pc,accel=xen are the only machine types
>> to support. Wouldn't it make sense to do the whitelisting in
>> xen_be_register_common() in spite of setting has_dynamic_sysbus?
>
> It would, but that would mean we would make the whitelisting
> mechanism more complex: in addition to the static
> per-machine-class whitelist, we would need a runtime whitelist.
>
> This would make the interface for querying available/supported
> device types more complex and messier, and I would like to avoid
> that.
>
Okay, understood. And I suppose you don't want to add the Xen
devices to the per-machine-class whitelist (after all my patch
did the same with the has_dynamic_sysbus flag) on demand.
Either way is fine with me.
Juergen
[Qemu-devel] [PULL for-2.9 1/3] trace: Fix backwards mirror_yield parameters
From: Eric Blake
block/trace-events lists the parameters for mirror_yield
consistently with other mirror events (cnt just after s, like in
mirror_before_sleep; in_flight last, like in mirror_yield_in_flight).
But the callers were passing parameters in the wrong order, leading
to poor trace messages, including type truncation when there are
more than 4G dirty sectors involved. Broken since its introduction
in commit bd48bde.
While touching this, ensure that all callers use the same type
(uint64_t) for cnt, as a later patch will enable the compiler to do
stricter type-checking.
Signed-off-by: Eric Blake
Signed-off-by: Stefan Hajnoczi
---
block/mirror.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/block/mirror.c b/block/mirror.c
index ca4baa5..9e2fecc 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -634,7 +634,8 @@ static int coroutine_fn mirror_dirty_init(MirrorBlockJob *s)
}
if (s->in_flight >= MAX_IN_FLIGHT) {
-trace_mirror_yield(s, s->in_flight, s->buf_free_count, -1);
+trace_mirror_yield(s, UINT64_MAX, s->buf_free_count,
+ s->in_flight);
mirror_wait_for_io(s);
continue;
}
@@ -809,7 +810,7 @@ static void coroutine_fn mirror_run(void *opaque)
s->common.iostatus == BLOCK_DEVICE_IO_STATUS_OK) {
if (s->in_flight >= MAX_IN_FLIGHT || s->buf_free_count == 0 ||
(cnt == 0 && s->in_flight > 0)) {
-trace_mirror_yield(s, s->in_flight, s->buf_free_count, cnt);
+trace_mirror_yield(s, cnt, s->buf_free_count, s->in_flight);
mirror_wait_for_io(s);
continue;
} else if (cnt != 0) {
--
2.9.3
[Qemu-devel] [PULL for-2.9 3/3] trace: Avoid abuse of amdvi_mmio_read
From: Eric Blake
hw/i386/trace-events has an amdvi_mmio_read trace that is used for
both normal reads (listing the register name, address, size, and
offset) and for an error case (abusing the register name to show
an error message, the address to show the maximum value supported,
then shoehorning address and size into the size and offset
parameters). The change from a wide address to a narrower size
parameter could truncate a (rather-large) bogus read attempt, so
it's better to create a separate dedicated trace with correct types,
rather than abusing the trace mechanism. Broken since its
introduction in commit d29a09c.
[Change trace event argument type from hwaddr to uint64_t since
user-defined types should not be used for trace events. This fixes a
build failure with LTTng UST.
--Stefan]
Signed-off-by: Eric Blake
Signed-off-by: Stefan Hajnoczi
---
hw/i386/amd_iommu.c | 3 +--
hw/i386/trace-events | 1 +
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index e0732cc..f86a40a 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -572,8 +572,7 @@ static uint64_t amdvi_mmio_read(void *opaque, hwaddr addr,
unsigned size)
uint64_t val = -1;
if (addr + size > AMDVI_MMIO_SIZE) {
-trace_amdvi_mmio_read("error: addr outside region: max ",
-(uint64_t)AMDVI_MMIO_SIZE, addr, size);
+trace_amdvi_mmio_read_invalid(AMDVI_MMIO_SIZE, addr, size);
return (uint64_t)-1;
}
diff --git a/hw/i386/trace-events b/hw/i386/trace-events
index 88ad5e4..baed874 100644
--- a/hw/i386/trace-events
+++ b/hw/i386/trace-events
@@ -37,6 +37,7 @@ amdvi_cache_update(uint16_t domid, uint8_t bus, uint8_t slot,
uint8_t func, uint
amdvi_completion_wait_fail(uint64_t addr) "error: fail to write at address
0x%"PRIx64
amdvi_mmio_write(const char *reg, uint64_t addr, unsigned size, uint64_t val,
uint64_t offset) "%s write addr 0x%"PRIx64", size %u, val 0x%"PRIx64", offset
0x%"PRIx64
amdvi_mmio_read(const char *reg, uint64_t addr, unsigned size, uint64_t
offset) "%s read addr 0x%"PRIx64", size %u offset 0x%"PRIx64
+amdvi_mmio_read_invalid(int max, uint64_t addr, unsigned size) "error: addr
outside region (max 0x%x): read addr 0x%" PRIx64 ", size %u"
amdvi_command_error(uint64_t status) "error: Executing commands with command
buffer disabled 0x%"PRIx64
amdvi_command_read_fail(uint64_t addr, uint32_t head) "error: fail to access
memory at 0x%"PRIx64" + 0x%"PRIx32
amdvi_command_exec(uint32_t head, uint32_t tail, uint64_t buf) "command buffer
head at 0x%"PRIx32" command buffer tail at 0x%"PRIx32" command buffer base at
0x%"PRIx64
--
2.9.3
[Qemu-devel] [PULL for-2.9 2/3] trace: Fix incorrect megasas trace parameters
From: Eric Blake
hw/scsi/trace-events lists cmd as the first parameter for both
megasas_iovec_overflow and megasas_iovec_underflow, but the caller
was mistakenly passing cmd->iov_size twice instead of the command
index. Also, trace_megasas_abort_invalid is called with parameters
in the wrong order. Broken since its introduction in commit
e8f943c3.
Signed-off-by: Eric Blake
Reviewed-by: Hannes Reinecke
Signed-off-by: Stefan Hajnoczi
---
hw/scsi/megasas.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index e3d59b7..84b8caf 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -291,7 +291,7 @@ static int megasas_map_sgl(MegasasState *s, MegasasCmd
*cmd, union mfi_sgl *sgl)
if (cmd->iov_size > iov_size) {
trace_megasas_iovec_overflow(cmd->index, iov_size, cmd->iov_size);
} else if (cmd->iov_size < iov_size) {
-trace_megasas_iovec_underflow(cmd->iov_size, iov_size, cmd->iov_size);
+trace_megasas_iovec_underflow(cmd->index, iov_size, cmd->iov_size);
}
cmd->iov_offset = 0;
return 0;
@@ -1924,8 +1924,8 @@ static int megasas_handle_abort(MegasasState *s,
MegasasCmd *cmd)
abort_ctx &= (uint64_t)0x;
}
if (abort_cmd->context != abort_ctx) {
-trace_megasas_abort_invalid_context(cmd->index, abort_cmd->index,
-abort_cmd->context);
+trace_megasas_abort_invalid_context(cmd->index, abort_cmd->context,
+abort_cmd->index);
s->event_count++;
return MFI_STAT_ABORT_NOT_POSSIBLE;
}
--
2.9.3
[Qemu-devel] [PULL for-2.9 0/3] Tracing patches
The following changes since commit 08329701199449bde497570dcfdb9c86062baf20: qom: Fix regression with 'qom-type' (2017-03-23 17:59:40 +) are available in the git repository at: git://github.com/stefanha/qemu.git tags/tracing-pull-request for you to fetch changes up to 0d3ef78829332f2fdc323d1b625b60fe9c89119c: trace: Avoid abuse of amdvi_mmio_read (2017-03-24 09:21:42 +) Eric Blake (3): trace: Fix backwards mirror_yield parameters trace: Fix incorrect megasas trace parameters trace: Avoid abuse of amdvi_mmio_read block/mirror.c | 5 +++-- hw/i386/amd_iommu.c | 3 +-- hw/scsi/megasas.c| 6 +++--- hw/i386/trace-events | 1 + 4 files changed, 8 insertions(+), 7 deletions(-) -- 2.9.3
Re: [Qemu-devel] [PATCH for-2.9 4/5] rbd: Peel off redundant RbdAuthMethod wrapper struct
On Fri, Mar 24, 2017 at 08:49:20AM -0500, Eric Blake wrote:
> On 03/24/2017 07:42 AM, Jeff Cody wrote:
>
> > Agree. My preference is to leave it as an array of methods, so long as that
> > is tenable to libvirt.
>
> The -drive syntax should remain unchanged (that's an absolute must for
> libvirt). But the QMP syntax being an array of methods sounds best to
> me, and I think password-secret should be part of the array. So my vote
> would be for:
>
> { "driver": "rbd", "image": "foo",
> "auth": [ { "type": "cephx", "password-secret": "sec0" },
> { "type": "none" } ],
> "pool": "bar" }
>
> It makes mapping -drive arguments into QMP form a bit trickier, but the
> QMP form is then easily extensible if we add another auth method down
> the road.
>
In that case, what about adding user as well, and not just password-secret?
Jeff
[Qemu-devel] [PATCH v5] qga: Add `guest-get-timezone` command
From: Vinzenz Feenstra
Adds a new command `guest-get-timezone` reporting the currently
configured timezone on the system. The information on what timezone is
currently is configured is useful in case of Windows VMs where the
offset of the hardware clock is required to have the same offset. This
can be used for management systems like `oVirt` to detect the timezone
difference and warn administrators of the misconfiguration.
Signed-off-by: Vinzenz Feenstra
---
qga/commands.c | 29 +
qga/qapi-schema.json | 26 ++
2 files changed, 55 insertions(+)
diff --git a/qga/commands.c b/qga/commands.c
index 4d92946..3b5789c 100644
--- a/qga/commands.c
+++ b/qga/commands.c
@@ -499,3 +499,32 @@ int ga_parse_whence(GuestFileWhence *whence, Error **errp)
error_setg(errp, "invalid whence code %"PRId64, whence->u.value);
return -1;
}
+
+GuestTimezone *qmp_guest_get_timezone(Error **errp)
+{
+GuestTimezone *info = g_new0(GuestTimezone, 1);
+GTimeZone *tz = g_time_zone_new_local();
+if (tz == NULL) {
+error_setg(errp, QERR_QGA_COMMAND_FAILED,
+ "Couldn't retrieve local timezone");
+goto error;
+}
+
+gint64 now = g_get_real_time() / G_USEC_PER_SEC;
+gint32 intv = g_time_zone_find_interval(tz, G_TIME_TYPE_UNIVERSAL, now);
+info->offset = g_time_zone_get_offset(tz, intv);
+gchar const *name = g_time_zone_get_abbreviation(tz, intv);
+if (name != NULL) {
+info->has_zone = true;
+info->zone = g_strdup(name);
+}
+g_time_zone_unref(tz);
+
+return info;
+
+error:
+g_time_zone_unref(tz);
+g_free(info);
+return NULL;
+}
+
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index a02dbf2..5183ea2 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -1042,3 +1042,29 @@
'data':{ 'path': 'str', '*arg': ['str'], '*env': ['str'],
'*input-data': 'str', '*capture-output': 'bool' },
'returns': 'GuestExec' }
+
+
+##
+# @GuestTimezone:
+#
+# @zone:Timezone name
+# @offset: Offset to UTC in seconds, negative numbers for time zones west of
+# GMT, positive numbers for east
+#
+# Since: 2.10
+##
+{ 'struct': 'GuestTimezone',
+ 'data': { '*zone': 'str', 'offset': 'int' } }
+
+
+##
+# @guest-get-timezone:
+#
+# Retrieves the timezone information from the guest.
+#
+# Returns: A GuestTimezone dictionary.
+#
+# Since: 2.10
+##
+{ 'command': 'guest-get-timezone',
+ 'returns': 'GuestTimezone' }
--
2.9.3
[Qemu-devel] [PATCH v5] qga: Add `guest-get-timezone` command
From: Vinzenz Feenstra Changes since v4: - Corrected usage of the optional zone (Missing has_zone = true) - Updated documentation string for offset in the schema - Removed unnecessary checks - Uses the current time to check for the interval, not 0 - Switched to UNIVERSAL time type, since that can't fail and is all that's needed Vinzenz Feenstra (1): qga: Add `guest-get-timezone` command qga/commands.c | 29 + qga/qapi-schema.json | 26 ++ 2 files changed, 55 insertions(+) -- 2.9.3
Re: [Qemu-devel] [PATCH] boot-serial-test: use -no-shutdown
On 24 March 2017 at 13:19, Christian Borntraeger wrote:
> a qemu with an empty s390 guest will exit very quickly. This races
> against the testsuite reading from the console pipe leading to
> intermittent test suite failures. Using -no-shutdown will keep
> the guest running.
>
> Fixes: 864111f422ba (vl: exit qemu on guest panic if -no-shutdown is not set)
> Reported-by: Peter Maydell
> Signed-off-by: Christian Borntraeger
> ---
> tests/boot-serial-test.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tests/boot-serial-test.c b/tests/boot-serial-test.c
> index 57edf6a..11f48b0 100644
> --- a/tests/boot-serial-test.c
> +++ b/tests/boot-serial-test.c
> @@ -79,8 +79,8 @@ static void test_machine(const void *data)
> g_assert(fd != -1);
>
> args = g_strdup_printf("-M %s,accel=tcg -chardev file,id=serial0,path=%s"
> - " -serial chardev:serial0 %s", test->machine,
> - tmpname, test->extra);
> + " -no-shutdown -serial chardev:serial0 %s",
> + test->machine, tmpname, test->extra);
>
> qtest_start(args);
> unlink(tmpname);
Applied to master, thanks. Let's see if this fixes the travis flapping...
-- PMM
Re: [Qemu-devel] q35 and sysbus devices
On 24 March 2017 at 13:48, Eduardo Habkost wrote: > The problem with today's mechanism is that we have no way to make > a machine accept one type of sysbus device without making it > start accepting every other sysbus devices. If we thought all > !cannot_instantiate_with_device_add_yet sysbus devices were > already safe, we wouldn't have has_dynamic_sysbus in the first > place, would we? Almost all sysbus devices are not dynamically instantiable and never will be because they need to map mmio and wire up IRQs and so on, and we have no mechanism for doing that. So the default position is "no, you can't do that (but maybe a subclass which knows better might override)". has_dynamic_sysbus is a dodgy hack for boards which support the "platform bus" which is a special case where the user can use -device for certain sysbus devices that the platform bus knows about and can wire up itself (in a range of irqs and memory that the board has set aside for that purpose). The default is still "no, you can't dynamically instantiate this", but instead of this (and the handful of exceptions) being imposed at compile time it's imposed (with exceptions) at runtime. So for instance the ARM 'virt' board sets has_dynamic_sysbus, which allows a few devices to be added with -device, but most still do not permit it (they'll end up hitting the error_report() in hw/arm/sysbus-fdt.c:add_fdt_node()). thanks -- PMM
Re: [Qemu-devel] [PATCH 2/2 v1] throttle: factor out the redundant code
On 3/23/2017 7:14 PM, Eric Blake wrote:
On 03/23/2017 09:46 AM, Eric Blake wrote:
On 03/23/2017 07:20 AM, Pradeep Jagadeesh wrote:
This patchset removes the throttle redundant code that was present
in block and fsdev files.
Signed-off-by: Pradeep Jagadeesh
---
I think you want portions of this patch to come first; you want to
refactor out the IOThrottle portion of existing types, and then extend
IOThrottle to be used in more places.
For the record, here's a nice example of how to factor out common fields
into a new base class:
https://lists.gnu.org/archive/html/qemu-devel/2017-03/msg04649.html
Nice one, but If I do this way, I can not remove some duplicate code
from initialization code. For example below code.
void hmp_block_set_io_throttle(Monitor *mon, const QDict *qdict)
{
Error *err = NULL;
-BlockIOThrottle throttle = {
-.has_device = true,
-.device = (char *) qdict_get_str(qdict, "device"),
-.bps = qdict_get_int(qdict, "bps"),
-.bps_rd = qdict_get_int(qdict, "bps_rd"),
-.bps_wr = qdict_get_int(qdict, "bps_wr"),
-.iops = qdict_get_int(qdict, "iops"),
-.iops_rd = qdict_get_int(qdict, "iops_rd"),
-.iops_wr = qdict_get_int(qdict, "iops_wr"),
-};
Also
void hmp_fsdev_set_io_throttle(Monitor *mon, const QDict *qdict)
{
Error *err = NULL;
-IOThrottle throttle = {
-.device = (char *) qdict_get_str(qdict, "device"),
-.bps = qdict_get_int(qdict, "bps"),
-.bps_rd = qdict_get_int(qdict, "bps_rd"),
-.bps_wr = qdict_get_int(qdict, "bps_wr"),
-.iops = qdict_get_int(qdict, "iops"),
-.iops_rd = qdict_get_int(qdict, "iops_rd"),
-.iops_wr = qdict_get_int(qdict, "iops_wr"),
-};
-
Regards,
Pradeep
Re: [Qemu-devel] guest-exec of qemu-guest-agent fails on Windows guest
Hi
On Fri, Mar 24, 2017 at 2:38 PM Armin Ranjbar wrote:
> Hi everyone,
>
> We are trying to use guest-exec[0] command of qemu-guest-agent. However,
> discovered that whatever command we try to run, it ends up with the
> following error on the host side:
>
> > error: Guest agent is not responding: Guest agent not available for now
>
I don't have this error, perhaps you could try running the ga under gdb and
see where it hangs?
>
> An example of what we run on the host side:
>
> > virsh qemu-agent-command --domain armin_1 '{ "execute": "guest-exec",
> "arguments": { "path": "C:/Windows/System32/ipconfig.exe",
> "capture-output": false } }'
>
> Because Windows was complaining about `gspawn-win64-helper-console.exe`
>
Where does it complain?
> whenever we ran a command on host, we tried to run it directly from cli and
> it failed on the following assertion:
>
> > g_assert (argc >= ARG_COUNT); // [1]
>
> This picture[2] might be a bit more useful as it includes the output of
> `qemu-ga` in verbose mode.
>
Any idea how we can solve it and/or what we are doing wrong?
>
>
I am pretty sure you can't call spawn-helper like this in command line.
the warning about child setup function is harmless
Which version qemu-ga are you testing?
> Thanks in advance.
>
> [0]
>
> https://github.com/qemu/qemu/blob/fbddc2e5608eb655493253d080598375db61a748/qga/qapi-schema.json#L1024
> [1]
>
> https://github.com/GNOME/glib/blob/8edcf67b0221efa3c2ada67c44eff29939b1704d/glib/gspawn-win32-helper.c#L208
> [2] https://ibin.co/3GTVo4WXfR7a.png
>
> ---
> Armin ranjbar
>
--
Marc-André Lureau
[Qemu-devel] [PATCH 1/2] virtio-input: free event queue when finalizing
VirtIOInput.queue was never freed. This commit adds an explicit
g_free to virtio_input_finalize and switches the allocation
function from realloc to g_realloc in virtio_input_send.
Signed-off-by: Ladi Prosek
---
hw/input/virtio-input.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/hw/input/virtio-input.c b/hw/input/virtio-input.c
index b678ee9..728832a 100644
--- a/hw/input/virtio-input.c
+++ b/hw/input/virtio-input.c
@@ -32,8 +32,8 @@ void virtio_input_send(VirtIOInput *vinput,
virtio_input_event *event)
/* queue up events ... */
if (vinput->qindex == vinput->qsize) {
vinput->qsize++;
-vinput->queue = realloc(vinput->queue, vinput->qsize *
-sizeof(virtio_input_event));
+vinput->queue = g_realloc(vinput->queue, vinput->qsize *
+ sizeof(virtio_input_event));
}
vinput->queue[vinput->qindex++] = *event;
@@ -272,6 +272,8 @@ static void virtio_input_finalize(Object *obj)
QTAILQ_REMOVE(&vinput->cfg_list, cfg, node);
g_free(cfg);
}
+
+g_free(vinput->queue);
}
static void virtio_input_device_unrealize(DeviceState *dev, Error **errp)
{
--
2.7.4
[Qemu-devel] [PATCH 2/2] virtio-input: fix eventq batching
virtio_input_send buffers input events until it sees a SYNC. Then it
either sends or drops the entire batch, depending on whether eventq
has enough space available. The case to avoid here is partial sends
where only part of the batch would get to the guest.
Using virtqueue_get_avail_bytes to check the state of eventq was not
correct. The queue may have a smaller number of larger buffers
available so bytes may be enough but the batch would still not be
possible to send, leading to the "Huh? No vq elem available" error.
Instead of checking available bytes, this patch optimistically pops
buffers from the queue and puts them back in case it runs out of
space and the batch needs to be dropped.
Signed-off-by: Ladi Prosek
---
hw/input/virtio-input.c | 29 ++---
include/hw/virtio/virtio-input.h | 5 -
2 files changed, 18 insertions(+), 16 deletions(-)
diff --git a/hw/input/virtio-input.c b/hw/input/virtio-input.c
index 728832a..0e42f0d 100644
--- a/hw/input/virtio-input.c
+++ b/hw/input/virtio-input.c
@@ -22,7 +22,6 @@
void virtio_input_send(VirtIOInput *vinput, virtio_input_event *event)
{
VirtQueueElement *elem;
-unsigned have, need;
int i, len;
if (!vinput->active) {
@@ -33,9 +32,9 @@ void virtio_input_send(VirtIOInput *vinput,
virtio_input_event *event)
if (vinput->qindex == vinput->qsize) {
vinput->qsize++;
vinput->queue = g_realloc(vinput->queue, vinput->qsize *
- sizeof(virtio_input_event));
+ sizeof(vinput->queue[0]));
}
-vinput->queue[vinput->qindex++] = *event;
+vinput->queue[vinput->qindex++].event = *event;
/* ... until we see a report sync ... */
if (event->type != cpu_to_le16(EV_SYN) ||
@@ -44,24 +43,24 @@ void virtio_input_send(VirtIOInput *vinput,
virtio_input_event *event)
}
/* ... then check available space ... */
-need = sizeof(virtio_input_event) * vinput->qindex;
-virtqueue_get_avail_bytes(vinput->evt, &have, NULL, need, 0);
-if (have < need) {
-vinput->qindex = 0;
-trace_virtio_input_queue_full();
-return;
-}
-
-/* ... and finally pass them to the guest */
for (i = 0; i < vinput->qindex; i++) {
elem = virtqueue_pop(vinput->evt, sizeof(VirtQueueElement));
if (!elem) {
-/* should not happen, we've checked for space beforehand */
-fprintf(stderr, "%s: Huh? No vq elem available ...\n", __func__);
+while (--i >= 0) {
+virtqueue_unpop(vinput->evt, vinput->queue[i].elem, 0);
+}
+vinput->qindex = 0;
+trace_virtio_input_queue_full();
return;
}
+vinput->queue[i].elem = elem;
+}
+
+/* ... and finally pass them to the guest */
+for (i = 0; i < vinput->qindex; i++) {
+elem = vinput->queue[i].elem;
len = iov_from_buf(elem->in_sg, elem->in_num,
- 0, vinput->queue+i, sizeof(virtio_input_event));
+ 0, &vinput->queue[i].event,
sizeof(virtio_input_event));
virtqueue_push(vinput->evt, elem, len);
g_free(elem);
}
diff --git a/include/hw/virtio/virtio-input.h b/include/hw/virtio/virtio-input.h
index 55db310..91df57e 100644
--- a/include/hw/virtio/virtio-input.h
+++ b/include/hw/virtio/virtio-input.h
@@ -62,7 +62,10 @@ struct VirtIOInput {
VirtQueue *evt, *sts;
char *serial;
-virtio_input_event*queue;
+struct {
+virtio_input_event event;
+VirtQueueElement *elem;
+} *queue;
uint32_t qindex, qsize;
bool active;
--
2.7.4
Re: [Qemu-devel] [PATCH for-2.9 4/5] rbd: Peel off redundant RbdAuthMethod wrapper struct
On 03/24/2017 09:10 AM, Jeff Cody wrote:
> On Fri, Mar 24, 2017 at 08:49:20AM -0500, Eric Blake wrote:
>> On 03/24/2017 07:42 AM, Jeff Cody wrote:
>>
>>> Agree. My preference is to leave it as an array of methods, so long as that
>>> is tenable to libvirt.
>>
>> The -drive syntax should remain unchanged (that's an absolute must for
>> libvirt). But the QMP syntax being an array of methods sounds best to
>> me, and I think password-secret should be part of the array. So my vote
>> would be for:
>>
>> { "driver": "rbd", "image": "foo",
>> "auth": [ { "type": "cephx", "password-secret": "sec0" },
>> { "type": "none" } ],
>> "pool": "bar" }
>>
>> It makes mapping -drive arguments into QMP form a bit trickier, but the
>> QMP form is then easily extensible if we add another auth method down
>> the road.
>>
>
> In that case, what about adding user as well, and not just password-secret?
Makes sense - anything that is associated solely with cephx should
belong to the same flat-union branch for cephx, rather than at the top
level.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 1/2] migration: Disable hotplug/unplug during migration
On Thu, 23 Mar 2017 21:50:24 +0100
Juan Quintela wrote:
> Until we have reviewed what can/can't be hotplug during migration,
> disable it. We can enable it later for the things that we know that
> work. For instance, memory hotplug during postcopy don't work
> currently.
>
> Signed-off-by: Juan Quintela
> ---
> hw/core/qdev.c | 5 +
> qdev-monitor.c | 7 ++-
> 2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 1e7fb33..8c4a3f3 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -277,6 +277,11 @@ void qdev_unplug(DeviceState *dev, Error **errp)
> HotplugHandler *hotplug_ctrl;
> HotplugHandlerClass *hdc;
>
> +if (!migration_is_idle()) {
> +error_setg(errp, "device_add not allowed while migrating");
s/device_add/device_del/
I'd put this check after "if (!dc->hotpluggable)" block, so error
message won't hide wrong upluggable device error cases
> +return;
> +}
> +
> if (dev->parent_bus && !qbus_is_hotpluggable(dev->parent_bus)) {
> error_setg(errp, QERR_BUS_NO_HOTPLUG, dev->parent_bus->name);
> return;
> diff --git a/qdev-monitor.c b/qdev-monitor.c
> index 5f2fcdf..e0622b4 100644
> --- a/qdev-monitor.c
> +++ b/qdev-monitor.c
> @@ -29,7 +29,7 @@
> #include "qemu/error-report.h"
> #include "qemu/help_option.h"
> #include "sysemu/block-backend.h"
> -
> +#include "migration/migration.h"
> /*
> * Aliases were a bad idea from the start. Let's keep them
> * from spreading further.
> @@ -566,6 +566,11 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error
> **errp)
> BusState *bus = NULL;
> Error *err = NULL;
>
> +if (!migration_is_idle()) {
> +error_setg(errp, "device_add not allowed while migrating");
> +return NULL;
> +}
the same here, do it right before "/* create device */"
> driver = qemu_opt_get(opts, "driver");
> if (!driver) {
> error_setg(errp, QERR_MISSING_PARAMETER, "driver");
[Qemu-devel] [PATCH 0/2] virtio-input: eventq batching fixes
Two bugs found when reviewing the code against the candidate spec: * memory leak: the event queue is never freed * incorrect virtqueue check: virtqueue_get_avail_bytes does not guarantee number of available buffers Ladi Prosek (2): virtio-input: free event queue when finalizing virtio-input: fix eventq batching hw/input/virtio-input.c | 33 + include/hw/virtio/virtio-input.h | 5 - 2 files changed, 21 insertions(+), 17 deletions(-) Thanks! Ladi
Re: [Qemu-devel] [PULL for-2.9 0/3] Tracing patches
On 24 March 2017 at 14:08, Stefan Hajnoczi wrote: > The following changes since commit 08329701199449bde497570dcfdb9c86062baf20: > > qom: Fix regression with 'qom-type' (2017-03-23 17:59:40 +) > > are available in the git repository at: > > git://github.com/stefanha/qemu.git tags/tracing-pull-request > > for you to fetch changes up to 0d3ef78829332f2fdc323d1b625b60fe9c89119c: > > trace: Avoid abuse of amdvi_mmio_read (2017-03-24 09:21:42 +) > > > > > > Eric Blake (3): > trace: Fix backwards mirror_yield parameters > trace: Fix incorrect megasas trace parameters > trace: Avoid abuse of amdvi_mmio_read > > block/mirror.c | 5 +++-- > hw/i386/amd_iommu.c | 3 +-- > hw/scsi/megasas.c| 6 +++--- > hw/i386/trace-events | 1 + > 4 files changed, 8 insertions(+), 7 deletions(-) > Applied, thanks. -- PMM
Re: [Qemu-devel] guest-exec of qemu-guest-agent fails on Windows guest
What steps did you use to install qga? Did you use the .msi installer? If not you'll need to make those glib helper executables available to the agent by placing them in the install directory or somewhere in its executable paths. The MSI installer should handle all this for you.
Re: [Qemu-devel] [PATCH 2/2] configure: use pkg-config for obtaining xen version
> -Original Message- > From: Stefano Stabellini [mailto:sstabell...@kernel.org] > Sent: 22 March 2017 18:22 > To: Juergen Gross > Cc: Stefano Stabellini ; qemu-devel@nongnu.org; > xen-de...@lists.xenproject.org; Anthony Perard > ; kra...@redhat.com; Paul Durrant > > Subject: Re: [PATCH 2/2] configure: use pkg-config for obtaining xen version > > On Wed, 22 Mar 2017, Juergen Gross wrote: > > On 21/03/17 19:54, Stefano Stabellini wrote: > > > On Tue, 21 Mar 2017, Juergen Gross wrote: > > >> On 17/03/17 19:33, Stefano Stabellini wrote: > > >>> On Fri, 17 Mar 2017, Juergen Gross wrote: > > On 16/03/17 21:20, Stefano Stabellini wrote: > > > On Thu, 16 Mar 2017, Juergen Gross wrote: > > >> Instead of trying to guess the Xen version to use by compiling > various > > >> test programs first just ask the system via pkg-config. Only if it > > >> can't return the version fall back to the test program scheme. > > > > > > That's OK, but why did you remove the Xen unstable test? > > > > >From Xen 4.9 on pkg-config will return the needed information. > There is > > no longer a need for a test program to determine the Xen version. > After > > all this was the main objective of my series adding the pkg-config > > files to Xen. > > >>> > > >>> I was going to say something like "yeah, but is pkg-config always > > >>> available?" In reality, QEMU already has pkg-config as build > > >>> dependency, so I guess there is no problem with that. > > >>> > > >>> Please add a note about this to the commit message. > > >>> > > >> > > >> Okay. > > > > > > Sorry to point this out only now, and I realize that it might be > > > unimportant for production builds, but it is important to me, and > > > developers in general, to be able to test a single QEMU tree against a > > > number of Xen trees (all releases from 4.3 onward). > > > > > > With this change (specifically dropping the 4.9 build test), out of tree > > > builds don't work anymore. I would like to be able to do: > > > > > > ./configure --enable-xen --target-list=i386-softmmu \ > > > --extra-cflags="-I$DIR/tools/include \ > > > -I$DIR/tools/libs/toollog/include \ > > > -I$DIR/tools/libs/evtchn/include \ > > > -I$DIR/tools/libs/gnttab/include \ > > > -I$DIR/tools/libs/foreignmemory/include \ > > > -I$DIR/tools/libs/devicemodel/include \ > > > -I$DIR/tools/libxc/include \ > > > -I$DIR/tools/xenstore/include \ > > > -I$DIR/tools/xenstore/compat/include" \ > > > --extra-ldflags="-L$DIR/tools/libxc \ > > > -L$DIR/tools/xenstore \ > > > -L$DIR/tools/libs/evtchn \ > > > -L$DIR/tools/libs/gnttab \ > > > -L$DIR/tools/libs/foreignmemory \ > > > -L$DIR/tools/libs/devicemodel \ > > > -Wl,-rpath-link=$DIR/tools/libs/toollog \ > > > -Wl,-rpath-link=$DIR/tools/libs/evtchn \ > > > -Wl,-rpath-link=$DIR/tools/libs/gnttab \ > > > -Wl,-rpath-link=$DIR/tools/libs/call \ > > > -Wl,-rpath-link=$DIR/tools/libs/foreignmemory \ > > > -Wl,-rpath-link=$DIR/tools/libs/devicemodel" \ > > > --disable-kvm > > > make > > > > > > And the make should succeed. Is there a way to do that with pkg-config? > > > > Sure, for Xen 4.9 just do: > > > > PKG_CONFIG_PATH=$(DIR)/tools/pkg-config ./configure \ > > --enable-xen --target-list=i386-softmmu \ > > --disable-kvm > > make > > Yes, that works, thanks! I committed it to my next branch adding > "pkg-config, which is already a build dependency of QEMU, will be used > exclusively to determine the Xen version from Xen 4.9 onward." to the > commit message. A further question... I have a xen tree which I've been using to build and install master against my own checked out QEMU repo. No problem with that. I've now reverted my tree to 4.7.0 and cannot build tools (even after a make distclean) because QEMU's configure is still getting up a xen_ctrl_version of 40900. This is because pkg-config is still finding a 4.9.0 xencontrol package? Where is it getting this from? Paul
Re: [Qemu-devel] [PULL v3 1/3] replay: add record/replay for audio passthrough
Gerd Hoffmann writes:
> From: Pavel Dovgalyuk
>
> This patch adds recording and replaying audio data. Is saves synchronization
> information for audio out and inputs from the microphone.
>
> v2: removed unneeded whitespace change
>
> Signed-off-by: Pavel Dovgalyuk
> Message-id: 20170202055054.4848.94901.st...@pasha-isp.lan02.inno
>
> [ kraxel: add qemu/error-report.h include to fix osx build failure ]
>
> Signed-off-by: Gerd Hoffmann
> ---
> audio/audio.c| 9 --
> audio/audio.h| 5 +++
> audio/mixeng.c | 32
> docs/replay.txt | 7 +
> include/sysemu/replay.h | 7 +
> replay/Makefile.objs | 1 +
> replay/replay-audio.c| 79
>
> replay/replay-internal.h | 4 +++
> 8 files changed, 142 insertions(+), 2 deletions(-)
> create mode 100644 replay/replay-audio.c
>
> diff --git a/replay/replay-internal.h b/replay/replay-internal.h
> index c26d079..ed66ed8 100644
> --- a/replay/replay-internal.h
> +++ b/replay/replay-internal.h
> @@ -29,6 +29,10 @@ enum ReplayEvents {
> /* for character device read all event */
> EVENT_CHAR_READ_ALL,
> EVENT_CHAR_READ_ALL_ERROR,
> +/* for audio out event */
> +EVENT_AUDIO_OUT,
> +/* for audio in event */
> +EVENT_AUDIO_IN,
> /* for clock read/writes */
> /* some of greater codes are reserved for clocks */
> EVENT_CLOCK,
Well one thing I noticed while I was trying to work out the difference
between pre/post mttcg replay problems is the log format ABI has
changed. REPLAY_VERSION needs to be bumped to prevent confusion.
--
Alex Bennée
Re: [Qemu-devel] guest-exec of qemu-guest-agent fails on Windows guest
We have tried qemu-ga from Fedora [1] which didn't have guest-exec and guest-exec-status (qemu-ga -b ? didn't list them and running guest-exec returned command not found) and also build msi from HEAD ourselves. gspawn-exec files are there, when we try to run guest-exec command qemu-ga windows application (which is running on foreground for testing) fails at gspawn-win32-helper.c [2] assertion, what happens on windows is that the "gspawn-win32-helper application stopped working" dialog appears. [1] https://fedoraproject.org/wiki/Windows_Virtio_Drivers [2] https://github.com/GNOME/glib/blob/8edcf67b0221efa3c2ada67c44eff29939b1704d/glib/gspawn-win32-helper.c#L208 --- Armin ranjbar On Fri, Mar 24, 2017 at 7:33 PM, Michael Roth wrote: > What steps did you use to install qga? Did you use the .msi > installer? If not you'll need to make those glib helper executables > available to the agent by placing them in the install directory or > somewhere in its executable paths. The MSI installer should handle > all this for you. > >
Re: [Qemu-devel] [PATCH 2/2] configure: use pkg-config for obtaining xen version
> -Original Message- > From: Paul Durrant > Sent: 24 March 2017 15:13 > To: 'Stefano Stabellini' ; Juergen Gross > > Cc: qemu-devel@nongnu.org; xen-de...@lists.xenproject.org; Anthony > Perard ; kra...@redhat.com > Subject: RE: [PATCH 2/2] configure: use pkg-config for obtaining xen version > > > -Original Message- > > From: Stefano Stabellini [mailto:sstabell...@kernel.org] > > Sent: 22 March 2017 18:22 > > To: Juergen Gross > > Cc: Stefano Stabellini ; qemu-devel@nongnu.org; > > xen-de...@lists.xenproject.org; Anthony Perard > > ; kra...@redhat.com; Paul Durrant > > > > Subject: Re: [PATCH 2/2] configure: use pkg-config for obtaining xen > version > > > > On Wed, 22 Mar 2017, Juergen Gross wrote: > > > On 21/03/17 19:54, Stefano Stabellini wrote: > > > > On Tue, 21 Mar 2017, Juergen Gross wrote: > > > >> On 17/03/17 19:33, Stefano Stabellini wrote: > > > >>> On Fri, 17 Mar 2017, Juergen Gross wrote: > > > On 16/03/17 21:20, Stefano Stabellini wrote: > > > > On Thu, 16 Mar 2017, Juergen Gross wrote: > > > >> Instead of trying to guess the Xen version to use by compiling > > various > > > >> test programs first just ask the system via pkg-config. Only if it > > > >> can't return the version fall back to the test program scheme. > > > > > > > > That's OK, but why did you remove the Xen unstable test? > > > > > > >From Xen 4.9 on pkg-config will return the needed information. > > There is > > > no longer a need for a test program to determine the Xen version. > > After > > > all this was the main objective of my series adding the pkg-config > > > files to Xen. > > > >>> > > > >>> I was going to say something like "yeah, but is pkg-config always > > > >>> available?" In reality, QEMU already has pkg-config as build > > > >>> dependency, so I guess there is no problem with that. > > > >>> > > > >>> Please add a note about this to the commit message. > > > >>> > > > >> > > > >> Okay. > > > > > > > > Sorry to point this out only now, and I realize that it might be > > > > unimportant for production builds, but it is important to me, and > > > > developers in general, to be able to test a single QEMU tree against a > > > > number of Xen trees (all releases from 4.3 onward). > > > > > > > > With this change (specifically dropping the 4.9 build test), out of tree > > > > builds don't work anymore. I would like to be able to do: > > > > > > > > ./configure --enable-xen --target-list=i386-softmmu \ > > > > --extra-cflags="-I$DIR/tools/include \ > > > > -I$DIR/tools/libs/toollog/include \ > > > > -I$DIR/tools/libs/evtchn/include \ > > > > -I$DIR/tools/libs/gnttab/include \ > > > > -I$DIR/tools/libs/foreignmemory/include \ > > > > -I$DIR/tools/libs/devicemodel/include \ > > > > -I$DIR/tools/libxc/include \ > > > > -I$DIR/tools/xenstore/include \ > > > > -I$DIR/tools/xenstore/compat/include" \ > > > > --extra-ldflags="-L$DIR/tools/libxc \ > > > > -L$DIR/tools/xenstore \ > > > > -L$DIR/tools/libs/evtchn \ > > > > -L$DIR/tools/libs/gnttab \ > > > > -L$DIR/tools/libs/foreignmemory \ > > > > -L$DIR/tools/libs/devicemodel \ > > > > -Wl,-rpath-link=$DIR/tools/libs/toollog \ > > > > -Wl,-rpath-link=$DIR/tools/libs/evtchn \ > > > > -Wl,-rpath-link=$DIR/tools/libs/gnttab \ > > > > -Wl,-rpath-link=$DIR/tools/libs/call \ > > > > -Wl,-rpath-link=$DIR/tools/libs/foreignmemory \ > > > > -Wl,-rpath-link=$DIR/tools/libs/devicemodel" \ > > > > --disable-kvm > > > > make > > > > > > > > And the make should succeed. Is there a way to do that with pkg- > config? > > > > > > Sure, for Xen 4.9 just do: > > > > > > PKG_CONFIG_PATH=$(DIR)/tools/pkg-config ./configure \ > > > --enable-xen --target-list=i386-softmmu \ > > > --disable-kvm > > > make > > > > Yes, that works, thanks! I committed it to my next branch adding > > "pkg-config, which is already a build dependency of QEMU, will be used > > exclusively to determine the Xen version from Xen 4.9 onward." to the > > commit message. > > A further question... > > I have a xen tree which I've been using to build and install master against my > own checked out QEMU repo. No problem with that. I've now reverted my > tree to 4.7.0 and cannot build tools (even after a make distclean) because > QEMU's configure is still getting up a xen_ctrl_version of 40900. This is > because pkg-config is still finding a 4.9.0 xencontrol package? Where is it > getting this from? > Even in a completely fresh checkout of xen.git RELEASE-4.7.2 tag I'm *still* getting a xen_ctrl_version of 40900, which presumably means it is coming from the version of xenctrl I have *installed*
