Re: [PATCH v1 2/4] virtio: make seg_max virtqueue size dependent
The 1st patch from the series seems to be useless. The patch extending queue length by adding machine type may break vm-s which use seabios with max queue size = 128. Looks like only this patch doesn't break anything and helps to express queue size and seg max dependency (the specification constraint) explicitly. So, I would like to re-send this patch as a standalone one and send other patches including the test later, when we all agree on how exactly to deal with issues posted in the thread. Any objections are welcome. Denis On 06.11.2019 14:54, Michael S. Tsirkin wrote: > On Wed, Nov 06, 2019 at 10:07:02AM +, Denis Lunev wrote: >> On 11/5/19 9:51 PM, Michael S. Tsirkin wrote: >>> On Tue, Nov 05, 2019 at 07:11:03PM +0300, Denis Plotnikov wrote: seg_max has a restriction to be less or equal to virtqueue size according to Virtio 1.0 specification Although seg_max can't be set directly, it's worth to express this dependancy directly in the code for sanity purpose. Signed-off-by: Denis Plotnikov >>> This is guest visible so needs to be machine type dependent, right? >> we have discussed this verbally with Stefan and think that >> there is no need to add that to the machine type as: >> >> - original default was 126, which matches 128 as queue >> length in old machine types >> - queue length > 128 is not observed in the field as >> SeaBios has quirk that asserts > Well that's just the SeaBios virtio driver. Not everyone's using that to > drive their devices. > >> - if queue length will be set to something < 128 - linux >> guest will crash > Again that's just one guest driver. Not everyone is using that either. > > >> If we really need to preserve original __buggy__ behavior - >> we can add boolean property, pls let us know. >> >> Den > Looks like some drivers are buggy but I'm not sure it's > the same as saying the behavior is buggy. > So yes, I'd say it's preferable to be compatible. > > --- hw/block/virtio-blk.c | 2 +- hw/scsi/virtio-scsi.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 06e57a4d39..21530304cf 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -903,7 +903,7 @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config) blk_get_geometry(s->blk, ); memset(, 0, sizeof(blkcfg)); virtio_stq_p(vdev, , capacity); -virtio_stl_p(vdev, _max, 128 - 2); +virtio_stl_p(vdev, _max, s->conf.queue_size - 2); virtio_stw_p(vdev, , conf->cyls); virtio_stl_p(vdev, _size, blk_size); virtio_stw_p(vdev, _io_size, conf->min_io_size / blk_size); diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c index 839f120256..f7e5533cd5 100644 --- a/hw/scsi/virtio-scsi.c +++ b/hw/scsi/virtio-scsi.c @@ -650,7 +650,7 @@ static void virtio_scsi_get_config(VirtIODevice *vdev, VirtIOSCSICommon *s = VIRTIO_SCSI_COMMON(vdev); virtio_stl_p(vdev, >num_queues, s->conf.num_queues); -virtio_stl_p(vdev, >seg_max, 128 - 2); +virtio_stl_p(vdev, >seg_max, s->conf.virtqueue_size - 2); virtio_stl_p(vdev, >max_sectors, s->conf.max_sectors); virtio_stl_p(vdev, >cmd_per_lun, s->conf.cmd_per_lun); virtio_stl_p(vdev, >event_info_size, sizeof(VirtIOSCSIEvent)); -- 2.17.0
Re: guest / host buffer sharing ...
On Fri, Nov 8, 2019 at 8:22 AM Gerd Hoffmann wrote: > > > Adding a list of common properties to the spec certainly makes sense, > > > so everybody uses the same names. Adding struct-ed properties for > > > common use cases might be useful too. > > > > Why not define VIRTIO devices for wayland and friends? > > There is an out-of-tree implementation of that, so yes, that surely is > an option. > > Wayland needs (a) shared buffers, mostly for gfx data, and (b) a stream > pipe as control channel. Pretty much the same for X11, except that > shared buffers are optional because the X protocol can also squeeze all > display updates through the stream pipe. > > So, if you want allow guests talk to the host display server you can run > the stream pipe over vsock. But there is nothing for the shared > buffers ... > > We could replicate vsock functionality elsewhere. I think that happened > in the out-of-tree virtio-wayland implementation. There also was some > discussion about adding streams to virtio-gpu, slightly pimped up so you > can easily pass around virtio-gpu resource references for buffer > sharing. But given that getting vsock right isn't exactly trivial > (consider all the fairness issues when multiplexing multiple streams > over a virtqueue for example) I don't think this is a good plan. I also think vsock isn't the right fit. Defining a virtio-wayland device makes sense to me: you get the guest RAM access via virtqueues, plus the VIRTIO infrastructure (device IDs, configuration space, feature bits, and existing reusable kernel/userspace/QEMU code). Stefan
Re: guest / host buffer sharing ...
Hi, > > Adding a list of common properties to the spec certainly makes sense, > > so everybody uses the same names. Adding struct-ed properties for > > common use cases might be useful too. > > Why not define VIRTIO devices for wayland and friends? There is an out-of-tree implementation of that, so yes, that surely is an option. Wayland needs (a) shared buffers, mostly for gfx data, and (b) a stream pipe as control channel. Pretty much the same for X11, except that shared buffers are optional because the X protocol can also squeeze all display updates through the stream pipe. So, if you want allow guests talk to the host display server you can run the stream pipe over vsock. But there is nothing for the shared buffers ... We could replicate vsock functionality elsewhere. I think that happened in the out-of-tree virtio-wayland implementation. There also was some discussion about adding streams to virtio-gpu, slightly pimped up so you can easily pass around virtio-gpu resource references for buffer sharing. But given that getting vsock right isn't exactly trivial (consider all the fairness issues when multiplexing multiple streams over a virtqueue for example) I don't think this is a good plan. cheers, Gerd
Re: [PATCH v1 4/4] iotests: add test for virtio-scsi and virtio-blk machine type settings
On 07.11.2019 19:30, Cleber Rosa wrote: > On Wed, Nov 06, 2019 at 04:26:41PM -0300, Eduardo Habkost wrote: >> On Wed, Nov 06, 2019 at 11:04:16AM +0100, Max Reitz wrote: >>> On 06.11.19 10:24, Stefan Hajnoczi wrote: On Tue, Nov 05, 2019 at 07:11:05PM +0300, Denis Plotnikov wrote: > It tests proper queue size settings for all available machine types. > > Signed-off-by: Denis Plotnikov > --- > tests/qemu-iotests/267 | 154 + > tests/qemu-iotests/267.out | 1 + > tests/qemu-iotests/group | 1 + > 3 files changed, 156 insertions(+) > create mode 100755 tests/qemu-iotests/267 > create mode 100644 tests/qemu-iotests/267.out The qemu-iotests maintainers might prefer for this to be at the top-level in tests/ since it's not really an iotest, but the code itself looks fine to me: Reviewed-by: Stefan Hajnoczi >>> Good question. I don’t really mind, but it would be weird if started >>> adding all kinds of “external” qemu tests (i.e. that use QMP) in the >>> iotests directory. >>> >>> What is the alternative? Just putting it in a different directory >>> doesn’t sound that appealing to me either, because it would still depend >>> on the iotests infrastructure, right? (i.e., iotests.py and check) >> We do have tests/acceptance for simple test cases written in >> Python. What's the reason for this test case to depend on the >> iotests infrastructure? >> >> -- >> Eduardo > This test does look similar in spirit to "tests/acceptance/virtio_version.py". > > Denis, > > If you think this is more of a generic test than an IO test, and would > rather want to have it a more agnostic location, I can provide you > with tips (or a patch) to do so. It would be great! Thanks! Denis > > Cheers, > - Cleber. >
Re: [virtio-dev] Re: guest / host buffer sharing ...
On Thu, Nov 07, 2019 at 11:16:18AM +, Dr. David Alan Gilbert wrote: > * Gerd Hoffmann (kra...@redhat.com) wrote: > > Hi, > > > > > > This is not about host memory, buffers are in guest ram, everything else > > > > would make sharing those buffers between drivers inside the guest (as > > > > dma-buf) quite difficult. > > > > > > Given it's just guest memory, can the guest just have a virt queue on > > > which it places pointers to the memory it wants to share as elements in > > > the queue? > > > > Well, good question. I'm actually wondering what the best approach is > > to handle long-living, large buffers in virtio ... > > > > virtio-blk (and others) are using the approach you describe. They put a > > pointer to the io request header, followed by pointer(s) to the io > > buffers directly into the virtqueue. That works great with storage for > > example. The queue entries are tagged being "in" or "out" (driver to > > device or visa-versa), so the virtio transport can set up dma mappings > > accordingly or even transparently copy data if needed. > > > > For long-living buffers where data can potentially flow both ways this > > model doesn't fit very well though. So what virtio-gpu does instead is > > transferring the scatter list as virtio payload. Does feel a bit > > unclean as it doesn't really fit the virtio architecture. It assumes > > the host can directly access guest memory for example (which is usually > > the case but explicitly not required by virtio). It also requires > > quirks in virtio-gpu to handle VIRTIO_F_IOMMU_PLATFORM properly, which > > in theory should be handled fully transparently by the virtio-pci > > transport. > > > > We could instead have a "create-buffer" command which adds the buffer > > pointers as elements to the virtqueue as you describe. Then simply > > continue using the buffer even after completing the "create-buffer" > > command. Which isn't exactly clean either. It would likewise assume > > direct access to guest memory, and it would likewise need quirks for > > VIRTIO_F_IOMMU_PLATFORM as the virtio-pci transport tears down the dma > > mappings for the virtqueue entries after command completion. > > > > Comments, suggestions, ideas? > > What about not completing the command while the device is using the > memory? Thought about that too, but I don't think this is a good idea for buffers which exist for a long time. Example #1: A video decoder would setup a bunch of buffers and use them robin-round, so they would exist until the video playback is finished. Example #2: virtio-gpu creates a framebuffer for fbcon which exists forever. And virtio-gpu potentially needs lots of buffers. With 3d active there can be tons of objects. Although they typically don't stay around that long we would still need a pretty big virtqueue to store them all I guess. And it also doesn't fully match the virtio spirit, it still assumes direct guest memory access. Without direct guest memory access updates to the fbcon object would never reach the host for example. In case a iommu is present we might need additional dma map flushes for updates happening after submitting the lingering "create-buffer" command. cheers, Gerd
Deprecating stuff for 4.2 (was: [Qemu-devel] Exposing feature deprecation to machine clients)
Vladimir Sementsov-Ogievskiy writes: > 07.11.2019 21:52, Philippe Mathieu-Daudé wrote: [...] >> Pre-release period, time to deprecate some stuffs :) >> >> How should we proceed? Do you have something in mind? >> >> There are older threads about this. Should we start a new thread? Gather the >> different ideas on the Wiki? >> >> (Obviously you are not the one responsible of this topic, you just happen to >> be the last one worried about it on the list). >> >> Regards, >> >> Phil. 4.2.0-rc0 has been tagged, i.e. we're in hard freeze already. Only bug fixes are accepted during hard freeze. We've occasionally bent this rule after -rc0 for borderline cases, e.g. to tweak a new external interface before the release calcifies it. Making a case for bending the rules becomes harder with each -rc. Ideally, we'd double-check new interfaces for gaffes before a release, and whether old interfaces that have been replaced now should be deprecated. There's rarely time for that, and pretty much never for releases right after KVM Forum. So no, I don't have anything in mind for 4.2. We intend to tag -rc1 next Tuesday. To make that deadline, we'd need patches, not just ideas. > Hi! > > I wanted to resend, but faced some problems, and understand that I can't do > it in time before soft-freeze.. > But you say, that we can deprecate something even after hard-freeze? See above. > Ok, the problem that I faced, is that deprecation warnings breaks some > iotests.. What can we do? > > 1. Update iotests... >1.1 Just update iotests outputs to show warnings. Then, in next release > cycle, update iotests, to not use deprecated things Sounds workable to me, but I'm not the maintainer. >or >1.2 Update iotests to not use deprecated things.. Not appropriate for hard > freeze. Unnecessarily risky compared to 1.1. > or > 2. Commit deprecations without warnings.. But how do people find out about > this? Not nice. We do it for QMP, but only because we still lack the means to warn there. > Next, what exactly to deprecate? As I understand, we can't deprecate > drive-mirror now? > So I propose to: > > 1. deprecate drive-backup > 2. add optional filter-node-name parameter to drive-mirror, to correspond to > commit and mirror > 3. deprecate that filter-node-name is optional for commit and mirror. To have a chance there, we need patches a.s.a.p.
Re: [PATCH v14 03/11] tests: Add test for QAPI builtin type time
On 11/7/2019 9:31 PM, Eduardo Habkost wrote:
On Thu, Nov 07, 2019 at 02:24:52PM +0800, Tao Xu wrote:
On 11/7/2019 4:53 AM, Eduardo Habkost wrote:
On Mon, Oct 28, 2019 at 03:52:12PM +0800, Tao Xu wrote:
Add tests for time input such as zero, around limit of precision,
signed upper limit, actual upper limit, beyond limits, time suffixes,
and etc.
Signed-off-by: Tao Xu
---
[...]
+/* Close to signed upper limit 0x7c00 (53 msbs set) */
+qdict = keyval_parse("time1=9223372036854774784," /* 7c00 */
+ "time2=9223372036854775295", /* 7dff */
+ NULL, _abort);
+v = qobject_input_visitor_new_keyval(QOBJECT(qdict));
+qobject_unref(qdict);
+visit_start_struct(v, NULL, NULL, 0, _abort);
+visit_type_time(v, "time1", , _abort);
+g_assert_cmphex(time, ==, 0x7c00);
+visit_type_time(v, "time2", , _abort);
+g_assert_cmphex(time, ==, 0x7c00);
I'm confused by this test case and the one below[1]. Are these
known bugs? Shouldn't we document them as known bugs?
Because do_strtosz() or do_strtomul() actually parse with strtod(), so the
precision is 53 bits, so in these cases, 7dff and
fbff are rounded.
My questions remain: why isn't this being treated like a bug?
Hi Markus,
I am confused about the code here too. Because in do_strtosz(), the
upper limit is
val * mul >= 0xfc00
So some data near 53 bit may be rounded. Is there a bug?
Re: The problems about COLO
Lukas Straub 於 2019年11月7日 週四 下午9:34寫道： > On Thu, 7 Nov 2019 16:14:43 +0800 > Daniel Cho wrote: > > > Hi Lukas, > > Thanks for your reply. > > > > However, we test the question 1 with steps below the error message, we > > notice the secondary VM's image > > will break while it reboots. > > Here is the error message. > > --- > > [1.280299] XFS (sda1): Mounting V5 Filesystem > > [1.428418] input: ImExPS/2 Generic Explorer Mouse as > > /devices/platform/i8042/serio1/input/input2 > > [1.501320] XFS (sda1): Starting recovery (logdev: internal) > > [1.504076] tsc: Refined TSC clocksource calibration: 3492.211 MHz > > [1.505534] Switched to clocksource tsc > > [2.031027] XFS (sda1): Internal error XFS_WANT_CORRUPTED_GOTO at line > > 1635 of file fs/xfs/libxfs/xfs_alloc.c. Caller > xfs_free_extent+0xfc/0x130 > > [xfs] > > [2.032743] CPU: 0 PID: 300 Comm: mount Not tainted > > 3.10.0-693.11.6.el7.x86_64 #1 > > [2.033982] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), > BIOS > > rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org 04/01/2014 > > [2.035882] Call Trace: > > [2.036494] [] dump_stack+0x19/0x1b > > [2.037315] [] xfs_error_report+0x3b/0x40 [xfs] > > [2.038150] [] ? xfs_free_extent+0xfc/0x130 [xfs] > > [2.039046] [] xfs_free_ag_extent+0x20a/0x780 [xfs] > > [2.039920] [] xfs_free_extent+0xfc/0x130 [xfs] > > [2.040768] [] xfs_trans_free_extent+0x26/0x60 > [xfs] > > [2.041642] [] xlog_recover_process_efi+0x17e/0x1c0 > > [xfs] > > [2.042558] [] > > xlog_recover_process_efis.isra.30+0x77/0xe0 [xfs] > > [2.043771] [] xlog_recover_finish+0x21/0xb0 [xfs] > > [2.044650] [] xfs_log_mount_finish+0x34/0x50 [xfs] > > [2.045518] [] xfs_mountfs+0x5d1/0x8b0 [xfs] > > [2.046341] [] ? > xfs_filestream_get_parent+0x80/0x80 > > [xfs] > > [2.047260] [] xfs_fs_fill_super+0x3bb/0x4d0 [xfs] > > [2.048116] [] mount_bdev+0x1b0/0x1f0 > > [2.048881] [] ? > > xfs_test_remount_options.isra.11+0x70/0x70 [xfs] > > [2.050105] [] xfs_fs_mount+0x15/0x20 [xfs] > > [2.050906] [] mount_fs+0x39/0x1b0 > > [2.051963] [] ? __alloc_percpu+0x15/0x20 > > [2.059431] [] vfs_kern_mount+0x67/0x110 > > [2.060283] [] do_mount+0x233/0xaf0 > > [2.061081] [] ? strndup_user+0x4b/0xa0 > > [2.061844] [] SyS_mount+0x96/0xf0 > > [2.062619] [] system_call_fastpath+0x16/0x1b > > [2.063512] XFS (sda1): Internal error xfs_trans_cancel at line 984 of > > file fs/xfs/xfs_trans.c. Caller xlog_recover_process_efi+0x18e/0x1c0 > [xfs] > > [2.065260] CPU: 0 PID: 300 Comm: mount Not tainted > > 3.10.0-693.11.6.el7.x86_64 #1 > > [2.066489] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), > BIOS > > rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org 04/01/2014 > > [2.068023] Call Trace: > > [2.068590] [] dump_stack+0x19/0x1b > > [2.069403] [] xfs_error_report+0x3b/0x40 [xfs] > > [2.070318] [] ? > xlog_recover_process_efi+0x18e/0x1c0 > > [xfs] > > [2.071538] [] xfs_trans_cancel+0xbd/0xe0 [xfs] > > [2.072429] [] xlog_recover_process_efi+0x18e/0x1c0 > > [xfs] > > [2.073339] [] > > xlog_recover_process_efis.isra.30+0x77/0xe0 [xfs] > > [2.074561] [] xlog_recover_finish+0x21/0xb0 [xfs] > > [2.075421] [] xfs_log_mount_finish+0x34/0x50 [xfs] > > [2.076301] [] xfs_mountfs+0x5d1/0x8b0 [xfs] > > [2.077128] [] ? > xfs_filestream_get_parent+0x80/0x80 > > [xfs] > > [2.078049] [] xfs_fs_fill_super+0x3bb/0x4d0 [xfs] > > [2.078900] [] mount_bdev+0x1b0/0x1f0 > > [2.079667] [] ? > > xfs_test_remount_options.isra.11+0x70/0x70 [xfs] > > [2.080883] [] xfs_fs_mount+0x15/0x20 [xfs] > > [2.081687] [] mount_fs+0x39/0x1b0 > > [2.082457] [] ? __alloc_percpu+0x15/0x20 > > [2.083258] [] vfs_kern_mount+0x67/0x110 > > [2.084057] [] do_mount+0x233/0xaf0 > > [2.084797] [] ? strndup_user+0x4b/0xa0 > > [2.085568] [] SyS_mount+0x96/0xf0 > > [2.086324] [] system_call_fastpath+0x16/0x1b > > [2.087161] XFS (sda1): xfs_do_force_shutdown(0x8) called from line > 985 > > of file fs/xfs/xfs_trans.c. Return address = 0xc0195966 > > [2.088795] XFS (sda1): Corruption of in-memory data detected. > Shutting > > down filesystem > > [2.090273] XFS (sda1): Please umount the filesystem and rectify the > > problem(s) > > [2.091519] XFS (sda1): Failed to recover EFIs > > [2.092299] XFS (sda1): log mount finish failed > > [FAILED] Failed to mount /sysroot. > > . > > . > > . > > Generating "/run/initramfs/rdsosreport.txt" > > [2.178103] blk_update_request: I/O error, dev fd0, sector 0 > > [2.246106] blk_update_request: I/O error, dev fd0, sector 0 > > --- > > > > Here is the replicated steps: > > *1. Start primary VM with command, and do every thing you want on PVM* > > qemu-system-x86_64
RE: [PATCH V2 0/4] Introduce Advanced Watch Dog module
Hi~ All~ Ping Anyone have time to review this series? I need more comments~ Thanks Zhang Chen > -Original Message- > From: Zhang, Chen > Sent: Friday, November 1, 2019 10:49 AM > To: Jason Wang ; Paolo Bonzini > ; Philippe Mathieu-Daudé ; > qemu-dev > Cc: Zhang Chen ; Zhang, Chen > > Subject: [PATCH V2 0/4] Introduce Advanced Watch Dog module > > From: Zhang Chen > > Advanced Watch Dog is an universal monitoring module on VMM side, it can > be used to detect network down(VMM to guest, VMM to VMM, VMM to > another remote server) and do previously set operation. Current AWD patch > just accept any input as the signal to refresh the watchdog timer, and we can > also make a certain interactive protocol here. For the output user can pre- > write some command or some messages in the AWD opt-script. We noticed > that there is no way for VMM communicate directly, maybe some people > think we don't need such things(up layer software like openstack can handle > it). But we engaged with real customer found that in some cases,they need a > lightweight and efficient mechanism to solve some practical > problems(openstack is too heavy). > for example: When it detects lost connection with the paired node,it will > send message to admin, notify another VMM, send qmp command to qemu > do some operation like restart the VM, build VMM heartbeat system, etc. > It make user have basic VM/Host network monitoring tools and basic false > tolerance and recovery solution. > > Demo usage(for COLO heartbeat service): > > In primary node: > > -chardev socket,id=h1,host=3.3.3.3,port=9009,server,nowait > -chardev socket,id=heartbeat0,host=3.3.3.3,port=4445 > -object iothread,id=iothread2 > -object advanced- > watchdog,id=heart1,server=on,awd_node=h1,notification_node=heartbeat > 0,opt_script=colo_opt_script_path,iothread=iothread1,pulse_interval=1000, > timeout=5000 > > In secondary node: > > -monitor tcp::4445,server,nowait > -chardev socket,id=h1,host=3.3.3.3,port=9009,reconnect=1 > -chardev socket,id=heart1,host=3.3.3.8,port=4445 > -object iothread,id=iothread1 > -object advanced- > watchdog,id=heart1,server=off,awd_node=h1,notification_node=heart1,op > t_script=colo_secondary_opt_script,iothread=iothread1,timeout=1 > > > V2: > - Addressed Philippe comments add configure selector for AWD. > > Initial: > - Initial version. > > Zhang Chen (4): > net/awd.c: Introduce Advanced Watch Dog module framework > net/awd.c: Initailize input/output chardev > net/awd.c: Load advanced watch dog worker thread job > vl.c: Make Advanced Watch Dog delayed initialization > > configure | 9 + > net/Makefile.objs | 1 + > net/awd.c | 491 > ++ > qemu-options.hx | 6 + > vl.c | 7 + > 5 files changed, 514 insertions(+) > create mode 100644 net/awd.c > > -- > 2.17.1
Re: Looking for issues/features for my first contribution
On 07-11-2019 07:33, Aleksandar Markovic wrote: I did a quick Google search on datasheets of existing RTC implemtations, and the result is: DS1338: https://datasheets.maximintegrated.com/en/ds/DS1338-DS1338Z.pdf M41T80: https://www.st.com/resource/en/datasheet/m41t80.pdf M48T59: http://www.elektronikjk.pl/elementy_czynne/IC/M48T59V.pdf MC146818: https://www.nxp.com/docs/en/data-sheet/MC146818.pdf PL031: http://infocenter.arm.com/help/topic/com.arm.doc.ddi0224c/real_time_clock_pl031_r1p3_technical_reference_manual_DDI0224C.pdf TWL92230: https://datasheet.octopart.com/TWL92230C-Texas-Instruments-datasheet-150321.pdf Zynq RTC: https://www.xilinx.com/support/documentation/user_guides/ug1085-zynq-ultrascale-trm.pdf (chapter 7) I have a few questions about this: [a] Is there any particular reason that you picked DS3231 ? Linux kernel has drivers for DS3232/34 only [1]. I did read the datasheets of both 3232 & 3231 and found that they are quite similar except for the 236 bytes of SRAM support found only in 3232. [b] As per the datasheet, DS3231 has a built-in temperature sensor. Temperature can be read from a dedicated register. There can be two approaches to emulating this: (1) Return a constant temperature value on every read (2) Throw a not-supported exception/warning. What is the qemu convention for handling such features ? [c] DS3231 also has programmable square-wave output + 32 KHz output pin. M41T80 chip also supports this feature. However, qemu does not support emulation of these features [2]. Do I take the same approach ? Thanks! Rajath Shashidhara References: [1] https://elixir.bootlin.com/linux/v5.4-rc6/source/drivers/rtc/rtc-ds3232.c [2] https://git.qemu.org/?p=qemu.git;a=blob;f=hw/rtc/m41t80.c;h=914ecac8f4db418633d6daf92608cb50f6b89052;hb=HEAD
Re: [RFC v2 11/14] linux-headers/kvm.h: add capability to forward hypercall
On 2019/11/7 20:12, Cornelia Huck wrote:
On Thu, 7 Nov 2019 19:57:22 +0800
Guoheyi wrote:
On 2019/11/7 16:57, Michael S. Tsirkin wrote:
On Thu, Nov 07, 2019 at 09:44:36AM +0800, Guoheyi wrote:
On 2019/11/7 1:55, Cornelia Huck wrote:
On Tue, 5 Nov 2019 17:10:53 +0800
Heyi Guo wrote:
To keep backward compatibility, we add new KVM capability
"KVM_CAP_FORWARD_HYPERCALL" to probe whether KVM supports forwarding
hypercall to userspace.
The capability should be enabled explicitly, for we don't want user
space application to deal with unexpected hypercall exits. After
enabling this cap, all HVC calls unhandled by kvm will be forwarded to
user space.
Signed-off-by: Heyi Guo
Cc: Peter Maydell
Cc: "Michael S. Tsirkin"
Cc: Cornelia Huck
Cc: Paolo Bonzini
Cc: Dave Martin
Cc: Marc Zyngier
Cc: Mark Rutland
Cc: James Morse
---
linux-headers/linux/kvm.h | 1 +
target/arm/sdei.c | 16
target/arm/sdei.h | 2 ++
3 files changed, 19 insertions(+)
diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
index 3d9b18f7f8..36c9b3859f 100644
--- a/linux-headers/linux/kvm.h
+++ b/linux-headers/linux/kvm.h
@@ -1000,6 +1000,7 @@ struct kvm_ppc_resize_hpt {
#define KVM_CAP_PMU_EVENT_FILTER 173
#define KVM_CAP_ARM_IRQ_LINE_LAYOUT_2 174
#define KVM_CAP_HYPERV_DIRECT_TLBFLUSH 175
+#define KVM_CAP_FORWARD_HYPERCALL 176
#ifdef KVM_CAP_IRQ_ROUTING
Is this cap upstream already? I would have thought your header sync
would have brought it in, then. (Saying this, that header sync looks
awfully small.)
If it is not upstream yet, please split off this hunk into a separate
patch -- it's a bit annoying, but makes life easier for merging.
No, it is not upstream yet. The whole framework and interfaces between KVM
and qemu are still under discussion. I'll keep in mind of this when moving
forward to next steps...
Thanks,
HG
It's best to add it in some other place meanwhile.
Do you mean to split this patch from the whole patch set and send it
separately? Sorry I'm not clear about maintainers' work and may bring
you some trouble...
My preferred approach:
- add a commit entitled "placeholder for headers update" that contains
the not-yet-upstream changes in the header files you need
- base the rest of your work on that
...
...
- if kernel changes are upstream: replace the placeholder patch with a
real update (may include separate patches, if you need an additional
header); maintainer merges
- if kernel changes are not yet upstream: maintainer merges with
placeholder to a feature branch, replaces with real update and merges
once kernel patches hit upstream
(not every maintainer does the second approach; they may ask you
instead to resend with a proper headers update once the kernel changes
are upstream)
Thanks a lot. I'll do that in the next version.
HG
.
Re: [PATCH v15 00/12] Build ACPI Heterogeneous Memory Attribute Table (HMAT)
On 11/7/2019 5:03 PM, Michael S. Tsirkin wrote: On Thu, Nov 07, 2019 at 03:44:59PM +0800, Tao Xu wrote: This series of patches will build Heterogeneous Memory Attribute Table (HMAT) according to the command line. The ACPI HMAT describes the memory attributes, such as memory side cache attributes and bandwidth and latency details, related to the Memory Proximity Domain. The software is expected to use HMAT information as hint for optimization. In the linux kernel, the codes in drivers/acpi/hmat/hmat.c parse and report the platform's HMAT tables. ok this looks good to me. Given we are in freeze, please ping me after the release to merge this. Thank you very much! The V14 patches link: https://patchwork.kernel.org/cover/11214887/ Changelog: v15: - Add a new patch to refactor do_strtosz() (Eduardo) - Make tests without breaking CI (Michael) v14: - Reuse the codes of do_strtosz to build qemu_strtotime_ns (Eduardo) - Squash patch v13 01/12 and 02/12 together (Daniel and Eduardo) - Drop time unit picosecond (Eric) - Use qemu ctz64 and clz64 instead of builtin function v13: - Modify some text description - Drop "initiator_valid" field in struct NodeInfo - Reuse Garray to store the raw bandwidth and bandwidth data - Calculate common base unit using range bitmap - Add a patch to alculate hmat latency and bandwidth entry list - Drop the total_levels option and use readable cache size - Remove the unnecessary head file - Use decimal notation with appropriate suffix for cache size v12: - Fix a bug that a memory-only node without initiator setting doesn't report error. (reported by Danmei Wei) - Fix a bug that if HMAT is enabled and without hmat-lb setting, QEMU will crash. (reported by Danmei Wei) v11: - Move numa option patches forward. - Add num_initiator in Numa_state to record the number of initiators. - Simplify struct HMAT_LB_Info, use uint64_t array to store data. - Drop hmat_get_base(). - Calculate base in build_hmat_lb(). v10: - Add qemu_strtotime_ps() to convert strings with time suffixes to numbers, and add some tests for it. - Add qapi buildin type time, and add some tests for it. - Add machine oprion properties "-machine hmat=on|off" for enabling or disabling HMAT in QEMU. Liu Jingqi (5): numa: Extend CLI to provide memory latency and bandwidth information numa: Extend CLI to provide memory side cache information hmat acpi: Build Memory Proximity Domain Attributes Structure(s) hmat acpi: Build System Locality Latency and Bandwidth Information Structure(s) hmat acpi: Build Memory Side Cache Information Structure(s) Tao Xu (7): util/cutils: refactor do_strtosz() to support suffixes list util/cutils: Add qemu_strtotime_ns() qapi: Add builtin type time tests: Add test for QAPI builtin type time numa: Extend CLI to provide initiator information for numa nodes numa: Calculate hmat latency and bandwidth entry list tests/bios-tables-test: add test cases for ACPI HMAT hw/acpi/Kconfig | 7 +- hw/acpi/Makefile.objs | 1 + hw/acpi/hmat.c| 263 hw/acpi/hmat.h| 42 hw/core/machine.c | 64 ++ hw/core/numa.c| 284 +- hw/i386/acpi-build.c | 5 + include/qapi/visitor-impl.h | 4 + include/qapi/visitor.h| 8 + include/qemu/cutils.h | 1 + include/sysemu/numa.h | 104 ++ qapi/machine.json | 178 +++- qapi/opts-visitor.c | 22 ++ qapi/qapi-visit-core.c| 12 ++ qapi/qobject-input-visitor.c | 18 ++ qapi/trace-events | 1 + qemu-options.hx | 96 - scripts/qapi/schema.py| 1 + tests/bios-tables-test-allowed-diff.h | 8 + tests/bios-tables-test.c | 44 tests/data/acpi/pc/APIC.acpihmat | 0 tests/data/acpi/pc/DSDT.acpihmat | 0 tests/data/acpi/pc/HMAT.acpihmat | 0 tests/data/acpi/pc/SRAT.acpihmat | 0 tests/data/acpi/q35/APIC.acpihmat | 0 tests/data/acpi/q35/DSDT.acpihmat | 0 tests/data/acpi/q35/HMAT.acpihmat | 0 tests/data/acpi/q35/SRAT.acpihmat | 0 tests/test-cutils.c | 204 ++ tests/test-keyval.c | 122 +++ tests/test-qobject-input-visitor.c| 29 +++ util/cutils.c | 86 +--- 32 files changed, 1562 insertions(+), 42 deletions(-) create mode 100644 hw/acpi/hmat.c create mode 100644 hw/acpi/hmat.h create mode 100644 tests/data/acpi/pc/APIC.acpihmat create mode 100644
Re: [Patch v2 5/6] migration/postcopy: enable random order target page arrival
On Thu, Nov 07, 2019 at 02:30:25PM +, Dr. David Alan Gilbert wrote: >* Wei Yang (richardw.y...@linux.intel.com) wrote: >> After using number of target page received to track one host page, we >> could have the capability to handle random order target page arrival in >> one host page. >> >> This is a preparation for enabling compress during postcopy. >> >> Signed-off-by: Wei Yang > >Yep, that's better > >Reviewed-by: Dr. David Alan Gilbert > Thanks for your suggestion :-) -- Wei Yang Help you, Help me
[ANNOUNCE] QEMU 4.2.0-rc0 is now available
Hello, On behalf of the QEMU Team, I'd like to announce the availability of the first release candidate for the QEMU 4.2 release. This release is meant for testing purposes and should not be used in a production environment. http://download.qemu-project.org/qemu-4.2.0-rc0.tar.xz http://download.qemu-project.org/qemu-4.2.0-rc0.tar.xz.sig A note from the maintainer: The rc0 is a couple of days later than scheduled because the planned schedule coincided with KVM Forum and I was travelling or on holiday for the first part of this week, and there was a bit of a backlog of pull requests I wanted to get in before rc0. rc1 should be on Tuesday next week as planned. You can help improve the quality of the QEMU 4.2 release by testing this release and reporting bugs on Launchpad: https://bugs.launchpad.net/qemu/ The release plan, as well a documented known issues for release candidates, are available at: http://wiki.qemu.org/Planning/4.2 Please add entries to the ChangeLog for the 4.2 release below: http://wiki.qemu.org/ChangeLog/4.2 Thank you to everyone involved!
RE: [PATCH v2] WHPX: support for xcr0
> > You will need the Windows 10 SDK for RS5 (build 17763) or above to > > to be able to compile this patch because of the definition of the > > XCR0 register. > > > > Changes since v1: > > - Added a sign-off line in the patch. > > > I am not very happy with the current situation which suggests using non > free header files from the Microsoft Windows SDK, thus making it > impossible to produce QEMU executables for Windows with WHPX support > without having legal complications. > > Could you please add the required headers with a suitable license to the > QEMU source code? That would clarify the license issue and make builds > with WHPX much easier because those files would not have to be extracted > from a very large SDK installation. > > It would also be acceptable if Microsoft could update the license > comments in those files and use a QEMU compatible license. > I agree in principle that there should be an easier way to consume the Windows SDK headers without having to play around with the licenses. I also agree that that will make life lot easier for many developers. I am reaching out internally here to see what can be done about this, but, that might take some time. Meanwhile, is it possible to make some progress on this patch? > Kind regards > Stefan Weil > >
Re: [PATCH 4/4] Added tests for close and change of logfile.
On Thu, 7 Nov 2019 at 15:12, Alex Bennée wrote:
>
>
> > For a fix, we could put this at the beginning of qemu_set_log_filename().
> > if (logfilename) {
> > g_free(logfilename);
> > logfilename = NULL;
> > }
>
> g_free(logfilename) should be safe against NULL. However we need to
> ensure that logfilename is NULL'ed after it so we don't see the double
> free.
>
This makes sense, will remove the if (logfilename).
> > We were curious to understand why we did not see it in our own
> > testing. Although we did run make check before our first post, we did
> > not see this issue. The docker tests seem to use something like
> > MALLOC_CHECK_, which catches memory issues like this. We will be
> > sure to run the docker tests as well in the future.
>
> I was just running in my normal checkout - it could depend on how glibc
> was built for your system though. Mine is Debian Buster.
>
Interesting. We had assumed it was just the way we were running the
test. But it sounds like something about our particular setup. I'm
using Ubuntu bionic on an aarch64 system. Will look further.
Thanks,
-Rob Foley
On Thu, 7 Nov 2019 at 15:12, Alex Bennée wrote:
>
>
> Robert Foley writes:
>
> > Thanks for providing the stack trace.
> >
> > We debugged this and it seems to come about because of an interesting
> > circumstance. We added our new tests after a pre-existing test,
> > parse_path(), which runs into an issue, a dangling pointer, which
> > could lead to a double free. There were no other tests after the test
> > that ran into the issue, so the double free was not exposed until we
> > added our test which called qemu_set_log_filename().
> >
> > Upon entry to qemu_set_log_filename() it frees logfilename. In the
> > case where we get an error, we return out without setting the
> > logfilename to NULL.
> > And on next call into this function we will end up with a double free.
> >
> > For a fix, we could put this at the beginning of qemu_set_log_filename().
> > if (logfilename) {
> > g_free(logfilename);
> > logfilename = NULL;
> > }
>
> g_free(logfilename) should be safe against NULL. However we need to
> ensure that logfilename is NULL'ed after it so we don't see the double
> free.
>
> > We were curious to understand why we did not see it in our own
> > testing. Although we did run make check before our first post, we did
> > not see this issue. The docker tests seem to use something like
> > MALLOC_CHECK_, which catches memory issues like this. We will be
> > sure to run the docker tests as well in the future.
>
> I was just running in my normal checkout - it could depend on how glibc
> was built for your system though. Mine is Debian Buster.
>
> >
> > On Thu, 7 Nov 2019 at 12:26, Alex Bennée wrote:
> >>
> >>
> >> Alex Bennée writes:
> >>
> >> > Robert Foley writes:
> >> >
> >> >> One test ensures that the logfile handle is still valid even if
> >> >> the logfile is changed during logging.
> >> >> The other test validates that the logfile handle remains valid under
> >> >> the logfile lock even if the logfile is closed.
> >>
> >> Also this doesn't see to work:
> >>
> >> 17:24:31 [alex@zen:~/l/q/b/all] review/rcu-logfile|… 2 +
> >> ./tests/test-logging
> >> /logging/parse_range: OK
> >> /logging/parse_path: OK
> >> /logging/logfile_write_path: free(): double free detected in tcache 2
> >> fish: “./tests/test-logging” terminated by signal SIGABRT (Abort)
> >>
> >> in gdb
> >>
> >> Starting program: /home/alex/lsrc/qemu.git/builds/all/tests/test-logging
> >> [Thread debugging using libthread_db enabled]
> >> Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
> >> [New Thread 0x76f38700 (LWP 28960)]
> >> /logging/parse_range: OK
> >> /logging/parse_path: OK
> >> /logging/logfile_write_path: free(): double free detected in tcache 2
> >>
> >> Thread 1 "test-logging" received signal SIGABRT, Aborted.
> >> __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
> >> 50 ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
> >> (gdb) bt
> >> #0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
> >> #1 0x77587535 in __GI_abort () at abort.c:79
> >> #2 0x775de508 in __libc_message (action=action@entry=do_abort,
> >> fmt=fmt@entry=0x776e928d "%s\n") at ../sysdeps/posix/libc_fatal.c:181
> >> #3 0x775e4c1a in malloc_printerr (str=str@entry=0x776eaf58
> >> "free(): double free detected in tcache 2") at malloc.c:5341
> >> #4 0x775e66fd in _int_free (av=0x77720c40 ,
> >> p=0x555cac40, have_lock=) at malloc.c:4193
> >> #5 0x555614a8 in qemu_set_log_filename (filename=0x555cb110
> >> "/tmp/qemu-test-logging.RO35A0/qemu_test_log_write0.log",
> >> errp=0x7fffdef0) at /home/alex/lsrc/qemu.git/util/log.c:148
> >> #6 0xd8be in test_logfile_write (data=0x555c7370) at
> >> /home/alex/lsrc/qemu.git/tests/test-logging.c:127
> >> #7
Re: [PATCH 0/3] Some memory leak fixes
Patchew URL: https://patchew.org/QEMU/20191107192731.17330-1-marcandre.lur...@redhat.com/ Hi, This series failed the docker-quick@centos7 build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #!/bin/bash make docker-image-centos7 V=1 NETWORK=1 time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1 === TEST SCRIPT END === TESTcheck-unit: tests/test-thread-pool TESTcheck-unit: tests/test-hbitmap ** ERROR:/tmp/qemu-test/src/tests/migration-test.c:903:wait_for_migration_fail: assertion failed: (!strcmp(status, "setup") || !strcmp(status, "failed") || (allow_active && !strcmp(status, "active"))) ERROR - Bail out! ERROR:/tmp/qemu-test/src/tests/migration-test.c:903:wait_for_migration_fail: assertion failed: (!strcmp(status, "setup") || !strcmp(status, "failed") || (allow_active && !strcmp(status, "active"))) make: *** [check-qtest-aarch64] Error 1 make: *** Waiting for unfinished jobs TESTiotest-qcow2: 013 TESTcheck-unit: tests/test-bdrv-drain --- raise CalledProcessError(retcode, cmd) subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=7099ba6365db4fc0b071e2fac87b5c55', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-softdboz/src/docker-src.2019-11-07-16.44.12.4395:/var/tmp/qemu:z,ro', 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2. filter=--filter=label=com.qemu.instance.uuid=7099ba6365db4fc0b071e2fac87b5c55 make[1]: *** [docker-run] Error 1 make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-softdboz/src' make: *** [docker-run-test-quick@centos7] Error 2 real13m27.012s user0m8.295s The full log is available at http://patchew.org/logs/20191107192731.17330-1-marcandre.lur...@redhat.com/testing.docker-quick@centos7/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [PATCH 1/4] Add a mutex to guarantee single writer to qemu_logfile handle.
On Thu, 7 Nov 2019 at 11:53, Alex Bennée wrote:
>
> It wouldn't be the worst thing in the world to expose:
>
> qemu_logfile_init()
>
> and make vl.c and main.c call it before the setup. Then you can drop the
> flag or even just g_assert(qemu_log_mutex_initialised) in qemu_set_log
> and qemu_set_logfile.
>
> In fact you could just use:
>
> static void __attribute__((__constructor__)) qemu_logfile_init(void)
>
> and make the compiler do it for you.
All good ideas. Will make the changes.
I agree, it is much cleaner to call init this way (constructor). We
can assert that qemu_log_mutex.initialized on use of the mutex
(qemu_set_log and qemu_set_logfile). Taking that one step further, we
could add simple helper functions for
qemu_logfile_mutex_lock()/unlock(), which g_assert() on
mutex.initialized first before lock/unlock.
Thanks,
-Rob
On Thu, 7 Nov 2019 at 11:53, Alex Bennée wrote:
>
>
> Robert Foley writes:
>
> > This is being added in preparation for using RCU with the logfile handle.
> > Also added qemu_logfile_init() for initializing the logfile mutex.
> >
> > Signed-off-by: Robert Foley
> > ---
> > util/log.c | 23 +++
> > 1 file changed, 23 insertions(+)
> >
> > diff --git a/util/log.c b/util/log.c
> > index 1ca13059ee..dff2f98c8c 100644
> > --- a/util/log.c
> > +++ b/util/log.c
> > @@ -24,8 +24,11 @@
> > #include "qapi/error.h"
> > #include "qemu/cutils.h"
> > #include "trace/control.h"
> > +#include "qemu/thread.h"
> >
> > static char *logfilename;
> > +static bool qemu_logfile_initialized;
> > +static QemuMutex qemu_logfile_mutex;
> > FILE *qemu_logfile;
> > int qemu_loglevel;
> > static int log_append = 0;
> > @@ -49,6 +52,14 @@ int qemu_log(const char *fmt, ...)
> > return ret;
> > }
> >
> > +static void qemu_logfile_init(void)
> > +{
> > +if (!qemu_logfile_initialized) {
> > +qemu_mutex_init(_logfile_mutex);
> > +qemu_logfile_initialized = true;
> > +}
> > +}
> > +
> > static bool log_uses_own_buffers;
> >
> > /* enable or disable low levels log */
> > @@ -58,6 +69,12 @@ void qemu_set_log(int log_flags)
> > #ifdef CONFIG_TRACE_LOG
> > qemu_loglevel |= LOG_TRACE;
> > #endif
> > +
> > +/* Is there a better place to call this to init the logfile subsystem?
> > */
> > +if (!qemu_logfile_initialized) {
> > +qemu_logfile_init();
> > +}
>
> It wouldn't be the worst thing in the world to expose:
>
> qemu_logfile_init()
>
> and make vl.c and main.c call it before the setup. Then you can drop the
> flag or even just g_assert(qemu_log_mutex_initialised) in qemu_set_log
> and qemu_set_logfile.
>
> In fact you could just use:
>
> static void __attribute__((__constructor__)) qemu_logfile_init(void)
>
> and make the compiler do it for you.
>
> > +qemu_mutex_lock(_logfile_mutex);
> > if (!qemu_logfile &&
> > (is_daemonized() ? logfilename != NULL : qemu_loglevel)) {
> > if (logfilename) {
> > @@ -93,6 +110,7 @@ void qemu_set_log(int log_flags)
> > log_append = 1;
> > }
> > }
> > +qemu_mutex_unlock(_logfile_mutex);
> > if (qemu_logfile &&
> > (is_daemonized() ? logfilename == NULL : !qemu_loglevel)) {
> > qemu_log_close();
> > @@ -114,6 +132,11 @@ void qemu_set_log_filename(const char *filename, Error
> > **errp)
> > char *pidstr;
> > g_free(logfilename);
> >
> > +/* Is there a better place to call this to init the logfile subsystem?
> > */
> > +if (!qemu_logfile_initialized) {
> > +qemu_logfile_init();
> > +}
> > +
> > pidstr = strstr(filename, "%");
> > if (pidstr) {
> > /* We only accept one %d, no other format strings */
>
>
> --
> Alex Bennée
Re: [PATCH 4/4] Added tests for close and change of logfile.
Agree with all the suggestions below. These are great ideas. Will
make the changes.
Thanks,
-Rob Foley
On Thu, 7 Nov 2019 at 11:32, Alex Bennée wrote:
>
>
> Robert Foley writes:
>
> > One test ensures that the logfile handle is still valid even if
> > the logfile is changed during logging.
> > The other test validates that the logfile handle remains valid under
> > the logfile lock even if the logfile is closed.
> >
> > Signed-off-by: Robert Foley
> > ---
> > tests/test-logging.c | 74
> > 1 file changed, 74 insertions(+)
> >
> > diff --git a/tests/test-logging.c b/tests/test-logging.c
> > index a12585f70a..a3190ff92c 100644
> > --- a/tests/test-logging.c
> > +++ b/tests/test-logging.c
> > @@ -108,6 +108,76 @@ static void test_parse_path(gconstpointer data)
> > error_free_or_abort();
> > }
> >
> > +static void test_logfile_write(gconstpointer data)
> > +{
> > +QemuLogFile *logfile;
> > +gchar const *dir = data;
> > +Error *err = NULL;
> > +gchar *file_path;
> > +gchar *file_path1;
>
> with g_autofree char *file_path you can avoid the free down bellow.
>
> > +FILE *orig_fd;
> > +
> > +file_path = g_build_filename(dir, "qemu_test_log_write0.log", NULL);
> > +file_path1 = g_build_filename(dir, "qemu_test_log_write1.log", NULL);
> > +
> > +/*
> > + * Test that even if an open file handle is changed,
> > + * our handle remains valid due to RCU.
> > + */
> > +qemu_set_log_filename(file_path, );
> > +g_assert(!err);
> > +rcu_read_lock();
> > +logfile = atomic_rcu_read(_logfile);
> > +orig_fd = logfile->fd;
> > +g_assert(logfile && logfile->fd);
> > +fprintf(logfile->fd, "%s 1st write to file\n", __func__);
> > +fflush(logfile->fd);
> > +
> > +/* Change the logfile and ensure that the handle is still valid. */
> > +qemu_set_log_filename(file_path1, );
> > +g_assert(!err);
>
> Maybe better would be:
>
> logfile2 = atomic_rcu_read(_logfile);
> g_assert(logfile->fd == orig_fd);
> g_assert(logfile2->fd != logfile->fd);
> fprintf(logfile2->fd, "%s 2nd write to file\n", __func__);
> fflush(logfile2->fd);
>
>
> > +g_assert(logfile->fd == orig_fd);
> > +fprintf(logfile->fd, "%s 2nd write to file\n", __func__);
> > +fflush(logfile->fd);
> > +rcu_read_unlock();
> > +
> > +g_free(file_path);
> > +g_free(file_path1);
> > +}
> > +
> > +static void test_logfile_lock(gconstpointer data)
> > +{
> > +FILE *logfile;
> > +gchar const *dir = data;
> > +Error *err = NULL;
> > +gchar *file_path;
>
> g_autofree
>
> > +
> > +file_path = g_build_filename(dir, "qemu_test_logfile_lock0.log", NULL);
> > +
> > +/*
> > + * Test the use of the logfile lock, such
> > + * that even if an open file handle is closed,
> > + * our handle remains valid for use due to RCU.
> > + */
> > +qemu_set_log_filename(file_path, );
> > +logfile = qemu_log_lock();
> > +g_assert(logfile);
> > +fprintf(logfile, "%s 1st write to file\n", __func__);
> > +fflush(logfile);
> > +
> > +/*
> > + * Initiate a close file and make sure our handle remains
> > + * valid since we still have the logfile lock.
> > + */
> > +qemu_log_close();
> > +fprintf(logfile, "%s 2nd write to file\n", __func__);
> > +fflush(logfile);
> > +qemu_log_unlock(logfile);
> > +
> > +g_assert(!err);
> > +g_free(file_path);
> > +}
> > +
> > /* Remove a directory and all its entries (non-recursive). */
> > static void rmdir_full(gchar const *root)
> > {
> > @@ -134,6 +204,10 @@ int main(int argc, char **argv)
> >
> > g_test_add_func("/logging/parse_range", test_parse_range);
> > g_test_add_data_func("/logging/parse_path", tmp_path, test_parse_path);
> > +g_test_add_data_func("/logging/logfile_write_path",
> > + tmp_path, test_logfile_write);
> > +g_test_add_data_func("/logging/logfile_lock_path",
> > + tmp_path, test_logfile_lock);
> >
> > rc = g_test_run();
>
>
> --
> Alex Bennée
Re: [PATCH 3/4] qemu_log_lock/unlock now preserves the qemu_logfile handle.
On Thu, 7 Nov 2019 at 11:25, Alex Bennée wrote:
>
> Ahh there it is!
>
> We probably want to put the API change through before we add the RCU
> support - so swap the patch order around.
>
Absolutely, we will swap around the order of the patch.
-Rob Foley
On Thu, 7 Nov 2019 at 11:25, Alex Bennée wrote:
>
>
> Robert Foley writes:
>
> > qemu_log_lock() now returns a handle and qemu_log_unlock() receives a
> > handle to unlock. This allows for changing the handle during logging
> > and ensures the lock() and unlock() are for the same file.
>
> Ahh there it is!
>
> We probably want to put the API change through before we add the RCU
> support - so swap the patch order around.
>
> >
> > Signed-off-by: Robert Foley
> > ---
> > include/qemu/log.h| 14 +++---
> > accel/tcg/cpu-exec.c | 4 ++--
> > accel/tcg/translate-all.c | 4 ++--
> > accel/tcg/translator.c| 4 ++--
> > exec.c| 4 ++--
> > hw/net/can/can_sja1000.c | 4 ++--
> > net/can/can_socketcan.c | 5 ++---
> > target/cris/translate.c | 4 ++--
> > target/i386/translate.c | 5 +++--
> > target/lm32/translate.c | 4 ++--
> > target/microblaze/translate.c | 4 ++--
> > target/nios2/translate.c | 4 ++--
> > target/tilegx/translate.c | 7 ---
> > target/unicore32/translate.c | 4 ++--
> > tcg/tcg.c | 16
> > 15 files changed, 44 insertions(+), 43 deletions(-)
> >
> > diff --git a/include/qemu/log.h b/include/qemu/log.h
> > index 975de18e23..3d0f47a479 100644
> > --- a/include/qemu/log.h
> > +++ b/include/qemu/log.h
> > @@ -71,25 +71,25 @@ static inline bool qemu_log_separate(void)
> > * qemu_loglevel is never set when qemu_logfile is unset.
> > */
> >
> > -static inline void qemu_log_lock(void)
> > +static inline FILE *qemu_log_lock(void)
> > {
> > QemuLogFile *logfile;
> > rcu_read_lock();
> > logfile = atomic_rcu_read(_logfile);
> > if (logfile) {
> > qemu_flockfile(logfile->fd);
> > +return logfile->fd;
> > }
> > rcu_read_unlock();
> > +return NULL;
> > }
> >
> > -static inline void qemu_log_unlock(void)
> > +static inline void qemu_log_unlock(FILE *fd)
> > {
> > -QemuLogFile *logfile;
> > -logfile = atomic_rcu_read(_logfile);
> > -if (logfile) {
> > -qemu_funlockfile(logfile->fd);
> > +if (fd) {
> > +qemu_funlockfile(fd);
> > +rcu_read_unlock();
> > }
> > -rcu_read_unlock();
> > }
> >
> > /* Logging functions: */
> > diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> > index c01f59c743..62068d10c3 100644
> > --- a/accel/tcg/cpu-exec.c
> > +++ b/accel/tcg/cpu-exec.c
> > @@ -156,7 +156,7 @@ static inline tcg_target_ulong cpu_tb_exec(CPUState
> > *cpu, TranslationBlock *itb)
> > #if defined(DEBUG_DISAS)
> > if (qemu_loglevel_mask(CPU_LOG_TB_CPU)
> > && qemu_log_in_addr_range(itb->pc)) {
> > -qemu_log_lock();
> > +FILE *logfile = qemu_log_lock();
> > int flags = 0;
> > if (qemu_loglevel_mask(CPU_LOG_TB_FPU)) {
> > flags |= CPU_DUMP_FPU;
> > @@ -165,7 +165,7 @@ static inline tcg_target_ulong cpu_tb_exec(CPUState
> > *cpu, TranslationBlock *itb)
> > flags |= CPU_DUMP_CCOP;
> > #endif
> > log_cpu_state(cpu, flags);
> > -qemu_log_unlock();
> > +qemu_log_unlock(logfile);
> > }
> > #endif /* DEBUG_DISAS */
> >
> > diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
> > index 9f48da9472..bb325a2bc4 100644
> > --- a/accel/tcg/translate-all.c
> > +++ b/accel/tcg/translate-all.c
> > @@ -1804,7 +1804,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
> > #ifdef DEBUG_DISAS
> > if (qemu_loglevel_mask(CPU_LOG_TB_OUT_ASM) &&
> > qemu_log_in_addr_range(tb->pc)) {
> > -qemu_log_lock();
> > +FILE *logfile = qemu_log_lock();
> > qemu_log("OUT: [size=%d]\n", gen_code_size);
> > if (tcg_ctx->data_gen_ptr) {
> > size_t code_size = tcg_ctx->data_gen_ptr - tb->tc.ptr;
> > @@ -1829,7 +1829,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
> > }
> > qemu_log("\n");
> > qemu_log_flush();
> > -qemu_log_unlock();
> > +qemu_log_unlock(logfile);
> > }
> > #endif
> >
> > diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
> > index f977682be7..603d17ff83 100644
> > --- a/accel/tcg/translator.c
> > +++ b/accel/tcg/translator.c
> > @@ -138,11 +138,11 @@ void translator_loop(const TranslatorOps *ops,
> > DisasContextBase *db,
> > #ifdef DEBUG_DISAS
> > if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)
> > && qemu_log_in_addr_range(db->pc_first)) {
> > -qemu_log_lock();
> > +FILE *logfile = qemu_log_lock();
> > qemu_log("\n");
> > ops->disas_log(db, cpu);
> > qemu_log("\n");
> > -qemu_log_unlock();
> > +
Re: [PATCH 2/4] Add use of RCU for qemu_logfile.
On Thu, 7 Nov 2019 at 11:24, Alex Bennée wrote:
>
> Robert Foley writes:
> > diff --git a/include/qemu/log.h b/include/qemu/log.h
> > index a91105b2ad..975de18e23 100644
> > --- a/include/qemu/log.h
> > +++ b/include/qemu/log.h
> > @@ -3,9 +3,17 @@
> >
> > /* A small part of this API is split into its own header */
> > #include "qemu/log-for-trace.h"
> > +#include "qemu/rcu.h"
> > +
> > +struct QemuLogFile {
> > +struct rcu_head rcu;
> > +FILE *fd;
> > +};
> > +typedef struct QemuLogFile QemuLogFile;
>
> If you are declaring a typedef then do it in one:
>
> typedef struct QemuLogFile { ...
>
> We only really use the second form for opaque structs where the handle
> is passed around but the contents hidden from the rest of QEMU.
>
OK, makes sense. Will correct it. Thanks for explaining.
> >
> > /* Private global variable, don't use */
> > -extern FILE *qemu_logfile;
> > +extern QemuLogFile *qemu_logfile;
> > +
>
> Do we need multiple
Not 100% sure on the meaning here. Are you asking if we need to
extern this? We have a few other cases outside of the log module
where this is getting used so the extern is needed here.
> >
> > /*
> > * The new API:
> > @@ -25,7 +33,17 @@ static inline bool qemu_log_enabled(void)
> > */
> > static inline bool qemu_log_separate(void)
> > {
> > -return qemu_logfile != NULL && qemu_logfile != stderr;
> > +QemuLogFile *logfile;
> > +
> > +if (qemu_log_enabled()) {
> > +rcu_read_lock();
> > +logfile = atomic_rcu_read(_logfile);
> > +if (logfile && logfile->fd != stderr) {
> > +return true;
> > +}
> > +rcu_read_unlock();
> > +}
> > +return false;
>
> This is unbalanced as you'll have incremented the reader count. Also
> qemu_log_enabled() is also synonymous with logfile existing so you could
> fold that into:
>
> bool res = false;
>
> rcu_read_lock();
> *logfile = atomic_rcu_read(_logfile);
> if (logfile && logfile->fd != stderr) {
> res = true;
> }
> rcu_read_unlock();
> return res;
>
> There is technically a race there as the answer may become invalid after
> qemu_log_separate() returns. However given the users I don't see what
> could fail afterwards.
>
I agree, will make these changes.
> > }
> >
> > #define CPU_LOG_TB_OUT_ASM (1 << 0)
> > @@ -55,12 +73,23 @@ static inline bool qemu_log_separate(void)
> >
> > static inline void qemu_log_lock(void)
> > {
> > -qemu_flockfile(qemu_logfile);
> > +QemuLogFile *logfile;
> > +rcu_read_lock();
> > +logfile = atomic_rcu_read(_logfile);
> > +if (logfile) {
> > +qemu_flockfile(logfile->fd);
> > +}
> > +rcu_read_unlock();
> > }
>
> static inline FILE *fd qemu_log_lock(void)
> {
QemuLogFile *logfile;
> rcu_read_lock();
logfile = atomic_rcu_read(_logfile);
if (logfile) {
> qemu_flockfile(logfile->fd);
return logfile->fd;
> } else {
> rcu_read_unlock();
> return NULL;
> }
> }
>
> static inline qemu_log_unlock(FILE *fd)
> {
> if (fd) {
> qemu_funlockfile(fd);
> rcu_read_unlock();
> }
> }
>
> While the rcu_read_lock() is in progress then we won't ever finish with
> the fd - which we don't want to do until the file locking is finished.
Agree with the adjustments you made to qemu_log_lock(). I updated the
code above with a few tweaks for the QemuLogFile type returned by
atomic_rcu_read(). Does this look OK or is there any other adjustment
we should make here?
The intent here was for qemu_log_lock() to hold the rcu_read_lock()
until after we can qemu_funlockfile(fd). The idea being that we want
to prevent the fclose(fd) from happening by holding the
rcu_read_lock() across the qemu_log_lock() until qemu_log_unlock().
So we have the qemu_funlockfile(fd) first, and then once we're done
with the fd, it is safe to rcu_read_unlock().
>
> > diff --git a/util/log.c b/util/log.c
> > @@ -65,6 +70,8 @@ static bool log_uses_own_buffers;
> > /* enable or disable low levels log */
> > void qemu_set_log(int log_flags)
> > {
> > +QemuLogFile *logfile;
> > +
> > qemu_loglevel = log_flags;
> > #ifdef CONFIG_TRACE_LOG
> > qemu_loglevel |= LOG_TRACE;
> > @@ -77,43 +84,51 @@ void qemu_set_log(int log_flags)
> > qemu_mutex_lock(_logfile_mutex);
> > if (!qemu_logfile &&
> > (is_daemonized() ? logfilename != NULL : qemu_loglevel)) {
> > +logfile = g_new0(QemuLogFile, 1);
> > if (logfilename) {
>
> You can assume logfilename exists as glib memory allocations would
> abort() otherwise.
This is good to know about the glib memory allocations as it relates
to the logfile above, since we did not add any check for null on
allocation.
We are thinking that logfilename might be NULL if we either never set
it with a call to qemu_set_log_filename(), or (with our intended new
fix) if we took an error actually opening the file in
qemu_set_log_filename(), and went down
Re: [PATCH v3 00/22] iotests: Allow ./check -o data_file
Patchew URL: https://patchew.org/QEMU/20191107163708.833192-1-mre...@redhat.com/ Hi, This series seems to have some coding style problems. See output below for more information: Subject: [PATCH v3 00/22] iotests: Allow ./check -o data_file Type: series Message-id: 20191107163708.833192-1-mre...@redhat.com === TEST SCRIPT BEGIN === #!/bin/bash git rev-parse base > /dev/null || exit 0 git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram ./scripts/checkpatch.pl --mailback base.. === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 Switched to a new branch 'test' 01a2839 iotests: Allow check -o data_file 31bc07b iotests: Disable data_file where it cannot be used 98a2575 iotests: Make 198 work with data_file e8f406f iotests: Make 137 work with data_file 46cc09d iotests: Make 110 work with data_file 1f7b2e5 iotests: Make 091 work with data_file 401d3ef iotests: Avoid cp/mv of test images a3746a2 iotests: Use _rm_test_img for deleting test images 37a01c8 iotests: Avoid qemu-img create a05c5ec iotests: Drop IMGOPTS use in 267 44aac70 iotests: Replace IMGOPTS='' by --no-opts cb9ee70 iotests: Replace IMGOPTS= by -o 3c2893f iotests: Inject space into -ocompat=0.10 in 051 8b5f9d4 iotests: Add -o and --no-opts to _make_test_img 239f933 iotests: Let _make_test_img parse its parameters 405ddde iotests: Drop compat=1.1 in 050 527ae22 iotests: Replace IMGOPTS by _unsupported_imgopts 77f688d iotests: Filter refcount_order in 036 3f29d5f iotests: Add _filter_json_filename 58975a8 iotests/qcow2.py: Split feature fields into bits 7ea641e iotests/qcow2.py: Add dump-header-exts 469af5e iotests: s/qocw2/qcow2/ === OUTPUT BEGIN === 1/22 Checking commit 469af5ede216 (iotests: s/qocw2/qcow2/) 2/22 Checking commit 7ea641ec6b0a (iotests/qcow2.py: Add dump-header-exts) ERROR: line over 90 characters #33: FILE: tests/qemu-iotests/qcow2.py:237: +[ 'dump-header-exts', cmd_dump_header_exts, 0, 'Dump image header extensions' ], total: 1 errors, 0 warnings, 17 lines checked Patch 2/22 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 3/22 Checking commit 58975a850885 (iotests/qcow2.py: Split feature fields into bits) 4/22 Checking commit 3f29d5f2c82a (iotests: Add _filter_json_filename) 5/22 Checking commit 77f688d94ac8 (iotests: Filter refcount_order in 036) 6/22 Checking commit 527ae221d7bc (iotests: Replace IMGOPTS by _unsupported_imgopts) 7/22 Checking commit 405dddedf22d (iotests: Drop compat=1.1 in 050) 8/22 Checking commit 239f933e104c (iotests: Let _make_test_img parse its parameters) 9/22 Checking commit 8b5f9d4a9fff (iotests: Add -o and --no-opts to _make_test_img) 10/22 Checking commit 3c2893f30375 (iotests: Inject space into -ocompat=0.10 in 051) 11/22 Checking commit cb9ee70ce491 (iotests: Replace IMGOPTS= by -o) 12/22 Checking commit 44aac701db74 (iotests: Replace IMGOPTS='' by --no-opts) 13/22 Checking commit a05c5ec14fb2 (iotests: Drop IMGOPTS use in 267) 14/22 Checking commit 37a01c83e4e6 (iotests: Avoid qemu-img create) 15/22 Checking commit a3746a2198bc (iotests: Use _rm_test_img for deleting test images) 16/22 Checking commit 401d3ef85556 (iotests: Avoid cp/mv of test images) 17/22 Checking commit 1f7b2e52555b (iotests: Make 091 work with data_file) 18/22 Checking commit 46cc09d0608f (iotests: Make 110 work with data_file) 19/22 Checking commit e8f406f2bda8 (iotests: Make 137 work with data_file) 20/22 Checking commit 98a25755fe17 (iotests: Make 198 work with data_file) 21/22 Checking commit 31bc07b55b8a (iotests: Disable data_file where it cannot be used) 22/22 Checking commit 01a283955091 (iotests: Allow check -o data_file) === OUTPUT END === Test command exited with code: 1 The full log is available at http://patchew.org/logs/20191107163708.833192-1-mre...@redhat.com/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [PATCH 2/3] qtest: fix qtest_qmp_device_add leak
Hi On Fri, Nov 8, 2019 at 12:31 AM Laurent Vivier wrote: > > On 07/11/2019 20:27, Marc-André Lureau wrote: > > Spotted by ASAN. > > > > Signed-off-by: Marc-André Lureau > > --- > > tests/libqtest.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/tests/libqtest.c b/tests/libqtest.c > > index 3706bccd8d..91e9cb220c 100644 > > --- a/tests/libqtest.c > > +++ b/tests/libqtest.c > > @@ -1274,6 +1274,7 @@ void qtest_qmp_device_add(QTestState *qts, const char > > *driver, const char *id, > > qdict_put_str(args, "id", id); > > > > qtest_qmp_device_add_qdict(qts, driver, args); > > +qobject_unref(args); > > } > > > > static void device_deleted_cb(void *opaque, const char *name, QDict *data) > > > > Stupid question: where is the qobject_ref()? The initial ref is from qobject_from_vjsonf_nofail() constructor -- Marc-André Lureau
Re: [RFC PATCH 02/18] qemu-storage-daemon: Add --object option
Kevin Wolf writes:
> Add a command line option to create user-creatable QOM objects.
>
> Signed-off-by: Kevin Wolf
> ---
> qemu-storage-daemon.c | 35 +++
> 1 file changed, 35 insertions(+)
>
> diff --git a/qemu-storage-daemon.c b/qemu-storage-daemon.c
> index a251dc255c..48d6af43a6 100644
> --- a/qemu-storage-daemon.c
> +++ b/qemu-storage-daemon.c
> @@ -35,6 +35,8 @@
> #include "qemu/log.h"
> #include "qemu/main-loop.h"
> #include "qemu/module.h"
> +#include "qemu/option.h"
> +#include "qom/object_interfaces.h"
>
> #include "trace/control.h"
>
> @@ -51,10 +53,26 @@ static void help(void)
> " specify tracing options\n"
> " -V, --version output version information and exit\n"
> "\n"
> +" --object define a QOM object such as 'secret' for\n"
> +" passwords and/or encryption keys\n"
This is less helpful than qemu-system-FOO's help:
-object TYPENAME[,PROP1=VALUE1,...]
create a new object of type TYPENAME setting properties
in the order they are specified. Note that the 'id'
property must be set. These objects are placed in the
'/objects' path.
> +"\n"
> QEMU_HELP_BOTTOM "\n",
> error_get_progname());
> }
>
> +enum {
> +OPTION_OBJECT = 256,
> +};
> +
> +static QemuOptsList qemu_object_opts = {
> +.name = "object",
> +.implied_opt_name = "qom-type",
> +.head = QTAILQ_HEAD_INITIALIZER(qemu_object_opts.head),
> +.desc = {
> +{ }
> +},
> +};
> +
Note for later: copied from vl.c.
> static int process_options(int argc, char *argv[], Error **errp)
> {
> int c;
> @@ -63,6 +81,7 @@ static int process_options(int argc, char *argv[], Error
> **errp)
>
> static const struct option long_options[] = {
> {"help", no_argument, 0, 'h'},
> +{"object", required_argument, 0, OPTION_OBJECT},
> {"version", no_argument, 0, 'V'},
> {"trace", required_argument, NULL, 'T'},
> {0, 0, 0, 0}
> @@ -88,6 +107,22 @@ static int process_options(int argc, char *argv[], Error
> **errp)
> g_free(trace_file);
> trace_file = trace_opt_parse(optarg);
> break;
> +case OPTION_OBJECT:
> +{
> +QemuOpts *opts;
> +const char *type;
> +
> +opts = qemu_opts_parse(_object_opts,
> + optarg, true, _fatal);
> +type = qemu_opt_get(opts, "qom-type");
> +
> +if (user_creatable_print_help(type, opts)) {
> +exit(EXIT_SUCCESS);
> +}
> +user_creatable_add_opts(opts, _fatal);
> +qemu_opts_del(opts);
> +break;
> +}
> }
> }
> if (optind != argc) {
PATCH 01 duplicates case QEMU_OPTION_trace pretty much verbatim. Makes
sense, as qemu-storage-daemon is basically qemu-system-FOO with "FOO"
and most "system" cut away.
This patch adds vl.c's case QEMU_OPTION_object in a much simpler form.
This is one of my least favourite options, and I'll tell you why below.
Let's compare the two versions.
vl.c:
case QEMU_OPTION_object:
opts = qemu_opts_parse_noisily(qemu_find_opts("object"),
optarg, true);
if (!opts) {
exit(1);
}
break;
Further down:
qemu_opts_foreach(qemu_find_opts("object"),
user_creatable_add_opts_foreach,
object_create_initial, _fatal);
Still further down:
qemu_opts_foreach(qemu_find_opts("object"),
user_creatable_add_opts_foreach,
object_create_delayed, _fatal);
These are basically
for opts in qemu_object_opts {
type = qemu_opt_get(opts, "qom-type");
if (type) {
if (user_creatable_print_help(type, opts)) {
exit(0);
}
if (!predicate(type)) {
continue;
}
}
obj = user_creatable_add_opts(opts, _fatal);
object_unref(obj);
}
where predicate(type) is true in exactly one of the two places for each
QOM type.
The reason for these gymnastics is to create objects at the right time
during startup, except there is no right time, but two.
Differences:
* Options are processed left to right without gymnastics. Getting their
order right is the user's problem. I consider this an improvement.
* You use _object_opts instead of qemu_find_opts("object"). Also
an improvement.
* You use qemu_opts_parse() instead of qemu_opts_parse_noisily().
The latter can print help. I failed to find a case where we lose help
compared to qemu-system-FOO. I didn't try very hard.
* You neglect to guard user_creatable_print_help():
$ qemu-storage-daemon
Re: [PATCH v7 7/8] Acceptance tests: depend on qemu-img
On 11/4/19 1:13 PM, Cleber Rosa wrote: Tests using the avocado.utils.vmimage library make use of qemu-img, and because it makes sense to use the version matching the rest of the source code, let's make sure it gets built. Its selection, instead of a possible qemu-img binary installed system wide, is already dealt with by the change that adds the build dir to the PATH during the test execution. This is based on the same work for qemu-iotests, and suggested by its author: - https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg00951.html CC: Philippe Mathieu-Daudé Signed-off-by: Cleber Rosa Reviewed-by: Philippe Mathieu-Daudé --- tests/Makefile.include | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Makefile.include b/tests/Makefile.include index 65e85f5275..559c3e6375 100644 --- a/tests/Makefile.include +++ b/tests/Makefile.include @@ -1174,7 +1174,7 @@ $(TESTS_RESULTS_DIR): check-venv: $(TESTS_VENV_DIR) -check-acceptance: check-venv $(TESTS_RESULTS_DIR) +check-acceptance: check-venv $(TESTS_RESULTS_DIR) qemu-img$(EXESUF) To be honest, I don't fell comfortable by the fact that the whole acceptance suite will depend on qemu-img which, in reality, is needed by only a sub-set of tests. Besides it, there might be some reason for someone to build QEMU with --disable-tools and this change will end up forcing the qemu-img built (of course if check-acceptance is issued). What if we instead: 1. Warn the users in case qemu tools weren't built. Alerting that qemu-img and friends will be picked up from system-wide (if any). 2. The tests that rely on avocado.utils.vmimage check for the presence of dependent tools, possible canceling itself on their lack. This may be done at test code level or perhaps using Avocado's tag mechanism + tweaking avocado_qemu. Thanks, Wainer $(call quiet-command, \ $(TESTS_VENV_DIR)/bin/python -m avocado \ --show=$(AVOCADO_SHOW) run --job-results-dir=$(TESTS_RESULTS_DIR) \
Re: [PATCH 2/3] qtest: fix qtest_qmp_device_add leak
On 07/11/2019 20:27, Marc-André Lureau wrote: > Spotted by ASAN. > > Signed-off-by: Marc-André Lureau > --- > tests/libqtest.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/tests/libqtest.c b/tests/libqtest.c > index 3706bccd8d..91e9cb220c 100644 > --- a/tests/libqtest.c > +++ b/tests/libqtest.c > @@ -1274,6 +1274,7 @@ void qtest_qmp_device_add(QTestState *qts, const char > *driver, const char *id, > qdict_put_str(args, "id", id); > > qtest_qmp_device_add_qdict(qts, driver, args); > +qobject_unref(args); > } > > static void device_deleted_cb(void *opaque, const char *name, QDict *data) > Stupid question: where is the qobject_ref()? Thanks, Laurent
Re: [PATCH 4/4] Added tests for close and change of logfile.
Robert Foley writes:
> Thanks for providing the stack trace.
>
> We debugged this and it seems to come about because of an interesting
> circumstance. We added our new tests after a pre-existing test,
> parse_path(), which runs into an issue, a dangling pointer, which
> could lead to a double free. There were no other tests after the test
> that ran into the issue, so the double free was not exposed until we
> added our test which called qemu_set_log_filename().
>
> Upon entry to qemu_set_log_filename() it frees logfilename. In the
> case where we get an error, we return out without setting the
> logfilename to NULL.
> And on next call into this function we will end up with a double free.
>
> For a fix, we could put this at the beginning of qemu_set_log_filename().
> if (logfilename) {
> g_free(logfilename);
> logfilename = NULL;
> }
g_free(logfilename) should be safe against NULL. However we need to
ensure that logfilename is NULL'ed after it so we don't see the double
free.
> We were curious to understand why we did not see it in our own
> testing. Although we did run make check before our first post, we did
> not see this issue. The docker tests seem to use something like
> MALLOC_CHECK_, which catches memory issues like this. We will be
> sure to run the docker tests as well in the future.
I was just running in my normal checkout - it could depend on how glibc
was built for your system though. Mine is Debian Buster.
>
> On Thu, 7 Nov 2019 at 12:26, Alex Bennée wrote:
>>
>>
>> Alex Bennée writes:
>>
>> > Robert Foley writes:
>> >
>> >> One test ensures that the logfile handle is still valid even if
>> >> the logfile is changed during logging.
>> >> The other test validates that the logfile handle remains valid under
>> >> the logfile lock even if the logfile is closed.
>>
>> Also this doesn't see to work:
>>
>> 17:24:31 [alex@zen:~/l/q/b/all] review/rcu-logfile|… 2 + ./tests/test-logging
>> /logging/parse_range: OK
>> /logging/parse_path: OK
>> /logging/logfile_write_path: free(): double free detected in tcache 2
>> fish: “./tests/test-logging” terminated by signal SIGABRT (Abort)
>>
>> in gdb
>>
>> Starting program: /home/alex/lsrc/qemu.git/builds/all/tests/test-logging
>> [Thread debugging using libthread_db enabled]
>> Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
>> [New Thread 0x76f38700 (LWP 28960)]
>> /logging/parse_range: OK
>> /logging/parse_path: OK
>> /logging/logfile_write_path: free(): double free detected in tcache 2
>>
>> Thread 1 "test-logging" received signal SIGABRT, Aborted.
>> __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
>> 50 ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
>> (gdb) bt
>> #0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
>> #1 0x77587535 in __GI_abort () at abort.c:79
>> #2 0x775de508 in __libc_message (action=action@entry=do_abort,
>> fmt=fmt@entry=0x776e928d "%s\n") at ../sysdeps/posix/libc_fatal.c:181
>> #3 0x775e4c1a in malloc_printerr (str=str@entry=0x776eaf58
>> "free(): double free detected in tcache 2") at malloc.c:5341
>> #4 0x775e66fd in _int_free (av=0x77720c40 ,
>> p=0x555cac40, have_lock=) at malloc.c:4193
>> #5 0x555614a8 in qemu_set_log_filename (filename=0x555cb110
>> "/tmp/qemu-test-logging.RO35A0/qemu_test_log_write0.log",
>> errp=0x7fffdef0) at /home/alex/lsrc/qemu.git/util/log.c:148
>> #6 0xd8be in test_logfile_write (data=0x555c7370) at
>> /home/alex/lsrc/qemu.git/tests/test-logging.c:127
>> #7 0x77cdc15a in test_case_run (tc=0x555c9c60) at
>> ../../../glib/gtestutils.c:2318
>> #8 g_test_run_suite_internal (suite=suite@entry=0x555c8a40,
>> path=path@entry=0x0) at ../../../glib/gtestutils.c:2403
>> #9 0x77cdc014 in g_test_run_suite_internal
>> (suite=suite@entry=0x555c8a20, path=path@entry=0x0) at
>> ../../../glib/gtestutils.c:2415
>> #10 0x77cdc412 in g_test_run_suite (suite=0x555c8a20) at
>> ../../../glib/gtestutils.c:2490
>> #11 0x77cdc431 in g_test_run () at ../../../glib/gtestutils.c:1755
>> #12 0xce07 in main (argc=, argv=)
>> at /home/alex/lsrc/qemu.git/tests/test-logging.c:212
>>
>>
>> >>
>> >> Signed-off-by: Robert Foley
>> >> ---
>> >> tests/test-logging.c | 74
>> >> 1 file changed, 74 insertions(+)
>> >>
>> >> diff --git a/tests/test-logging.c b/tests/test-logging.c
>> >> index a12585f70a..a3190ff92c 100644
>> >> --- a/tests/test-logging.c
>> >> +++ b/tests/test-logging.c
>> >> @@ -108,6 +108,76 @@ static void test_parse_path(gconstpointer data)
>> >> error_free_or_abort();
>> >> }
>> >>
>> >> +static void test_logfile_write(gconstpointer data)
>> >> +{
>> >> +QemuLogFile *logfile;
>> >> +gchar const *dir = data;
>> >> +Error *err = NULL;
>> >> +gchar *file_path;
>> >> +gchar
Re: [PATCH v2] WHPX: support for xcr0
Am 07.11.19 um 20:48 schrieb Sunil Muthuswamy: > Support for xcr0 to be able to enable xsave/xrstor. This by itself > is not sufficient to enable xsave/xrstor. WHPX XSAVE API's also > needs to be hooked up. > > Signed-off-by: Sunil Muthuswamy > --- > You will need the Windows 10 SDK for RS5 (build 17763) or above to > to be able to compile this patch because of the definition of the > XCR0 register. > > Changes since v1: > - Added a sign-off line in the patch. I am not very happy with the current situation which suggests using non free header files from the Microsoft Windows SDK, thus making it impossible to produce QEMU executables for Windows with WHPX support without having legal complications. Could you please add the required headers with a suitable license to the QEMU source code? That would clarify the license issue and make builds with WHPX much easier because those files would not have to be extracted from a very large SDK installation. It would also be acceptable if Microsoft could update the license comments in those files and use a QEMU compatible license. Kind regards Stefan Weil
Re: Should QEMU's configure script check for bzip2 ?
On 11/7/19 5:43 PM, Thomas Huth wrote: Hi, I just tried to compile QEMU on a freshly installed system. "configure" finished without problems, but during "make" I hit this error: BUNZIP2 pc-bios/edk2-i386-secure-code.fd.bz2 /bin/sh: bzip2: command not found make: *** [Makefile:305: pc-bios/edk2-i386-secure-code.fd] Error 127 make: *** Deleting file 'pc-bios/edk2-i386-secure-code.fd' make: *** Waiting for unfinished jobs This error happened with me a few times as well. And I always wondered why bzip2 isn't checked on configure. Sure, it's easy to fix, but maybe "configure" should already check for the availablity of "bzip2", so that we then either skip the installation of the edk2 images if "bzip2" is not available, or bail out during "configure" already? IMHO, it should be checked since bzip2 is a build dependency. Thanks, Wainer Thomas
[PATCH v2] WHPX: support for xcr0
Support for xcr0 to be able to enable xsave/xrstor. This by itself
is not sufficient to enable xsave/xrstor. WHPX XSAVE API's also
needs to be hooked up.
Signed-off-by: Sunil Muthuswamy
---
You will need the Windows 10 SDK for RS5 (build 17763) or above to
to be able to compile this patch because of the definition of the
XCR0 register.
Changes since v1:
- Added a sign-off line in the patch.
target/i386/whp-dispatch.h | 3 ++
target/i386/whpx-all.c | 88 ++
2 files changed, 91 insertions(+)
diff --git a/target/i386/whp-dispatch.h b/target/i386/whp-dispatch.h
index 23791fbb47..b5d56b22a3 100644
--- a/target/i386/whp-dispatch.h
+++ b/target/i386/whp-dispatch.h
@@ -6,6 +6,9 @@
#include
#include
+/* This should eventually come from the Windows SDK */
+#define WHV_E_UNKNOWN_PROPERTY 0x80370302
+
#define LIST_WINHVPLATFORM_FUNCTIONS(X) \
X(HRESULT, WHvGetCapability, (WHV_CAPABILITY_CODE CapabilityCode, VOID*
CapabilityBuffer, UINT32 CapabilityBufferSizeInBytes, UINT32*
WrittenSizeInBytes)) \
X(HRESULT, WHvCreatePartition, (WHV_PARTITION_HANDLE* Partition)) \
diff --git a/target/i386/whpx-all.c b/target/i386/whpx-all.c
index ed95105eae..1abaac70db 100644
--- a/target/i386/whpx-all.c
+++ b/target/i386/whpx-all.c
@@ -161,10 +161,15 @@ struct whpx_vcpu {
static bool whpx_allowed;
static bool whp_dispatch_initialized;
static HMODULE hWinHvPlatform, hWinHvEmulation;
+static WHV_PROCESSOR_XSAVE_FEATURES whpx_xsave_cap;
struct whpx_state whpx_global;
struct WHPDispatch whp_dispatch;
+static bool whpx_has_xsave(void)
+{
+return whpx_xsave_cap.XsaveSupport;
+}
/*
* VP support
@@ -216,6 +221,28 @@ static SegmentCache whpx_seg_h2q(const
WHV_X64_SEGMENT_REGISTER *hs)
return qs;
}
+/* X64 Extended Control Registers */
+static void whpx_set_xcrs(CPUState *cpu)
+{
+struct CPUX86State *env = (CPUArchState *)(cpu->env_ptr);
+HRESULT hr;
+struct whpx_state *whpx = _global;
+WHV_REGISTER_VALUE xcr0;
+WHV_REGISTER_NAME xcr0_name = WHvX64RegisterXCr0;
+
+if (!whpx_has_xsave()) {
+return;
+}
+
+/* Only xcr0 is supported by the hypervisor currently */
+xcr0.Reg64 = env->xcr0;
+hr = whp_dispatch.WHvSetVirtualProcessorRegisters(
+whpx->partition, cpu->cpu_index, _name, 1, );
+if (FAILED(hr)) {
+error_report("WHPX: Failed to set register xcr0, hr=%08lx", hr);
+}
+}
+
static void whpx_set_registers(CPUState *cpu)
{
struct whpx_state *whpx = _global;
@@ -291,6 +318,12 @@ static void whpx_set_registers(CPUState *cpu)
/* 8 Debug Registers - Skipped */
+/*
+ * Extended control registers needs to be handled separately depending
+ * on whether xsave is supported/enabled or not.
+ */
+whpx_set_xcrs(cpu);
+
/* 16 XMM registers */
assert(whpx_register_names[idx] == WHvX64RegisterXmm0);
idx_next = idx + 16;
@@ -380,6 +413,30 @@ static void whpx_set_registers(CPUState *cpu)
return;
}
+/* X64 Extended Control Registers */
+static void whpx_get_xcrs(CPUState *cpu)
+{
+struct CPUX86State *env = (CPUArchState *)(cpu->env_ptr);
+HRESULT hr;
+struct whpx_state *whpx = _global;
+WHV_REGISTER_VALUE xcr0;
+WHV_REGISTER_NAME xcr0_name = WHvX64RegisterXCr0;
+
+if (!whpx_has_xsave()) {
+return;
+}
+
+/* Only xcr0 is supported by the hypervisor currently */
+hr = whp_dispatch.WHvGetVirtualProcessorRegisters(
+whpx->partition, cpu->cpu_index, _name, 1, );
+if (FAILED(hr)) {
+error_report("WHPX: Failed to get register xcr0, hr=%08lx", hr);
+return;
+}
+
+env->xcr0 = xcr0.Reg64;
+}
+
static void whpx_get_registers(CPUState *cpu)
{
struct whpx_state *whpx = _global;
@@ -457,6 +514,12 @@ static void whpx_get_registers(CPUState *cpu)
/* 8 Debug Registers - Skipped */
+/*
+ * Extended control registers needs to be handled separately depending
+ * on whether xsave is supported/enabled or not.
+ */
+whpx_get_xcrs(cpu);
+
/* 16 XMM registers */
assert(whpx_register_names[idx] == WHvX64RegisterXmm0);
idx_next = idx + 16;
@@ -1395,6 +1458,31 @@ static int whpx_accel_init(MachineState *ms)
goto error;
}
+/*
+ * Query the XSAVE capability of the partition. Any error here is not
+ * considered fatal.
+ */
+hr = whp_dispatch.WHvGetPartitionProperty(
+whpx->partition,
+WHvPartitionPropertyCodeProcessorXsaveFeatures,
+_xsave_cap,
+sizeof(whpx_xsave_cap),
+_cap_size);
+
+/*
+ * Windows version which don't support this property will return with the
+ * specific error code.
+ */
+if (FAILED(hr) && hr != WHV_E_UNKNOWN_PROPERTY) {
+error_report("WHPX: Failed to query XSAVE capability, hr=%08lx", hr);
+}
+
+if (whpx_has_xsave()) {
+printf("WHPX: Partition XSAVE capable\n");
+} else {
+printf("WHPX:
Re: [PATCH v7 6/8] Acceptance tests: add the build directory to the system PATH
On 11/4/19 1:13 PM, Cleber Rosa wrote:
So that when binaries such as qemu-img are searched for, those in the
build tree will be favored. As a clarification, SRC_ROOT_DIR is
dependent on the location from where tests are executed, so they are
equal to the build directory if one is being used.
The original motivation is that Avocado libraries such as
avocado.utils.vmimage.get() may use the matching binaries, but it may
also apply to any other binary that test code may eventually attempt
to execute.
Signed-off-by: Cleber Rosa
---
tests/acceptance/avocado_qemu/__init__.py | 6 ++
1 file changed, 6 insertions(+)
diff --git a/tests/acceptance/avocado_qemu/__init__.py
b/tests/acceptance/avocado_qemu/__init__.py
index 17ce583c87..a4bb796a47 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -110,6 +110,12 @@ class Test(avocado.Test):
return None
def setUp(self):
+# Some utility code uses binaries from the system's PATH. For
+# instance, avocado.utils.vmimage.get() uses qemu-img, to
+# create a snapshot image. This is a transparent way of
Because PATH is changed in a transparent way, wouldn't be better to also
self.log.info() that fact?
+# making sure those utilities find and use binaries on the
+# build tree by default.
+os.environ['PATH'] = '%s:%s' % (SRC_ROOT_DIR, os.environ['PATH'])
I think PATH should be set only once at class initialization. Perhaps in
setUpClass()?
- Wainer
self._vms = {}
self.arch = self.params.get('arch',
Should QEMU's configure script check for bzip2 ?
Hi, I just tried to compile QEMU on a freshly installed system. "configure" finished without problems, but during "make" I hit this error: BUNZIP2 pc-bios/edk2-i386-secure-code.fd.bz2 /bin/sh: bzip2: command not found make: *** [Makefile:305: pc-bios/edk2-i386-secure-code.fd] Error 127 make: *** Deleting file 'pc-bios/edk2-i386-secure-code.fd' make: *** Waiting for unfinished jobs Sure, it's easy to fix, but maybe "configure" should already check for the availablity of "bzip2", so that we then either skip the installation of the edk2 images if "bzip2" is not available, or bail out during "configure" already? Thomas
Re: [PATCH 4/4] Added tests for close and change of logfile.
Thanks for providing the stack trace.
We debugged this and it seems to come about because of an interesting
circumstance. We added our new tests after a pre-existing test,
parse_path(), which runs into an issue, a dangling pointer, which
could lead to a double free. There were no other tests after the test
that ran into the issue, so the double free was not exposed until we
added our test which called qemu_set_log_filename().
Upon entry to qemu_set_log_filename() it frees logfilename. In the
case where we get an error, we return out without setting the
logfilename to NULL.
And on next call into this function we will end up with a double free.
For a fix, we could put this at the beginning of qemu_set_log_filename().
if (logfilename) {
g_free(logfilename);
logfilename = NULL;
}
We were curious to understand why we did not see it in our own
testing. Although we did run make check before our first post, we did
not see this issue. The docker tests seem to use something like
MALLOC_CHECK_, which catches memory issues like this. We will be
sure to run the docker tests as well in the future.
On Thu, 7 Nov 2019 at 12:26, Alex Bennée wrote:
>
>
> Alex Bennée writes:
>
> > Robert Foley writes:
> >
> >> One test ensures that the logfile handle is still valid even if
> >> the logfile is changed during logging.
> >> The other test validates that the logfile handle remains valid under
> >> the logfile lock even if the logfile is closed.
>
> Also this doesn't see to work:
>
> 17:24:31 [alex@zen:~/l/q/b/all] review/rcu-logfile|… 2 + ./tests/test-logging
> /logging/parse_range: OK
> /logging/parse_path: OK
> /logging/logfile_write_path: free(): double free detected in tcache 2
> fish: “./tests/test-logging” terminated by signal SIGABRT (Abort)
>
> in gdb
>
> Starting program: /home/alex/lsrc/qemu.git/builds/all/tests/test-logging
> [Thread debugging using libthread_db enabled]
> Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
> [New Thread 0x76f38700 (LWP 28960)]
> /logging/parse_range: OK
> /logging/parse_path: OK
> /logging/logfile_write_path: free(): double free detected in tcache 2
>
> Thread 1 "test-logging" received signal SIGABRT, Aborted.
> __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
> 50 ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
> (gdb) bt
> #0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
> #1 0x77587535 in __GI_abort () at abort.c:79
> #2 0x775de508 in __libc_message (action=action@entry=do_abort,
> fmt=fmt@entry=0x776e928d "%s\n") at ../sysdeps/posix/libc_fatal.c:181
> #3 0x775e4c1a in malloc_printerr (str=str@entry=0x776eaf58
> "free(): double free detected in tcache 2") at malloc.c:5341
> #4 0x775e66fd in _int_free (av=0x77720c40 ,
> p=0x555cac40, have_lock=) at malloc.c:4193
> #5 0x555614a8 in qemu_set_log_filename (filename=0x555cb110
> "/tmp/qemu-test-logging.RO35A0/qemu_test_log_write0.log",
> errp=0x7fffdef0) at /home/alex/lsrc/qemu.git/util/log.c:148
> #6 0xd8be in test_logfile_write (data=0x555c7370) at
> /home/alex/lsrc/qemu.git/tests/test-logging.c:127
> #7 0x77cdc15a in test_case_run (tc=0x555c9c60) at
> ../../../glib/gtestutils.c:2318
> #8 g_test_run_suite_internal (suite=suite@entry=0x555c8a40,
> path=path@entry=0x0) at ../../../glib/gtestutils.c:2403
> #9 0x77cdc014 in g_test_run_suite_internal
> (suite=suite@entry=0x555c8a20, path=path@entry=0x0) at
> ../../../glib/gtestutils.c:2415
> #10 0x77cdc412 in g_test_run_suite (suite=0x555c8a20) at
> ../../../glib/gtestutils.c:2490
> #11 0x77cdc431 in g_test_run () at ../../../glib/gtestutils.c:1755
> #12 0xce07 in main (argc=, argv=)
> at /home/alex/lsrc/qemu.git/tests/test-logging.c:212
>
>
> >>
> >> Signed-off-by: Robert Foley
> >> ---
> >> tests/test-logging.c | 74
> >> 1 file changed, 74 insertions(+)
> >>
> >> diff --git a/tests/test-logging.c b/tests/test-logging.c
> >> index a12585f70a..a3190ff92c 100644
> >> --- a/tests/test-logging.c
> >> +++ b/tests/test-logging.c
> >> @@ -108,6 +108,76 @@ static void test_parse_path(gconstpointer data)
> >> error_free_or_abort();
> >> }
> >>
> >> +static void test_logfile_write(gconstpointer data)
> >> +{
> >> +QemuLogFile *logfile;
> >> +gchar const *dir = data;
> >> +Error *err = NULL;
> >> +gchar *file_path;
> >> +gchar *file_path1;
> >
> > with g_autofree char *file_path you can avoid the free down bellow.
> >
> >> +FILE *orig_fd;
> >> +
> >> +file_path = g_build_filename(dir, "qemu_test_log_write0.log", NULL);
> >> +file_path1 = g_build_filename(dir, "qemu_test_log_write1.log", NULL);
> >> +
> >> +/*
> >> + * Test that even if an open file handle is changed,
> >> + * our handle remains valid due to RCU.
> >> + */
Re: [PATCH-for-4.1? 0/7] vl: Allow building with CONFIG_BLUETOOTH disabled
On 07/11/2019 20.09, Philippe Mathieu-Daudé wrote: [...] Bluetooth is dead, long live BT! v4.2.0-rc0 just got tagged. We should stop linking unmaintained dead code. If nobody step in to nuke BT, we should consider applying this series before we release QEMU 5.0 with dead Bluetooth. This approach is still better than burying our head in the sand. FWIW, I'm planning to send some patches to remove the bluetooth code in the 5.0 development cycle. Unless you want to beat me to it. But I don't think it makes sense to still make this configurable. The bluetooth code has been marked as deprecated for a while now, and nobody spoke up that they are still using it (and as far as I can tell, it's currently not usable anymore anyway), so it should simply be removed now. Thomas
[PATCH 2/3] qtest: fix qtest_qmp_device_add leak
Spotted by ASAN. Signed-off-by: Marc-André Lureau --- tests/libqtest.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/libqtest.c b/tests/libqtest.c index 3706bccd8d..91e9cb220c 100644 --- a/tests/libqtest.c +++ b/tests/libqtest.c @@ -1274,6 +1274,7 @@ void qtest_qmp_device_add(QTestState *qts, const char *driver, const char *id, qdict_put_str(args, "id", id); qtest_qmp_device_add_qdict(qts, driver, args); +qobject_unref(args); } static void device_deleted_cb(void *opaque, const char *name, QDict *data) -- 2.24.0.rc0.20.gd81542e6f3
[PATCH 1/3] virtio-input: fix memory leak on unrealize
Spotted by ASAN + minor stylistic change.
Signed-off-by: Marc-André Lureau
---
hw/input/virtio-input.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/hw/input/virtio-input.c b/hw/input/virtio-input.c
index 51617a5885..ec54e46ad6 100644
--- a/hw/input/virtio-input.c
+++ b/hw/input/virtio-input.c
@@ -275,6 +275,7 @@ static void virtio_input_finalize(Object *obj)
g_free(vinput->queue);
}
+
static void virtio_input_device_unrealize(DeviceState *dev, Error **errp)
{
VirtIOInputClass *vic = VIRTIO_INPUT_GET_CLASS(dev);
@@ -288,6 +289,8 @@ static void virtio_input_device_unrealize(DeviceState *dev,
Error **errp)
return;
}
}
+virtio_del_queue(vdev, 0);
+virtio_del_queue(vdev, 1);
virtio_cleanup(vdev);
}
--
2.24.0.rc0.20.gd81542e6f3
[PATCH 0/3] Some memory leak fixes
Hi, Another bunch of fixes spotted by ASAN when running check-qtest-x86_64. Marc-André Lureau (3): virtio-input: fix memory leak on unrealize qtest: fix qtest_qmp_device_add leak cpu-plug-test: fix leaks hw/input/virtio-input.c | 3 +++ tests/cpu-plug-test.c | 2 ++ tests/libqtest.c| 1 + 3 files changed, 6 insertions(+) -- 2.24.0.rc0.20.gd81542e6f3
[PATCH 3/3] cpu-plug-test: fix leaks
Spotted by ASAN.
Signed-off-by: Marc-André Lureau
---
tests/cpu-plug-test.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tests/cpu-plug-test.c b/tests/cpu-plug-test.c
index 058cef5ac1..30e514bbfb 100644
--- a/tests/cpu-plug-test.c
+++ b/tests/cpu-plug-test.c
@@ -99,6 +99,7 @@ static void test_plug_with_device_add(gconstpointer data)
cpu = qobject_to(QDict, e);
if (qdict_haskey(cpu, "qom-path")) {
+qobject_unref(e);
continue;
}
@@ -107,6 +108,7 @@ static void test_plug_with_device_add(gconstpointer data)
qtest_qmp_device_add_qdict(qts, td->device_model, props);
hotplugged++;
+qobject_unref(e);
}
/* make sure that there were hotplugged CPUs */
--
2.24.0.rc0.20.gd81542e6f3
Re: [PATCH v7 5/8] Acceptance tests: keep a stable reference to the QEMU build dir
On 11/4/19 1:13 PM, Cleber Rosa wrote: This is related to the the differences in in-tree and out-of-tree builds in QEMU. For simplification, means my build directory. Currently, by running a `make check-acceptance` one gets (in tests/acceptance/avocado_qemu/__init__.py): SRC_ROOT_DIR: /tests/acceptance/avocado_qemu/../../.. This in itself is problematic, because after the parent directories are applied, one may be left not with a pointer to the build directory as intended, but with the location of the source tree (assuming they differ). Built binaries, such as qemu-img, are of course not there and can't be found. Given that a Python '__file__' will contain the absolute path to the file backing the module, say: __file__: /tests/acceptance/avocado_qemu/__init__.py | 4 | 3| 2 | 1 | A solution is to not "evaluate" the third parent dir (marked as 4 here) because that ends up following the "tests" directory symlink to the source tree. In fact, there's no need to keep or evaluate any of the parent directories, we can just drop the rightmost 4 components, and we'll keep a stable reference to the build directory (with no symlink being followed). This works for either a dedicated build directory or also a combined source and build tree. Signed-off-by: Cleber Rosa --- tests/acceptance/avocado_qemu/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py index 6618ea67c1..17ce583c87 100644 --- a/tests/acceptance/avocado_qemu/__init__.py +++ b/tests/acceptance/avocado_qemu/__init__.py @@ -16,7 +16,7 @@ import tempfile import avocado -SRC_ROOT_DIR = os.path.join(os.path.dirname(__file__), '..', '..', '..') +SRC_ROOT_DIR = os.path.dirname(os.path.dirname(os.path.dirname(os.path.dirname(__file__ In this case, wouldn't make sense to rename the constant from SRC_ROOT_DIR to BUILD_ROOT_DIR? This patch looks good to me besides that. - Wainer sys.path.append(os.path.join(SRC_ROOT_DIR, 'python')) from qemu.machine import QEMUMachine
Re: [PATCH v1 Resend] target/i386: set the CPUID level to 0x14 on old machine-type
On Wed, Nov 06, 2019 at 12:55:32AM +, Kang, Luwei wrote:
> > > The CPUID level need to be set to 0x14 manually on old machine-type if
> > > Intel PT is enabled in guest. e.g. in Qemu 3.1 -machine pc-i440fx-3.1
> > > -cpu qemu64,+intel-pt will be CPUID[0].EAX(level)=7 and
> > > CPUID[7].EBX[25](intel-pt)=1.
> > >
> > > Some Intel PT capabilities are exposed by leaf 0x14 and the missing
> > > capabilities will cause some MSRs access failed.
> > > This patch add a warning message to inform the user to extend the
> > > CPUID level.
> >
> > Note that a warning is not an acceptable fix for a QEMU crash.
> > We still need to fix the QEMU crash reported at:
> > https://lore.kernel.org/qemu-devel/20191024141536.gu6...@habkost.net/
> >
> >
> > >
> > > Suggested-by: Eduardo Habkost
> > > Signed-off-by: Luwei Kang
> >
> > The subject line says "v1", but this patch is different from the
> > v1 you sent earlier.
> >
> > If you are sending a different patch, please indicate it is a new version.
> > Please also
> > indicate what changed between different patch versions, to help review.
>
> Got it. I fix a code style problem in resending patch (remove the '\n').
>
> ERROR: Error messages should not contain newlines
> #36: FILE: target/i386/cpu.c:5448:
> +"by \"-cpu ...,+intel-pt,level=0x14\"\n");
> total: 1 errors, 0 warnings, 14 lines checked
>
> >
> > > ---
> > > target/i386/cpu.c | 8 ++--
> > > 1 file changed, 6 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c index
> > > a624163..f67c479 100644
> > > --- a/target/i386/cpu.c
> > > +++ b/target/i386/cpu.c
> > > @@ -5440,8 +5440,12 @@ static void x86_cpu_expand_features(X86CPU
> > > *cpu, Error **errp)
> > >
> > > /* Intel Processor Trace requires CPUID[0x14] */
> > > if ((env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_INTEL_PT) &&
> > > - kvm_enabled() && cpu->intel_pt_auto_level) {
> >
> > Not directly related to the warning: do you know why we have a
> > kvm_enabled() check here? It seems unnecessary. We want CPUID level to be
> > correct
> > for all accelerators.
>
> Intel PT virtualization enabling in KVM guest need some hardware enhancement
> and
> EPT must be enabled in KVM. I think it can't work for e.g. tcg pure
> simulation accelerator.
I don't get it. If what you are saying is true, checking for
kvm_enabled() here is completely unnecessary. If it is not,
checking for kvm_enabled() here is incorrect.
>
> >
> > > -x86_cpu_adjust_level(cpu, >env.cpuid_min_level, 0x14);
> > > + kvm_enabled()) {
> > > +if (cpu->intel_pt_auto_level)
> > > +x86_cpu_adjust_level(cpu, >env.cpuid_min_level,
> > > 0x14);
> > > +else
> > > +warn_report("Intel PT need CPUID leaf 0x14, please set "
> > > +"by \"-cpu ...,+intel-pt,level=0x14\"");
> >
> > The warning shouldn't be triggered if level is already >= 0x14.
> >
> > It is probably a good idea to mention that this happens only on
> > pc-*-3.1 and older, as updating the machine-type is a better solution to
> > the problem
> > than manually setting the "level"
> > property.
> >
> > This will print the warning multiple times if there are multiple VCPUs.
> > You can use
> > warn_report_once() to avoid that.
>
> Got it. Will fix.
>
> As you mentioned in this email " a warning is not an acceptable fix for a
> QEMU crash."
> We can't change the configuration of the old machine type because it may
> break the
> ABI compatibility. May I add more check on Intel PT, if CPUID[7].EBX[25]
> (intel-pt) = 1
> and level is <0x14, mask off this feature? Or do you have any other
> suggestions?
Masking off the feature if level is < 0x14 would possibly work if
we are 100% sure that existing kernel+QEMU versions crashed when
(intel-pt=on && level < 0x14)
so there's no ABI compatibility with working configurations to
worry about. But it would be even better to make kvm_put_msrs()
less fragile. Unexpected CPUID data shouldn't make the function
crash.
--
Eduardo
Re: [Qemu-devel] Exposing feature deprecation to machine clients
07.11.2019 21:52, Philippe Mathieu-Daudé wrote:
> Hi Markus,
>
> On 8/15/19 7:40 PM, John Snow wrote:
>> On 8/15/19 10:16 AM, Markus Armbruster wrote:
>>> John Snow writes:
> [...]
I asked Markus this not too long ago; do we want to amend the QAPI
schema specification to allow commands to return with "Warning" strings,
or "Deprecated" stings to allow in-band deprecation notices for cases
like these?
example:
{ "return": {},
"deprecated": True,
"warning": "Omitting filter-node-name parameter is deprecated, it will
be required in the future"
}
There's no "error" key, so this should be recognized as success by
compatible clients, but they'll definitely see the extra information.
>>>
>>> This is a compatible evolution of the QMP protocol.
>>>
Part of my motivation is to facilitate a more aggressive deprecation of
legacy features by ensuring that we are able to rigorously notify users
through any means that they need to adjust their scripts.
>>>
>>> Yes, we should help libvirt etc. with detecting use of deprecated
>>> features. We discussed this at the KVM Forum 2018 BoF on deprecating
>>> stuff. Minutes:
>>>
>>> Message-ID: <87mur0ls8o@dusky.pond.sub.org>
>>> https://lists.nongnu.org/archive/html/qemu-devel/2018-10/msg05828.html
>>>
>>> Last item is relevant here.
>>>
>>> Adding deprecation information to QMP's success response belongs to "We
>>> can also pass the buck to the next layer up", next to "emit a QMP
>>> event".
>>>
>>> Let's compare the two, i.e. "deprecation info in success response"
>>> vs. "deprecation event".
>>>
>>> 1. Possible triggers
>>>
>>> Anything we put in the success response should only ever apply to the
>>> (successful) command. So this one's limited to QMP commands.
>>>
>>> A QMP event is not limited to QMP commands. For instance, it could be
>>> emitted for deprecated CLI features (long after the fact, in addition to
>>> human-readable warnings on stderr), or when we detect use of a
>>> deprecated feature only after we sent the success response, say in a
>>> job. Neither use case is particularly convincing. Reporting use of
>>> deprecated CLI in QMP feels like a work-around for the CLI's
>>> machine-unfriendliness. Job-like commands should really check their
>>> arguments upfront.
>>>
>>> 2. Connection to trigger
>>>
>>> Connecting responses to commands is the QMP protocol's responsibility.
>>> Transmitting deprecation information in the response trivially ties it
>>> to the offending command.
>>>
>>> The QMP protocol doesn't tie events to anything. Thus, a deprecation
>>> event needs an event-specific tie to its trigger.
>>>
>>> The obvious way to tie it to a command mirrors how the QMP protocol ties
>>> responses to commands: by command ID. The event either has to be sent
>>> just to the offending monitor (currently, all events are broadcast to
>>> all monitors), or include a suitable monitor ID.
>>>
>>> For non-command triggers, we'd have to invent some other tie.
>>>
>>> 3. Interface complexity
>>>
>>> Tying the event to some arbitrary trigger adds complexity.
>>>
>>> Do we need non-command triggers, and badly enough to justify the
>>> additional complexity?
>>>
>>> 4. Implementation complexity
>>>
>>> Emitting an event could be as simple as
>>>
>>> qapi_event_send_deprecated(qmp_command_id(),
>>> "Omitting 'filter-node-name'");
>>>
>>> where qmp_command_id() returns the ID of the currently executing
>>> command. Making qmp_command_id() work is up to the QMP core. Simple
>>> enough as long as each QMP command runs to completion before its monitor
>>> starts the next one.
>>>
>>> The event is "fire and forget". There is no warning object propagated
>>> up the call chain into the QMP core like errors objects are.
>>>
>>> "Fire and forget" is ideal for letting arbitrary code decide "this is
>>> deprecated".
>>>
>>> Note the QAPI schema remains untouched.
>>>
>>> Unlike an event, which can be emitted anywhere, the success response
>>> gets built in the QMP core. To have the core add deprecation info to
>>> it, we need to get the info to the core.
>>>
>>> If deprecation info originates in command code, like errors do, we need
>>> to propagate it up the call chain into the QMP core like errors.
>>>
>>> Propagating errors is painful. It has caused massive churn all over the
>>> place.
>>>
>>> I don't think we can hitch deprecation info to the existing error
>>> propagation, since we need to take the success path back to the QMP
>>> core, not an error path.
>>>
>>> Propagating a second object for warnings... thanks, but no thanks.
>>>
>>
>> Probably the best argument against it. Fire-and-forget avoids the
>> problem. Events might work just fine, but the "tie" bit seems like a yak
>> in need of a shave.
>>
>>> The QMP core could provide a function for recording deprecation info for
>>> the currently executing QMP command.
Re: [PATCH-for-4.1? 0/7] vl: Allow building with CONFIG_BLUETOOTH disabled
On 8/14/19 2:45 PM, Philippe Mathieu-Daudé wrote: On 8/13/19 4:04 PM, Peter Maydell wrote: On Tue, 13 Aug 2019 at 15:01, Philippe Mathieu-Daudé wrote: On 7/15/19 3:13 PM, Thomas Huth wrote: On 12/07/2019 15.39, Philippe Mathieu-Daudé wrote: A series of obvious patches to build without the deprecated bluetooth devices. Still worth for 4.1 or too late? It is clearly not a bugfix. I wonder whether this series is worth the effort right now, or whether we should simply nuke the bluetooth code after 4.1 has been released? Well, perfect is the enemy of good :) This series is already done and is an improvement to what we have. Regarding nuking it, it depends on the Nokia N-series boards, they might become useless without BT support. Er, they're not useless at all without BT support. The BT hardware is a really tiny part that I doubt many users of the board models ever used. As long as we retain a "simulate doing nothing much" model of the BT device to show the guest I don't care whether the BT backend code disappears. OK, I won't insist then. Bluetooth is dead, long live BT! v4.2.0-rc0 just got tagged. We should stop linking unmaintained dead code. If nobody step in to nuke BT, we should consider applying this series before we release QEMU 5.0 with dead Bluetooth. This approach is still better than burying our head in the sand.
Re: [PATCH v2] virtio: notify virtqueue via host notifier when available
Thanks Stefan for the quick fix! Sorry for not adding a Tested-by.
It's implicit. :)
F.
> On Nov 6, 2019, at 11:33 AM, Michael S. Tsirkin wrote:
>
> On Tue, Nov 05, 2019 at 03:09:46PM +0100, Stefan Hajnoczi wrote:
>> Host notifiers are used in several cases:
>> 1. Traditional ioeventfd where virtqueue notifications are handled in
>> the main loop thread.
>> 2. IOThreads (aio_handle_output) where virtqueue notifications are
>> handled in an IOThread AioContext.
>> 3. vhost where virtqueue notifications are handled by kernel vhost or
>> a vhost-user device backend.
>>
>> Most virtqueue notifications from the guest use the ioeventfd mechanism,
>> but there are corner cases where QEMU code calls virtio_queue_notify().
>> This currently honors the host notifier for the IOThreads
>> aio_handle_output case, but not for the vhost case. The result is that
>> vhost does not receive virtqueue notifications from QEMU when
>> virtio_queue_notify() is called.
>>
>> This patch extends virtio_queue_notify() to set the host notifier
>> whenever it is enabled instead of calling the vq->(aio_)handle_output()
>> function directly. We track the host notifier state for each virtqueue
>> separately since some devices may use it only for certain virtqueues.
>>
>> This fixes the vhost case although it does add a trip through the
>> eventfd for the traditional ioeventfd case. I don't think it's worth
>> adding a fast path for the traditional ioeventfd case because calling
>> virtio_queue_notify() is rare when ioeventfd is enabled.
>>
>> Reported-by: Felipe Franciosi
>> Signed-off-by: Stefan Hajnoczi
>
> queued, thanks!
>
>> ---
>> v2:
>> * Track host notifier enabled/disabled state per virtqueue [Yongji Xie]
>> * Tested with contrib/vhost-user-blk and contrib/vhost-user-scsi
>>
>> hw/virtio/virtio-bus.c | 4
>> hw/virtio/virtio.c | 9 -
>> include/hw/virtio/virtio.h | 1 +
>> 3 files changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
>> index b2c804292e..d6332d45c3 100644
>> --- a/hw/virtio/virtio-bus.c
>> +++ b/hw/virtio/virtio-bus.c
>> @@ -288,6 +288,10 @@ int virtio_bus_set_host_notifier(VirtioBusState *bus,
>> int n, bool assign)
>> k->ioeventfd_assign(proxy, notifier, n, false);
>> }
>>
>> +if (r == 0) {
>> +virtio_queue_set_host_notifier_enabled(vq, assign);
>> +}
>> +
>> return r;
>> }
>>
>> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
>> index 762df12f4c..04716b5f6c 100644
>> --- a/hw/virtio/virtio.c
>> +++ b/hw/virtio/virtio.c
>> @@ -128,6 +128,7 @@ struct VirtQueue
>> VirtIODevice *vdev;
>> EventNotifier guest_notifier;
>> EventNotifier host_notifier;
>> +bool host_notifier_enabled;
>> QLIST_ENTRY(VirtQueue) node;
>> };
>>
>> @@ -2271,7 +2272,7 @@ void virtio_queue_notify(VirtIODevice *vdev, int n)
>> }
>>
>> trace_virtio_queue_notify(vdev, vq - vdev->vq, vq);
>> -if (vq->handle_aio_output) {
>> +if (vq->host_notifier_enabled) {
>> event_notifier_set(>host_notifier);
>> } else if (vq->handle_output) {
>> vq->handle_output(vdev, vq);
>> @@ -3145,6 +3146,7 @@ void virtio_init(VirtIODevice *vdev, const char *name,
>> vdev->vq[i].vector = VIRTIO_NO_VECTOR;
>> vdev->vq[i].vdev = vdev;
>> vdev->vq[i].queue_index = i;
>> +vdev->vq[i].host_notifier_enabled = false;
>> }
>>
>> vdev->name = name;
>> @@ -3436,6 +3438,11 @@ EventNotifier
>> *virtio_queue_get_host_notifier(VirtQueue *vq)
>> return >host_notifier;
>> }
>>
>> +void virtio_queue_set_host_notifier_enabled(VirtQueue *vq, bool enabled)
>> +{
>> +vq->host_notifier_enabled = enabled;
>> +}
>> +
>> int virtio_queue_set_host_notifier_mr(VirtIODevice *vdev, int n,
>> MemoryRegion *mr, bool assign)
>> {
>> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
>> index 3448d67d2a..c32a815303 100644
>> --- a/include/hw/virtio/virtio.h
>> +++ b/include/hw/virtio/virtio.h
>> @@ -312,6 +312,7 @@ int virtio_device_grab_ioeventfd(VirtIODevice *vdev);
>> void virtio_device_release_ioeventfd(VirtIODevice *vdev);
>> bool virtio_device_ioeventfd_enabled(VirtIODevice *vdev);
>> EventNotifier *virtio_queue_get_host_notifier(VirtQueue *vq);
>> +void virtio_queue_set_host_notifier_enabled(VirtQueue *vq, bool enabled);
>> void virtio_queue_host_notifier_read(EventNotifier *n);
>> void virtio_queue_aio_set_host_notifier_handler(VirtQueue *vq, AioContext
>> *ctx,
>> VirtIOHandleAIOOutput
>> handle_output);
>> --
>> 2.23.0
Re: [PULL 0/1] Usb 20191107 patches
On Thu, 7 Nov 2019 at 18:57, Philippe Mathieu-Daudé wrote: > > Hi Peter, > > On 11/7/19 7:26 PM, Peter Maydell wrote: > > On Thu, 7 Nov 2019 at 08:58, Gerd Hoffmann wrote: > >> > >> The following changes since commit > >> 412fbef3d076c43e56451bacb28c4544858c66a3: > >> > >>Merge remote-tracking branch > >> 'remotes/philmd-gitlab/tags/fw_cfg-next-pull-request' into staging > >> (2019-11-05 20:17:11 +) > >> > >> are available in the Git repository at: > >> > >>git://git.kraxel.org/qemu tags/usb-20191107-pull-request > >> > >> for you to fetch changes up to 1dfe2b91dcb1633d0ba450a8139d53006e700a9b: > >> > >>usb-host: add option to allow all resets. (2019-11-06 13:26:04 +0100) > >> > >> > >> usb: fix for usb-host > >> > >> > >> > >> Gerd Hoffmann (1): > >>usb-host: add option to allow all resets. > >> > >> hw/usb/host-libusb.c | 13 + > >> 1 file changed, 9 insertions(+), 4 deletions(-) > > > > This didn't quite make rc0 but it'll go in for rc1. > > Won't this make bisection confusing? No, why should it? thanks -- PMM
Re: [PATCH for 4.2 v1 1/1] riscv/virt: Increase flash size
Alistair Francis writes: > On Thu, Nov 7, 2019 at 10:01 AM Peter Maydell > wrote: >> >> On Thu, 7 Nov 2019 at 17:09, Palmer Dabbelt wrote: >> > >> > On Wed, 06 Nov 2019 16:47:20 PST (-0800), Alistair Francis wrote: >> > > Coreboot developers have requested that they have at least 32MB of flash >> > > to load binaries. We currently have 32MB of flash, but it is split in >> > > two to allow loading two flash binaries. Let's increase the flash size >> > > from 32MB to 64MB to ensure we have a single region that is 32MB. >> > > >> > > No QEMU release has include flash in the RISC-V virt machine, so this >> > > isn't a breaking change. >> > >> > Even if we had, I wouldn't consider it a breaking change because it adds to >> > the memory map so existing programs will continue to run fine. >> >> I have a feeling you may find that some old command lines won't >> work any more because they specified a flash contents binary >> that was the old 32MB and now it needs to be padded out to 64MB. > > Yes, that is correct. Everyone using -pflash will need to change the > size of their binaries. This was only just merged into QEMU master > though and hasn't been in a release so I don't think many people are > using it. > > I only know of two users, one is me and someone from Coreboot who > requested the larger size. It doesn't seem like a problem users will > see. At least the error message they get will be more informative now ;-) -- Alex Bennée
Re: [PULL 0/1] Usb 20191107 patches
Hi Peter, On 11/7/19 7:26 PM, Peter Maydell wrote: On Thu, 7 Nov 2019 at 08:58, Gerd Hoffmann wrote: The following changes since commit 412fbef3d076c43e56451bacb28c4544858c66a3: Merge remote-tracking branch 'remotes/philmd-gitlab/tags/fw_cfg-next-pull-request' into staging (2019-11-05 20:17:11 +) are available in the Git repository at: git://git.kraxel.org/qemu tags/usb-20191107-pull-request for you to fetch changes up to 1dfe2b91dcb1633d0ba450a8139d53006e700a9b: usb-host: add option to allow all resets. (2019-11-06 13:26:04 +0100) usb: fix for usb-host Gerd Hoffmann (1): usb-host: add option to allow all resets. hw/usb/host-libusb.c | 13 + 1 file changed, 9 insertions(+), 4 deletions(-) This didn't quite make rc0 but it'll go in for rc1. Won't this make bisection confusing?
Re: [PATCH v7 4/8] Acceptance tests: use relative location for tests
On 11/4/19 1:13 PM, Cleber Rosa wrote: An Avocado Test ID[1] is composed by a number of components, but it starts with the Test Name, usually a file system location that was given to the loader. Because the source directory is being given as a prefix to the "tests/acceptance" directory containing the acceptance tests, the test names will needlessly include the directory the user is using to host the QEMU sources (and/or build tree). Let's remove the source dir (or a build dir) from the path given to the test loader. This should give more constant names, and when using result servers and databases, it should give the same test names across executions from different people or from different directories. Completely agree. [1] - https://avocado-framework.readthedocs.io/en/69.0/ReferenceGuide.html#test-id Signed-off-by: Cleber Rosa --- tests/Makefile.include | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Wainer dos Santos Moschetta - Wainer diff --git a/tests/Makefile.include b/tests/Makefile.include index 56f73b46e2..65e85f5275 100644 --- a/tests/Makefile.include +++ b/tests/Makefile.include @@ -1180,7 +1180,7 @@ check-acceptance: check-venv $(TESTS_RESULTS_DIR) --show=$(AVOCADO_SHOW) run --job-results-dir=$(TESTS_RESULTS_DIR) \ --filter-by-tags-include-empty --filter-by-tags-include-empty-key \ $(AVOCADO_TAGS) \ ---failfast=on $(SRC_PATH)/tests/acceptance, \ +--failfast=on tests/acceptance, \ "AVOCADO", "tests/acceptance") # Consolidated targets
Re: Looking for issues/features for my first contribution
Hi, Thanks Stefan ! I spoke to Dinah and this issue is still up for grabs. I would be working on both SeaBIOS MMConfig task and the DS3231 RTC emulation feature. Thanks for your help! Regards, Rajath Shashidhara On 07-11-2019 07:54, Stefan Hajnoczi wrote: On Wed, Nov 06, 2019 at 05:50:44PM -0600, Rajath Shashidhara wrote: Hi all, I am a Computer Science graduate student at The University of Texas at Austin (UT, Austin). I am looking forward to contributing to qemu ! This semester, I am taking a class in Virtualization (https://github.com/vijay03/cs378-f19) and contributing to a virtualization related open-source project is a significant part of the course. I would be interested in contributing a patchset to qemu - possibly a self-contained feature or a reasonably complex bug fix that can be completed in under a month's time. I did look at both the bugtracker and the QEMU Google Summer of Code 2019 page [https://wiki.qemu.org/Google_Summer_of_Code_2019] for ideas. However, I would be interested in hearing from the community and I would be delighted if somebody can be suggest a suitable project ! I am an advanced C programmer with both professional and academic background in systems design & implementation - especially OS & Networks. Given my background, I feel fairly confident that I can pickup the QEMU codebase quickly. Please check with Dinah Baum whether the SeaBIOS MMConfig task is already taken, maybe you'd like to work on it if the task is still available: https://lore.kernel.org/qemu-devel/20191105163952.GI166646@stefanha-x1.localdomain/ Stefan
Re: [Qemu-devel] Exposing feature deprecation to machine clients
Hi Markus,
On 8/15/19 7:40 PM, John Snow wrote:
On 8/15/19 10:16 AM, Markus Armbruster wrote:
John Snow writes:
[...]
I asked Markus this not too long ago; do we want to amend the QAPI
schema specification to allow commands to return with "Warning" strings,
or "Deprecated" stings to allow in-band deprecation notices for cases
like these?
example:
{ "return": {},
"deprecated": True,
"warning": "Omitting filter-node-name parameter is deprecated, it will
be required in the future"
}
There's no "error" key, so this should be recognized as success by
compatible clients, but they'll definitely see the extra information.
This is a compatible evolution of the QMP protocol.
Part of my motivation is to facilitate a more aggressive deprecation of
legacy features by ensuring that we are able to rigorously notify users
through any means that they need to adjust their scripts.
Yes, we should help libvirt etc. with detecting use of deprecated
features. We discussed this at the KVM Forum 2018 BoF on deprecating
stuff. Minutes:
Message-ID: <87mur0ls8o@dusky.pond.sub.org>
https://lists.nongnu.org/archive/html/qemu-devel/2018-10/msg05828.html
Last item is relevant here.
Adding deprecation information to QMP's success response belongs to "We
can also pass the buck to the next layer up", next to "emit a QMP
event".
Let's compare the two, i.e. "deprecation info in success response"
vs. "deprecation event".
1. Possible triggers
Anything we put in the success response should only ever apply to the
(successful) command. So this one's limited to QMP commands.
A QMP event is not limited to QMP commands. For instance, it could be
emitted for deprecated CLI features (long after the fact, in addition to
human-readable warnings on stderr), or when we detect use of a
deprecated feature only after we sent the success response, say in a
job. Neither use case is particularly convincing. Reporting use of
deprecated CLI in QMP feels like a work-around for the CLI's
machine-unfriendliness. Job-like commands should really check their
arguments upfront.
2. Connection to trigger
Connecting responses to commands is the QMP protocol's responsibility.
Transmitting deprecation information in the response trivially ties it
to the offending command.
The QMP protocol doesn't tie events to anything. Thus, a deprecation
event needs an event-specific tie to its trigger.
The obvious way to tie it to a command mirrors how the QMP protocol ties
responses to commands: by command ID. The event either has to be sent
just to the offending monitor (currently, all events are broadcast to
all monitors), or include a suitable monitor ID.
For non-command triggers, we'd have to invent some other tie.
3. Interface complexity
Tying the event to some arbitrary trigger adds complexity.
Do we need non-command triggers, and badly enough to justify the
additional complexity?
4. Implementation complexity
Emitting an event could be as simple as
qapi_event_send_deprecated(qmp_command_id(),
"Omitting 'filter-node-name'");
where qmp_command_id() returns the ID of the currently executing
command. Making qmp_command_id() work is up to the QMP core. Simple
enough as long as each QMP command runs to completion before its monitor
starts the next one.
The event is "fire and forget". There is no warning object propagated
up the call chain into the QMP core like errors objects are.
"Fire and forget" is ideal for letting arbitrary code decide "this is
deprecated".
Note the QAPI schema remains untouched.
Unlike an event, which can be emitted anywhere, the success response
gets built in the QMP core. To have the core add deprecation info to
it, we need to get the info to the core.
If deprecation info originates in command code, like errors do, we need
to propagate it up the call chain into the QMP core like errors.
Propagating errors is painful. It has caused massive churn all over the
place.
I don't think we can hitch deprecation info to the existing error
propagation, since we need to take the success path back to the QMP
core, not an error path.
Propagating a second object for warnings... thanks, but no thanks.
Probably the best argument against it. Fire-and-forget avoids the
problem. Events might work just fine, but the "tie" bit seems like a yak
in need of a shave.
The QMP core could provide a function for recording deprecation info for
the currently executing QMP command. This is how we used to record
errors in QMP commands, until Anthony rammed through what we have now.
The commit messages (e.g. d5ec4f27c38) provide no justification. I
dimly recall adamant (oral?) claims that recording errors in the Monitor
object cannot work for us.
I smell a swamp.
Can we avoid plumbing deprecation info from command code to QMP core?
Only if the QMP core itself can recognize deprecated interfaces. I
believe it can for the cases we can expose in introspecion. Let me
explain.
Re: Looking for issues/features for my first contribution
Thank you Aleksandar ! This is really helpful. Rajath Shashidhara On 07-11-2019 07:33, Aleksandar Markovic wrote: On Thu, Nov 7, 2019 at 11:37 AM Aleksandar Markovic wrote: On Thursday, November 7, 2019, Rajath Shashidhara wrote: Hi all, I am a Computer Science graduate student at The University of Texas at Austin (UT, Austin). I am looking forward to contributing to qemu ! This semester, I am taking a class in Virtualization (https://github.com/vijay03/cs378-f19) and contributing to a virtualization related open-source project is a significant part of the course. I would be interested in contributing a patchset to qemu - possibly a self-contained feature or a reasonably complex bug fix that can be completed in under a month's time. I did look at both the bugtracker and the QEMU Google Summer of Code 2019 page [https://wiki.qemu.org/Google_Summer_of_Code_2019] for ideas. However, I would be interested in hearing from the community and I would be delighted if somebody can be suggest a suitable project ! Hello, Rajath! Thank you for expressing interest in QEMU open source project. There is certainly a place for you and your contributions in QEMU, and you are very welcomed! It looks to me the following project would fit your description: 'Implement emulation of DS3231 real time clock in QEMU' Datasheet: https://datasheets.maximintegrated.com/en/ds/DS3231.pdf The steps needed to complete it (in my opinion): - collect datasheets of as many as possible RTC chips already emulated in QEMU (there are around of dozen of them, see folder hw/rtc) I did a quick Google search on datasheets of existing RTC implemtations, and the result is: DS1338: https://datasheets.maximintegrated.com/en/ds/DS1338-DS1338Z.pdf M41T80: https://www.st.com/resource/en/datasheet/m41t80.pdf M48T59: http://www.elektronikjk.pl/elementy_czynne/IC/M48T59V.pdf MC146818: https://www.nxp.com/docs/en/data-sheet/MC146818.pdf PL031: http://infocenter.arm.com/help/topic/com.arm.doc.ddi0224c/real_time_clock_pl031_r1p3_technical_reference_manual_DDI0224C.pdf TWL92230: https://datasheet.octopart.com/TWL92230C-Texas-Instruments-datasheet-150321.pdf Zynq RTC: https://www.xilinx.com/support/documentation/user_guides/ug1085-zynq-ultrascale-trm.pdf (chapter 7) Aleksandar - do a comparative analysis of selected RTC implementations in QEMU - get to know general QEMU device model - design and implement DS3231 emulation I can give you (unfortunately constrained by tight time limits) some help and guidance. But there are other people in community too (more knowledgable in the area than me). I salute your initiative! Yours, Aleksandar I am an advanced C programmer with both professional and academic background in systems design & implementation - especially OS & Networks. Given my background, I feel fairly confident that I can pickup the QEMU codebase quickly. Eagerly looking forward to hearing from the community ! Thanks, Rajath Shashidhara
Re: [PATCH v7 0/8] Acceptance test: Add "boot_linux" acceptance test
Hi Cleber, On 11/4/19 1:13 PM, Cleber Rosa wrote: This acceptance test, validates that a full blown Linux guest can successfully boot in QEMU. In this specific case, the guest chosen is Fedora version 31. * x86_64, pc and q35 machine types, with and without kvm as an accellerator * aarch64 and virt machine type, with and without kvm as an accellerator * ppc64 and pseries machine type * s390x and s390-ccw-virtio machine type This has been tested on x86_64 and ppc64le hosts and has been running reliably (in my experience) on Travis CI. Some hopefully useful pointers for reviewers: = Git Info: - URI: https://github.com/clebergnu/qemu/tree/test_boot_linux_v7 - Remote: https://github.com/clebergnu/qemu - Branch: test_boot_linux_v7 Travis CI Info: - Build: https://travis-ci.org/clebergnu/qemu/builds/606191094 - Job: https://travis-ci.org/clebergnu/qemu/jobs/606191120 Previous version: - v6: https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg01202.html - v5: https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg04652.html - v4: https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg02032.html - v3: https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg01677.html - v2: https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg04318.html - v1: http://lists.nongnu.org/archive/html/qemu-devel/2018-09/msg02530.html Changes from v6: * Bumped Fedora to most recently released version (31). * Included new architectures (ppc64 and s390x), consolidating all tests into the same commit. * New commit: "Acceptance tests: use avocado tags for machine type" * New commit: "Acceptance tests: introduce utility method for tags unique vals" It seems 02 and 03 are re-sending of patches originally sent in the series with Message-Id: 20190924194501.9303-1-cr...@redhat.com. On the original thread you can find my reviews. - Wainer * New commit: "Acceptance test x86_cpu_model_versions: use default vm", needed to normalize the use of the machine type tags * Added a lot of leniency to the test setup (and reliability to the test/job), canceling the test if there are any failures while downloading/preparing the boot images. * Made use of Avocado's data drainer a regular feature (dropped the commit with RFC) and squashed it. * Bumped pycdlib version to 1.8.0 * Dropped explicit "--enable-slirp=git" (added on v5) to Travis CI configure line, as the default configuration on Travis CI now results in user networking capabilities. Changes from v5: * Added explicit "--enable-slirp=git" to Travis CI configure line, as these tests depend on "-netdev user" like networking. * Bumped Fedora to most recently released version (30). * Changed "checksum" parameter to 'sha256' and use the same hashes as provided by the Fedora project (instead of using Avocado's default sha1 and compute and use a different hash value). * New commit: Add "boot_linux" test for aarch64 and virt machine type * New commit: [RFC]: use Avocado data drainer for console logging Changes from v4: * New commit "Acceptance tests: use relative location for tests" * New commit "Acceptance tests: keep a stable reference to the QEMU build dir" * Pinned the Fedora 29 image by adding a checksum. The goal is to never allow more than one component to change at a time (the one allowed to change is QEMU itself). Updates to the image should be manual. (Based on comments from Cornelia) * Moved the downloading of the Fedora 29 cloud image to the test setUp() method, canceling the test if the image can not be downloaded. * Removed the ":avocado: enable" tag, given that Avocado versions 68.0 and later operate on a "recursive by default" manner, that is able to correctly identify this as an Avocado test. Changes from v3: * New patch "Acceptance tests: depend on qemu-img" Known Issues on v3 (no longer applicable): == * A recent TCG performance regression[1] affects this test in a number of ways: - The test execution may timeout by itself - The generation of SSH host keys in the guest's first boot is also affected (possibly also a timeout) - The cloud-init "phone home" feature attempts to read the host keys and fails, causing the test to timeout and fail These are not observed anymore once the fix[2] is applied. [1] - https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg00338.html [2] - https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg01129.html Changes from v2: * Updated the tag to include the "arch:" key, in a similar fashion as to the tests in the "Acceptance Tests: target architecture support": -
Re: [PATCH 0/2] Acceptance test: update kernel used on m68k/q800 test
Le 07/11/2019 à 19:00, Philippe Mathieu-Daudé a écrit : > On 11/7/19 6:18 PM, Laurent Vivier wrote: >> Le 07/11/2019 à 17:38, Cleber Rosa a écrit : >>> - Original Message - From: "Eric Blake" To: "Cleber Rosa" , qemu-devel@nongnu.org Cc: "Peter Maydell" , "Eduardo Habkost" , "Philippe Mathieu-Daudé" , "Wainer dos Santos Moschetta" , "Laurent Vivier" , "Willian Rampazzo" , "Philippe Mathieu-Daudé" Sent: Thursday, November 7, 2019 10:43:08 AM Subject: Re: [PATCH 0/2] Acceptance test: update kernel used on m68k/q800 test On 10/29/19 6:23 PM, Cleber Rosa wrote: > The boot_linux_console.py:BootLinuxConsole.test_m68k_q800 was very > recently merged, but between its last review and now, the Kernel > package used went missing. > meta-question: Why was this series posted in-reply-to the pull request, rather than as a new top-level thread? I nearly missed it because I don't expect to see unreviewed patches buried in threading like that. My workflow would have been to post the series in isolation, then manually reply to the pull request to mention the message-id of the related series proposed as a followup. >>> >>> Hi Eric, >>> >>> That was my attempt to signal that it was a fix to something which >>> had *just* >>> being merged as part of that pull request (though now caused by it). >>> >>> I basically did not know how to act properly, so I thank you for the >>> workflow >>> suggestion. I'll certainly follow it next time. >> >> IMHO, you should send your series and then replies to the pull request >> to tell you have sent your series that fixes the patch in the pull >> request, or vice-versa. >> >> But your series has been queued by Alex, so there is no problem... > > I prepared a different fix around the same time, but closed my laptop > before the patch was sent and noticed the next day: > https://lists.gnu.org/archive/html/qemu-devel/2019-10/msg08120.html > > Laurent, are you OK with the new kernel being tested? > I'm fine. We could have problems with 5.4 because the address mapping has been changed for SWIM (aee6bff1c325 "m68k: mac: Revisit floppy disc controller base addresses), but this has been fixed by my patch that has been merged today in QEMU (653901ca2b "q800: fix I/O memory map"). Thanks, Laurent
Re: [PATCH 2/3] Acceptance tests: introduce utility method for tags unique vals
On 10/28/19 8:02 PM, Cleber Rosa wrote:
On Thu, Oct 24, 2019 at 06:12:25PM -0300, Wainer dos Santos Moschetta wrote:
Hi Cleber,
On 9/24/19 4:45 PM, Cleber Rosa wrote:
Currently a test can describe the target architecture binary that it
should primarily be run with, be setting a single tag value.
The same approach is expected to be done with other QEMU aspects to be
tested, for instance, the machine type and accelerator, so let's
generalize the logic into a utility method.
Signed-off-by: Cleber Rosa
---
tests/acceptance/avocado_qemu/__init__.py | 19 +--
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/tests/acceptance/avocado_qemu/__init__.py
b/tests/acceptance/avocado_qemu/__init__.py
index bd41e0443c..02775bafcf 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -54,14 +54,21 @@ def pick_default_qemu_bin(arch=None):
class Test(avocado.Test):
+def _get_unique_tag_val(self, tag_name):
+"""
+Gets a tag value, if unique for a key
+"""
+vals = self.tags.get(tag_name, [])
+if len(vals) == 1:
An small optimization:
if vals:
return vals.pop()
IIUC, this would break the idea of uniqueness that this method, for
now, has. Read on.
+return vals.pop()
+return None
Does it allows to express a scenario like "I want my test method to run on
x86_64 and aarch64" using tags? If so, _get_unique_tag_val logic returns
None for multi-value tags (e.g. 'tags=arch:x86_64,arch:aarch64').
I thought that initially we should attempt to pick a default arch or
machine type only of len(vals) == 1. Not because what you describe
can't be done, but because I would like to go through the tests and
make sure we run them in all the given tagged arches when we allow
that.
Ok, understood the rationale now.
Reviewed-by: Wainer dos Santos Moschetta
Thanks,
- Cleber.
Thanks,
Wainer
+
def setUp(self):
self._vms = {}
-arches = self.tags.get('arch', [])
-if len(arches) == 1:
-arch = arches.pop()
-else:
-arch = None
-self.arch = self.params.get('arch', default=arch)
+
+self.arch = self.params.get('arch',
+default=self._get_unique_tag_val('arch'))
+
default_qemu_bin = pick_default_qemu_bin(arch=self.arch)
self.qemu_bin = self.params.get('qemu_bin',
default=default_qemu_bin)
Re: [PULL 0/1] Usb 20191107 patches
On Thu, 7 Nov 2019 at 08:58, Gerd Hoffmann wrote: > > The following changes since commit 412fbef3d076c43e56451bacb28c4544858c66a3: > > Merge remote-tracking branch > 'remotes/philmd-gitlab/tags/fw_cfg-next-pull-request' into staging > (2019-11-05 20:17:11 +) > > are available in the Git repository at: > > git://git.kraxel.org/qemu tags/usb-20191107-pull-request > > for you to fetch changes up to 1dfe2b91dcb1633d0ba450a8139d53006e700a9b: > > usb-host: add option to allow all resets. (2019-11-06 13:26:04 +0100) > > > usb: fix for usb-host > > > > Gerd Hoffmann (1): > usb-host: add option to allow all resets. > > hw/usb/host-libusb.c | 13 + > 1 file changed, 9 insertions(+), 4 deletions(-) This didn't quite make rc0 but it'll go in for rc1. thanks -- PMM
Re: [PULL 0/3] Block patches for 4.2.0-rc0/4.1.1
On Thu, 7 Nov 2019 at 14:34, Max Reitz wrote: > > The following changes since commit d0f90e1423b4f412adc620eee93e8bfef8af4117: > > Merge remote-tracking branch > 'remotes/kraxel/tags/audio-20191106-pull-request' into staging (2019-11-07 > 09:21:52 +) > > are available in the Git repository at: > > https://github.com/XanClic/qemu.git tags/pull-block-2019-11-07 > > for you to fetch changes up to b7cd2c11f76d27930f53d3cf26d7b695c78d613b: > > iotests: Add test for 4G+ compressed qcow2 write (2019-11-07 14:37:46 +0100) > > > Block patches for 4.2.0-rc0/4.1.1: > - Fix writing to compressed qcow2 images > 4 GB > - Fix size sanity check for qcow2 bitmaps > Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/4.2 for any user-visible changes. -- PMM
Re: [PATCH for 4.2 v1 1/1] riscv/virt: Increase flash size
On Thu, 7 Nov 2019 at 17:09, Palmer Dabbelt wrote: > > On Wed, 06 Nov 2019 16:47:20 PST (-0800), Alistair Francis wrote: > > Coreboot developers have requested that they have at least 32MB of flash > > to load binaries. We currently have 32MB of flash, but it is split in > > two to allow loading two flash binaries. Let's increase the flash size > > from 32MB to 64MB to ensure we have a single region that is 32MB. > > > > No QEMU release has include flash in the RISC-V virt machine, so this > > isn't a breaking change. > > Even if we had, I wouldn't consider it a breaking change because it adds to > the memory map so existing programs will continue to run fine. I have a feeling you may find that some old command lines won't work any more because they specified a flash contents binary that was the old 32MB and now it needs to be padded out to 64MB. But I haven't tested whether this theory is correct (it will depend on how the flash contents are specified -- --bios will be ok, as will loading contents directly as an ELF file or similar, specifying contents by a -drive option intended to be consumed by the pflash is the case which likely needs extra padding.) thanks -- PMM
[PATCH v2 2/2] i386: Add 2nd Generation AMD EPYC processors
Adds the support for 2nd Gen AMD EPYC Processors. The model display
name will be EPYC-Rome.
Adds the following new feature bits on top of the feature bits from the
first generation EPYC models.
perfctr-core : core performance counter extensions support. Enables the VM to
use extended performance counter support. It enables six
programmable counters instead of four counters.
clzero : instruction zeroes out the 64 byte cache line specified in RAX.
xsaveerptr : XSAVE, XSAVE, FXSAVEOPT, XSAVEC, XSAVES always save error
pointers and FXRSTOR, XRSTOR, XRSTORS always restore error
pointers.
wbnoinvd : Write back and do not invalidate cache
ibpb : Indirect Branch Prediction Barrier
amd-stibp: Single Thread Indirect Branch Predictor
clwb : Cache Line Write Back and Retain
xsaves : XSAVES, XRSTORS and IA32_XSS support
rdpid: Read Processor ID instruction support
umip : User-Mode Instruction Prevention support
The Reference documents are available at
https://developer.amd.com/wp-content/resources/55803_0.54-PUB.pdf
https://www.amd.com/system/files/TechDocs/24594.pdf
Depends on following kernel commits:
40bc47b08b6e ("kvm: x86: Enumerate support for CLZERO instruction")
504ce1954fba ("KVM: x86: Expose XSAVEERPTR to the guest")
6d61e3c32248 ("kvm: x86: Expose RDPID in KVM_GET_SUPPORTED_CPUID")
52297436199d ("kvm: svm: Update svm_xsaves_supported")
Signed-off-by: Babu Moger
---
target/i386/cpu.c | 102 -
target/i386/cpu.h |2 +
2 files changed, 103 insertions(+), 1 deletion(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 6b7b0f8a4b..70afc3fb30 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -1133,7 +1133,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] =
{
"clzero", NULL, "xsaveerptr", NULL,
NULL, NULL, NULL, NULL,
NULL, "wbnoinvd", NULL, NULL,
-"ibpb", NULL, NULL, NULL,
+"ibpb", NULL, NULL, "amd-stibp",
NULL, NULL, NULL, NULL,
NULL, NULL, NULL, NULL,
"amd-ssbd", "virt-ssbd", "amd-no-ssb", NULL,
@@ -1796,6 +1796,56 @@ static CPUCaches epyc_cache_info = {
},
};
+static CPUCaches epyc_rome_cache_info = {
+.l1d_cache = &(CPUCacheInfo) {
+.type = DATA_CACHE,
+.level = 1,
+.size = 32 * KiB,
+.line_size = 64,
+.associativity = 8,
+.partitions = 1,
+.sets = 64,
+.lines_per_tag = 1,
+.self_init = 1,
+.no_invd_sharing = true,
+},
+.l1i_cache = &(CPUCacheInfo) {
+.type = INSTRUCTION_CACHE,
+.level = 1,
+.size = 32 * KiB,
+.line_size = 64,
+.associativity = 8,
+.partitions = 1,
+.sets = 64,
+.lines_per_tag = 1,
+.self_init = 1,
+.no_invd_sharing = true,
+},
+.l2_cache = &(CPUCacheInfo) {
+.type = UNIFIED_CACHE,
+.level = 2,
+.size = 512 * KiB,
+.line_size = 64,
+.associativity = 8,
+.partitions = 1,
+.sets = 1024,
+.lines_per_tag = 1,
+},
+.l3_cache = &(CPUCacheInfo) {
+.type = UNIFIED_CACHE,
+.level = 3,
+.size = 16 * MiB,
+.line_size = 64,
+.associativity = 16,
+.partitions = 1,
+.sets = 16384,
+.lines_per_tag = 1,
+.self_init = true,
+.inclusive = true,
+.complex_indexing = true,
+},
+};
+
static X86CPUDefinition builtin_x86_defs[] = {
{
.name = "qemu64",
@@ -3204,6 +3254,56 @@ static X86CPUDefinition builtin_x86_defs[] = {
.model_id = "Hygon Dhyana Processor",
.cache_info = _cache_info,
},
+{
+.name = "EPYC-Rome",
+.level = 0xd,
+.vendor = CPUID_VENDOR_AMD,
+.family = 23,
+.model = 49,
+.stepping = 0,
+.features[FEAT_1_EDX] =
+CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX | CPUID_CLFLUSH |
+CPUID_PSE36 | CPUID_PAT | CPUID_CMOV | CPUID_MCA | CPUID_PGE |
+CPUID_MTRR | CPUID_SEP | CPUID_APIC | CPUID_CX8 | CPUID_MCE |
+CPUID_PAE | CPUID_MSR | CPUID_TSC | CPUID_PSE | CPUID_DE |
+CPUID_VME | CPUID_FP87,
+.features[FEAT_1_ECX] =
+CPUID_EXT_RDRAND | CPUID_EXT_F16C | CPUID_EXT_AVX |
+CPUID_EXT_XSAVE | CPUID_EXT_AES | CPUID_EXT_POPCNT |
+CPUID_EXT_MOVBE | CPUID_EXT_SSE42 | CPUID_EXT_SSE41 |
+CPUID_EXT_CX16 | CPUID_EXT_FMA | CPUID_EXT_SSSE3 |
+CPUID_EXT_MONITOR | CPUID_EXT_PCLMULQDQ | CPUID_EXT_SSE3,
+.features[FEAT_8000_0001_EDX] =
+CPUID_EXT2_LM | CPUID_EXT2_RDTSCP | CPUID_EXT2_PDPE1GB |
+CPUID_EXT2_FFXSR | CPUID_EXT2_MMXEXT | CPUID_EXT2_NX |
+CPUID_EXT2_SYSCALL,
+.features[FEAT_8000_0001_ECX] =
+
[PATCH v2 1/2] i386: Add missing cpu feature bits in EPYC model
Adds the following missing CPUID bits:
perfctr-core : core performance counter extensions support. Enables the VM
to use extended performance counter support. It enables six
programmable counters instead of 4 counters.
clzero : instruction zeroes out the 64 byte cache line specified in RAX.
xsaveerptr : XSAVE, XSAVE, FXSAVEOPT, XSAVEC, XSAVES always save error
pointers and FXRSTOR, XRSTOR, XRSTORS always restore error
pointers.
ibpb : Indirect Branch Prediction Barrie.
xsaves : XSAVES, XRSTORS and IA32_XSS supported.
Depends on following kernel commits:
40bc47b08b6e ("kvm: x86: Enumerate support for CLZERO instruction")
504ce1954fba ("KVM: x86: Expose XSAVEERPTR to the guest")
52297436199d ("kvm: svm: Update svm_xsaves_supported")
These new features will be added in EPYC-v3. The -cpu help output after the
change.
x86 EPYC-v1 AMD EPYC Processor
x86 EPYC-v2 AMD EPYC Processor (with IBPB)
x86 EPYC-v3 AMD EPYC Processor
Signed-off-by: Babu Moger
---
target/i386/cpu.c | 17 +
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 07cf562d89..6b7b0f8a4b 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -3116,10 +3116,6 @@ static X86CPUDefinition builtin_x86_defs[] = {
CPUID_7_0_EBX_SMEP | CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_RDSEED |
CPUID_7_0_EBX_ADX | CPUID_7_0_EBX_SMAP | CPUID_7_0_EBX_CLFLUSHOPT |
CPUID_7_0_EBX_SHA_NI,
-/* Missing: XSAVES (not supported by some Linux versions,
- * including v4.1 to v4.12).
- * KVM doesn't yet expose any XSAVES state save component.
- */
.features[FEAT_XSAVE] =
CPUID_XSAVE_XSAVEOPT | CPUID_XSAVE_XSAVEC |
CPUID_XSAVE_XGETBV1,
@@ -3142,6 +3138,19 @@ static X86CPUDefinition builtin_x86_defs[] = {
{ /* end of list */ }
}
},
+{
+.version = 3,
+.props = (PropValue[]) {
+{ "ibpb", "on" },
+{ "perfctr-core", "on" },
+{ "clzero", "on" },
+{ "xsaveerptr", "on" },
+{ "xsaves", "on" },
+{ "model-id",
+ "AMD EPYC Processor" },
+{ /* end of list */ }
+}
+},
{ /* end of list */ }
}
},
Re: [PATCH v8 0/3] RTC support for QEMU RISC-V virt machine
On 11/7/19 5:52 PM, Palmer Dabbelt wrote: On Wed, 06 Nov 2019 03:56:29 PST (-0800), Anup Patel wrote: This series adds RTC device to QEMU RISC-V virt machine. We have selected Goldfish RTC device model for this. It's a pretty simple synthetic device with few MMIO registers and no dependency external clock. The driver for Goldfish RTC is already available in Linux so we just need to enable it in Kconfig for RISCV and also update Linux defconfigs. We have tested this series with Linux-5.4-rc4 plus defconfig changes available in 'goldfish_rtc_v2' branch of: https://github.com/avpatel/linux.git Changes since v7: - Fix broken "stdout-path" in "/chosen" DT node of virt machine Changes since v6: - Rebased on latest QEMU master - Addressed all nit comments from Philippe Mathieu-Daude Changes since v5: - Rebased on latest QEMU master - Added maintainer entry for Goldfish RTC Changes since v4: - Fixed typo in trace event usage - Moved goldfish_rtc.h to correct location Changes since v3: - Address all nit comments from Alistair Changes since v2: - Rebased on RTC code refactoring Changes since v1: - Implemented VMState save/restore callbacks Anup Patel (3): hw: rtc: Add Goldfish RTC device riscv: virt: Use Goldfish RTC device MAINTAINERS: Add maintainer entry for Goldfish RTC MAINTAINERS | 8 + hw/riscv/Kconfig | 1 + hw/riscv/virt.c | 16 ++ hw/rtc/Kconfig | 3 + hw/rtc/Makefile.objs | 1 + hw/rtc/goldfish_rtc.c | 285 ++ hw/rtc/trace-events | 4 + include/hw/riscv/virt.h | 2 + include/hw/rtc/goldfish_rtc.h | 46 ++ 9 files changed, 366 insertions(+) create mode 100644 hw/rtc/goldfish_rtc.c create mode 100644 include/hw/rtc/goldfish_rtc.h Thanks. I've updated the patches on my queue, LMK if there are any more changes! I'm happy with the series, thanks Anup for addressing all the comments. Regards, Phil.
Re: [PATCH for 4.2 v1 1/1] riscv/virt: Increase flash size
On Thu, Nov 7, 2019 at 10:01 AM Peter Maydell wrote: > > On Thu, 7 Nov 2019 at 17:09, Palmer Dabbelt wrote: > > > > On Wed, 06 Nov 2019 16:47:20 PST (-0800), Alistair Francis wrote: > > > Coreboot developers have requested that they have at least 32MB of flash > > > to load binaries. We currently have 32MB of flash, but it is split in > > > two to allow loading two flash binaries. Let's increase the flash size > > > from 32MB to 64MB to ensure we have a single region that is 32MB. > > > > > > No QEMU release has include flash in the RISC-V virt machine, so this > > > isn't a breaking change. > > > > Even if we had, I wouldn't consider it a breaking change because it adds to > > the memory map so existing programs will continue to run fine. > > I have a feeling you may find that some old command lines won't > work any more because they specified a flash contents binary > that was the old 32MB and now it needs to be padded out to 64MB. Yes, that is correct. Everyone using -pflash will need to change the size of their binaries. This was only just merged into QEMU master though and hasn't been in a release so I don't think many people are using it. I only know of two users, one is me and someone from Coreboot who requested the larger size. It doesn't seem like a problem users will see. Alistair > But I haven't tested whether this theory is correct (it will > depend on how the flash contents are specified -- --bios will > be ok, as will loading contents directly as an ELF file or > similar, specifying contents by a -drive option intended to be > consumed by the pflash is the case which likely needs extra padding.) > > thanks > -- PMM
[PATCH v2 0/2] Add support for 2nd generation AMD EPYC processors
The following series adds the support for 2nd generation AMD EPYC Processors on qemu guests. The model display name for 2nd generation will be EPYC-Rome. Also fixes few missed cpu feature bits in 1st generation EPYC models. The Reference documents are available at https://developer.amd.com/wp-content/resources/55803_0.54-PUB.pdf https://www.amd.com/system/files/TechDocs/24594.pdf --- v2: Used the versioned CPU models instead of machine-type-based CPU compatibility (commented by Eduardo). Babu Moger (2): i386: Add missing cpu feature bits in EPYC model i386: Add 2nd Generation AMD EPYC processors target/i386/cpu.c | 119 +++-- target/i386/cpu.h |2 + 2 files changed, 116 insertions(+), 5 deletions(-) --
[Bug 1851664] Re: qemu-system-x86_64: "VFIO_MAP_DMA : -28" error when we attache 6 VF's to guest machine
Presumably w-bits (aw-bits?) implies using intel-iommu, there's a opportunity for the vfio iommu backend to return -ENOSPC (-28) if we exceed the default number of in-flight DMA mappings per container. The default limit is 65535. You can try increasing this by changing the dma_entry_limit module option on the vfio_iommu_type1 module. Note that in a typical vIOMMU config there's a container per device, so the number of VFs attached is possibly not a factor. It is however a lot of DMA mappings for a single device if this is the issue and you'd generally want to boot the guest with iommu=pt in order to have reasonable assigned device performance with a vIOMMU, which would also greatly reduce the number of mappings. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1851664 Title: qemu-system-x86_64: "VFIO_MAP_DMA : -28" error when we attache 6 VF's to guest machine Status in QEMU: Incomplete Bug description: We are trying to attach 6 VF's to the guest machine on 4.1.1 qemu emulator. We are observing "VFIO_MAP_DMA : -28" error. We are using w-bits=48 bits while lunching VM. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1851664/+subscriptions
Re: [PATCH v3 01/22] iotests: s/qocw2/qcow2/
On 11/7/19 10:36 AM, Max Reitz wrote: Probably due to blind copy-pasting, we have several instances of "qocw2" in our iotests. Fix them. Reported-by: Maxim Levitsky Signed-off-by: Max Reitz --- tests/qemu-iotests/060 | 2 +- Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
[Bug 1851664] Re: qemu-system-x86_64: "VFIO_MAP_DMA : -28" error when we attache 6 VF's to guest machine
qemu-system-x86_64 -name guest=fedora24 -machine q35,accel=kvm,kernel-irqchip=split \ -enable-kvm \ -m 4G \ -smp 8,sockets=1,cores=8,threads=1 \ -device intel-iommu,intremap=on,caching-mode=on,aw-bits=48 \ -drive file=,format=raw \ -device ioh3420,id=pcie.1,chassis=1 \ -device virtio-net-pci,bus=pcie.1,disable-legacy=on,disable-modern=off,iommu_platform=on,ats=on,netdev=net0 \ -netdev user,id=net0,hostfwd=tcp::-:22\ -device vfio-pci,host=3f:02.1 \ -device vfio-pci,host=3f:02.2 \ -device vfio-pci,host=3f:02.3 \ -device vfio-pci,host=3f:02.4 \ -device vfio-pci,host=3d:02.4 \ -device vfio-pci,host=3d:02.5 \ -device vfio-pci,host=3d:02.6 \ -nographic -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1851664 Title: qemu-system-x86_64: "VFIO_MAP_DMA : -28" error when we attache 6 VF's to guest machine Status in QEMU: Incomplete Bug description: We are trying to attach 6 VF's to the guest machine on 4.1.1 qemu emulator. We are observing "VFIO_MAP_DMA : -28" error. We are using w-bits=48 bits while lunching VM. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1851664/+subscriptions
[Bug 1851664] Re: qemu-system-x86_64: "VFIO_MAP_DMA : -28" error when we attache 6 VF's to guest machine
Please find the above qemu command to lunch guest machine -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1851664 Title: qemu-system-x86_64: "VFIO_MAP_DMA : -28" error when we attache 6 VF's to guest machine Status in QEMU: Incomplete Bug description: We are trying to attach 6 VF's to the guest machine on 4.1.1 qemu emulator. We are observing "VFIO_MAP_DMA : -28" error. We are using w-bits=48 bits while lunching VM. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1851664/+subscriptions
Re: [PATCH 0/2] Acceptance test: update kernel used on m68k/q800 test
On 11/7/19 6:18 PM, Laurent Vivier wrote: Le 07/11/2019 à 17:38, Cleber Rosa a écrit : - Original Message - From: "Eric Blake" To: "Cleber Rosa" , qemu-devel@nongnu.org Cc: "Peter Maydell" , "Eduardo Habkost" , "Philippe Mathieu-Daudé" , "Wainer dos Santos Moschetta" , "Laurent Vivier" , "Willian Rampazzo" , "Philippe Mathieu-Daudé" Sent: Thursday, November 7, 2019 10:43:08 AM Subject: Re: [PATCH 0/2] Acceptance test: update kernel used on m68k/q800 test On 10/29/19 6:23 PM, Cleber Rosa wrote: The boot_linux_console.py:BootLinuxConsole.test_m68k_q800 was very recently merged, but between its last review and now, the Kernel package used went missing. meta-question: Why was this series posted in-reply-to the pull request, rather than as a new top-level thread? I nearly missed it because I don't expect to see unreviewed patches buried in threading like that. My workflow would have been to post the series in isolation, then manually reply to the pull request to mention the message-id of the related series proposed as a followup. Hi Eric, That was my attempt to signal that it was a fix to something which had *just* being merged as part of that pull request (though now caused by it). I basically did not know how to act properly, so I thank you for the workflow suggestion. I'll certainly follow it next time. IMHO, you should send your series and then replies to the pull request to tell you have sent your series that fixes the patch in the pull request, or vice-versa. But your series has been queued by Alex, so there is no problem... I prepared a different fix around the same time, but closed my laptop before the patch was sent and noticed the next day: https://lists.gnu.org/archive/html/qemu-devel/2019-10/msg08120.html Laurent, are you OK with the new kernel being tested?
Re: [PATCH v1 0/6] testing/next (netbsd stuff)
On Thu, 7 Nov 2019 at 17:54, Kamil Rytarowski wrote: > > On 07.11.2019 18:46, Peter Maydell wrote: > > On Mon, 4 Nov 2019 at 17:39, Alex Bennée wrote: > >> > >> Hi, > >> > >> As we approach hard-freeze I'm trying to temper what comes in through > >> the testing/next tree. However it would be nice to get the NetBSD upto > >> speed with the other NetBSDs. Although the serial install is working > >> well for me this has had a rocky road so if others could also give it > >> a good testing that would be great. I've also disabled one of the > >> regular failing tests for non-Linux targets. There are other tests > >> that still fail however including the tests/test-aio-multithread which > >> asserts in the async utils around about 20% of the time: > >> > >> assertion "QSLIST_EMPTY(>scheduled_coroutines)" failed: file > >> "/home/qemu/qemu-test.nS1czd/src/util/async.c", line 279, function > >> "aio_ctx_finalize" > > > > This is unrelated to your NetBSD update in this series -- it's > > one of the persistent intermittents I see on the BSDs: > > https://lore.kernel.org/qemu-devel/20190916153312.GD25552@stefanha-x1.localdomain/t/ > > > > (though the failure rate I see is I think <20%, but I haven't > > really carefully measured it.) > Does this patch rely on AIO API in the kernel? If so than this is > unreliable as of today on NetBSD. We plan to fix it, but there is no > expected time of accomplishment. No, we use our own AIO implementation which puts fds into non-blocking mode and uses a thread which polls them to identify when they're ready to actually perform IO (plus a lot of coroutine magic). thanks -- PMM
Re: [PATCH v15 02/12] util/cutils: Add qemu_strtotime_ns()
On Thu, Nov 07, 2019 at 03:45:01PM +0800, Tao Xu wrote: > To convert strings with time suffixes to numbers, support time unit are > "ns" for nanosecond, "us" for microsecond, "ms" for millisecond or "s" > for second. Add test for qemu_strtotime_ns, test the input of basic, > time suffixes, float, invaild, trailing and overflow. > > Signed-off-by: Tao Xu Reviewed-by: Eduardo Habkost -- Eduardo
Re: [PATCH v1 0/6] testing/next (netbsd stuff)
On 07.11.2019 18:46, Peter Maydell wrote: > On Mon, 4 Nov 2019 at 17:39, Alex Bennée wrote: >> >> Hi, >> >> As we approach hard-freeze I'm trying to temper what comes in through >> the testing/next tree. However it would be nice to get the NetBSD upto >> speed with the other NetBSDs. Although the serial install is working >> well for me this has had a rocky road so if others could also give it >> a good testing that would be great. I've also disabled one of the >> regular failing tests for non-Linux targets. There are other tests >> that still fail however including the tests/test-aio-multithread which >> asserts in the async utils around about 20% of the time: >> >> assertion "QSLIST_EMPTY(>scheduled_coroutines)" failed: file >> "/home/qemu/qemu-test.nS1czd/src/util/async.c", line 279, function >> "aio_ctx_finalize" > > This is unrelated to your NetBSD update in this series -- it's > one of the persistent intermittents I see on the BSDs: > https://lore.kernel.org/qemu-devel/20190916153312.GD25552@stefanha-x1.localdomain/t/ > > (though the failure rate I see is I think <20%, but I haven't > really carefully measured it.) > > thanks > -- PMM > Does this patch rely on AIO API in the kernel? If so than this is unreliable as of today on NetBSD. We plan to fix it, but there is no expected time of accomplishment. signature.asc Description: OpenPGP digital signature
Re: [PATCH v15 01/12] util/cutils: refactor do_strtosz() to support suffixes list
On Thu, Nov 07, 2019 at 03:45:00PM +0800, Tao Xu wrote: > Add do_strtomul() to convert string according to different suffixes. > > Signed-off-by: Tao Xu Reviewed-by: Eduardo Habkost -- Eduardo
[Bug 1810105] Re: Hint showing volume never disappears, blocking buttons to minimize, maximize and close
I finally found a real computer on which this bug is present. The computer in question is a netbook manufactured by Positivo, the Mobo 5900, which is common on schools. It has a touchscreen with a pen which is compatible with evdev but that doesn't work with libinput. Its touchscreen is: Bus 004 Device 006: ID 22b9:0005 eTurboTouch Technology, Inc. The attached image not only shows the hint from the pulseaudio-plugin, but another one caused by pavucontrol. As a virtual keyboard is normally used because the netbook can be used as a tablet, the hints can make some keys impossible to use. ** Attachment added: "Captura de tela_2019-11-07_14-12-19.png" https://bugs.launchpad.net/ubuntu/+source/xfce4-pulseaudio-plugin/+bug/1810105/+attachment/5303552/+files/Captura%20de%20tela_2019-11-07_14-12-19.png -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1810105 Title: Hint showing volume never disappears, blocking buttons to minimize, maximize and close Status in QEMU: New Status in xfce4-pulseaudio-plugin package in Ubuntu: New Bug description: When hovering the mouse over the volume indicator or changing its volume using the mouse wheel it shows the current volume set as a hint. For example: Volume 100% The problem is that the hint never disappears, not even clicking on it. On some occasions the hint can cover the minimize, maximize and close buttons, causing significant problems on using the desktop environment, as these three buttons won't be usable anymore with the hint over it. Where the hint appears it's no longer possible to interact with the screen. ProblemType: Bug DistroRelease: Ubuntu 18.04 Package: xfce4-pulseaudio-plugin 0.4.1-0ubuntu1 ProcVersionSignature: Ubuntu 4.15.0-43.46-generic 4.15.18 Uname: Linux 4.15.0-43-generic x86_64 ApportVersion: 2.20.9-0ubuntu7.5 Architecture: amd64 CurrentDesktop: XFCE Date: Sun Dec 30 17:09:23 2018 InstallationDate: Installed on 2018-12-30 (0 days ago) InstallationMedia: Xubuntu 18.04.2 LTS "Bionic Beaver" - Beta amd64 (20181230) SourcePackage: xfce4-pulseaudio-plugin UpgradeStatus: No upgrade log present (probably fresh install) To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1810105/+subscriptions
Re: [PATCH v1 0/6] testing/next (netbsd stuff)
On Mon, 4 Nov 2019 at 17:39, Alex Bennée wrote: > > Hi, > > As we approach hard-freeze I'm trying to temper what comes in through > the testing/next tree. However it would be nice to get the NetBSD upto > speed with the other NetBSDs. Although the serial install is working > well for me this has had a rocky road so if others could also give it > a good testing that would be great. I've also disabled one of the > regular failing tests for non-Linux targets. There are other tests > that still fail however including the tests/test-aio-multithread which > asserts in the async utils around about 20% of the time: > > assertion "QSLIST_EMPTY(>scheduled_coroutines)" failed: file > "/home/qemu/qemu-test.nS1czd/src/util/async.c", line 279, function > "aio_ctx_finalize" This is unrelated to your NetBSD update in this series -- it's one of the persistent intermittents I see on the BSDs: https://lore.kernel.org/qemu-devel/20190916153312.GD25552@stefanha-x1.localdomain/t/ (though the failure rate I see is I think <20%, but I haven't really carefully measured it.) thanks -- PMM
Re: [PATCH 2/5] ipmi: Add support to customize OEM functions
On 07/11/2019 18:25, Corey Minyard wrote: > On Thu, Nov 07, 2019 at 06:14:58PM +0100, Cédric Le Goater wrote: > What's the plan for merging this, once it's ready? Is there an IPMI > tree for it to be staged in? If not I could take it through the ppc > tree, but I'd need some Acked-bys in that case. I have an IPMI tree for this. I was assuming it was going in to the PPC tree, but it's not big deal. >>> >>> I'd be more comfortable if the generic ipmi changes went through the >>> ipmi tree. >> >> Here is the patch : >> >> http://patchwork.ozlabs.org/patch/1185187/ > > Ok, I have this in my tree. > > I assume there is nothing like the linux-next tree for qemu, right? no. These IPMI OEM commands are handled by the OPAL firmware only. C.
Re: [PATCH 4/4] Added tests for close and change of logfile.
Alex Bennée writes:
> Robert Foley writes:
>
>> One test ensures that the logfile handle is still valid even if
>> the logfile is changed during logging.
>> The other test validates that the logfile handle remains valid under
>> the logfile lock even if the logfile is closed.
Also this doesn't see to work:
17:24:31 [alex@zen:~/l/q/b/all] review/rcu-logfile|… 2 + ./tests/test-logging
/logging/parse_range: OK
/logging/parse_path: OK
/logging/logfile_write_path: free(): double free detected in tcache 2
fish: “./tests/test-logging” terminated by signal SIGABRT (Abort)
in gdb
Starting program: /home/alex/lsrc/qemu.git/builds/all/tests/test-logging
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
[New Thread 0x76f38700 (LWP 28960)]
/logging/parse_range: OK
/logging/parse_path: OK
/logging/logfile_write_path: free(): double free detected in tcache 2
Thread 1 "test-logging" received signal SIGABRT, Aborted.
__GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
50 ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt
#0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1 0x77587535 in __GI_abort () at abort.c:79
#2 0x775de508 in __libc_message (action=action@entry=do_abort,
fmt=fmt@entry=0x776e928d "%s\n") at ../sysdeps/posix/libc_fatal.c:181
#3 0x775e4c1a in malloc_printerr (str=str@entry=0x776eaf58
"free(): double free detected in tcache 2") at malloc.c:5341
#4 0x775e66fd in _int_free (av=0x77720c40 ,
p=0x555cac40, have_lock=) at malloc.c:4193
#5 0x555614a8 in qemu_set_log_filename (filename=0x555cb110
"/tmp/qemu-test-logging.RO35A0/qemu_test_log_write0.log", errp=0x7fffdef0)
at /home/alex/lsrc/qemu.git/util/log.c:148
#6 0xd8be in test_logfile_write (data=0x555c7370) at
/home/alex/lsrc/qemu.git/tests/test-logging.c:127
#7 0x77cdc15a in test_case_run (tc=0x555c9c60) at
../../../glib/gtestutils.c:2318
#8 g_test_run_suite_internal (suite=suite@entry=0x555c8a40,
path=path@entry=0x0) at ../../../glib/gtestutils.c:2403
#9 0x77cdc014 in g_test_run_suite_internal
(suite=suite@entry=0x555c8a20, path=path@entry=0x0) at
../../../glib/gtestutils.c:2415
#10 0x77cdc412 in g_test_run_suite (suite=0x555c8a20) at
../../../glib/gtestutils.c:2490
#11 0x77cdc431 in g_test_run () at ../../../glib/gtestutils.c:1755
#12 0xce07 in main (argc=, argv=) at
/home/alex/lsrc/qemu.git/tests/test-logging.c:212
>>
>> Signed-off-by: Robert Foley
>> ---
>> tests/test-logging.c | 74
>> 1 file changed, 74 insertions(+)
>>
>> diff --git a/tests/test-logging.c b/tests/test-logging.c
>> index a12585f70a..a3190ff92c 100644
>> --- a/tests/test-logging.c
>> +++ b/tests/test-logging.c
>> @@ -108,6 +108,76 @@ static void test_parse_path(gconstpointer data)
>> error_free_or_abort();
>> }
>>
>> +static void test_logfile_write(gconstpointer data)
>> +{
>> +QemuLogFile *logfile;
>> +gchar const *dir = data;
>> +Error *err = NULL;
>> +gchar *file_path;
>> +gchar *file_path1;
>
> with g_autofree char *file_path you can avoid the free down bellow.
>
>> +FILE *orig_fd;
>> +
>> +file_path = g_build_filename(dir, "qemu_test_log_write0.log", NULL);
>> +file_path1 = g_build_filename(dir, "qemu_test_log_write1.log", NULL);
>> +
>> +/*
>> + * Test that even if an open file handle is changed,
>> + * our handle remains valid due to RCU.
>> + */
>> +qemu_set_log_filename(file_path, );
>> +g_assert(!err);
>> +rcu_read_lock();
>> +logfile = atomic_rcu_read(_logfile);
>> +orig_fd = logfile->fd;
>> +g_assert(logfile && logfile->fd);
>> +fprintf(logfile->fd, "%s 1st write to file\n", __func__);
>> +fflush(logfile->fd);
>> +
>> +/* Change the logfile and ensure that the handle is still valid. */
>> +qemu_set_log_filename(file_path1, );
>> +g_assert(!err);
>
> Maybe better would be:
>
> logfile2 = atomic_rcu_read(_logfile);
> g_assert(logfile->fd == orig_fd);
> g_assert(logfile2->fd != logfile->fd);
> fprintf(logfile2->fd, "%s 2nd write to file\n", __func__);
> fflush(logfile2->fd);
>
>
>> +g_assert(logfile->fd == orig_fd);
>> +fprintf(logfile->fd, "%s 2nd write to file\n", __func__);
>> +fflush(logfile->fd);
>> +rcu_read_unlock();
>> +
>> +g_free(file_path);
>> +g_free(file_path1);
>> +}
>> +
>> +static void test_logfile_lock(gconstpointer data)
>> +{
>> +FILE *logfile;
>> +gchar const *dir = data;
>> +Error *err = NULL;
>> +gchar *file_path;
>
> g_autofree
>
>> +
>> +file_path = g_build_filename(dir, "qemu_test_logfile_lock0.log", NULL);
>> +
>> +/*
>> + * Test the use of the logfile lock, such
>> + * that even if an open file handle is closed,
>> + * our
Re: [PATCH 2/5] ipmi: Add support to customize OEM functions
On Thu, Nov 07, 2019 at 06:14:58PM +0100, Cédric Le Goater wrote: > >>> What's the plan for merging this, once it's ready? Is there an IPMI > >>> tree for it to be staged in? If not I could take it through the ppc > >>> tree, but I'd need some Acked-bys in that case. > >> > >> I have an IPMI tree for this. I was assuming it was going in to the PPC > >> tree, but it's not big deal. > > > > I'd be more comfortable if the generic ipmi changes went through the > > ipmi tree. > > Here is the patch : > > http://patchwork.ozlabs.org/patch/1185187/ Ok, I have this in my tree. I assume there is nothing like the linux-next tree for qemu, right? -corey > > > > Note that I've moved the initial ppc specific patch from > > my ppc-for-4.2 tree to my ppc-for-4.3 tree, since it missed my > > previous pull request and it's not really post-freeze material. > > OK. I was wondering where it had gone. > > Thanks, > > C.
Re: [PATCH 0/2] Acceptance test: update kernel used on m68k/q800 test
Le 07/11/2019 à 17:38, Cleber Rosa a écrit : > > > - Original Message - >> From: "Eric Blake" >> To: "Cleber Rosa" , qemu-devel@nongnu.org >> Cc: "Peter Maydell" , "Eduardo Habkost" >> , "Philippe Mathieu-Daudé" >> , "Wainer dos Santos Moschetta" , >> "Laurent Vivier" , >> "Willian Rampazzo" , "Philippe Mathieu-Daudé" >> >> Sent: Thursday, November 7, 2019 10:43:08 AM >> Subject: Re: [PATCH 0/2] Acceptance test: update kernel used on m68k/q800 >> test >> >> On 10/29/19 6:23 PM, Cleber Rosa wrote: >>> The boot_linux_console.py:BootLinuxConsole.test_m68k_q800 was very >>> recently merged, but between its last review and now, the Kernel >>> package used went missing. >>> >> >> meta-question: Why was this series posted in-reply-to the pull request, >> rather than as a new top-level thread? I nearly missed it because I >> don't expect to see unreviewed patches buried in threading like that. >> My workflow would have been to post the series in isolation, then >> manually reply to the pull request to mention the message-id of the >> related series proposed as a followup. >> > > Hi Eric, > > That was my attempt to signal that it was a fix to something which had *just* > being merged as part of that pull request (though now caused by it). > > I basically did not know how to act properly, so I thank you for the workflow > suggestion. I'll certainly follow it next time. IMHO, you should send your series and then replies to the pull request to tell you have sent your series that fixes the patch in the pull request, or vice-versa. But your series has been queued by Alex, so there is no problem... Thanks, Laurent
Re: [PATCH 2/5] ipmi: Add support to customize OEM functions
>>> What's the plan for merging this, once it's ready? Is there an IPMI >>> tree for it to be staged in? If not I could take it through the ppc >>> tree, but I'd need some Acked-bys in that case. >> >> I have an IPMI tree for this. I was assuming it was going in to the PPC >> tree, but it's not big deal. > > I'd be more comfortable if the generic ipmi changes went through the > ipmi tree. Here is the patch : http://patchwork.ozlabs.org/patch/1185187/ > Note that I've moved the initial ppc specific patch from > my ppc-for-4.2 tree to my ppc-for-4.3 tree, since it missed my > previous pull request and it's not really post-freeze material. OK. I was wondering where it had gone. Thanks, C.
Re: [PULL v3 0/3] Trivial branch patches
On Wed, 6 Nov 2019 at 16:26, Laurent Vivier wrote: > > The following changes since commit 36609b4fa36f0ac934874371874416f7533a5408: > > Merge remote-tracking branch > 'remotes/palmer/tags/palmer-for-master-4.2-sf1' into staging (2019-11-02 > 17:59:03 +) > > are available in the Git repository at: > > git://github.com/vivier/qemu.git tags/trivial-branch-pull-request > > for you to fetch changes up to df59feb197cda31a8b807c13bf509259db9e018f: > > global: Squash 'the the' (2019-11-06 17:19:40 +0100) > > > Trivial fixes (20191105-v3) > > v3: remove disas/libvixl/vixl/invalset.h changes > v2: remove patch from Greg that has lines with more than 80 columns > > > > Dr. David Alan Gilbert (1): > global: Squash 'the the' > > Philippe Mathieu-Daudé (2): > hw/misc/grlib_ahb_apb_pnp: Avoid crash when writing to PnP registers > hw/misc/grlib_ahb_apb_pnp: Fix 8-bit accesses > Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/4.2 for any user-visible changes. -- PMM
Re: [PATCH for 4.2 v1 1/1] riscv/virt: Increase flash size
On Wed, 06 Nov 2019 16:47:20 PST (-0800), Alistair Francis wrote:
Coreboot developers have requested that they have at least 32MB of flash
to load binaries. We currently have 32MB of flash, but it is split in
two to allow loading two flash binaries. Let's increase the flash size
from 32MB to 64MB to ensure we have a single region that is 32MB.
No QEMU release has include flash in the RISC-V virt machine, so this
isn't a breaking change.
Even if we had, I wouldn't consider it a breaking change because it adds to
the memory map so existing programs will continue to run fine.
Signed-off-by: Alistair Francis
---
hw/riscv/virt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index cc8f311e6b..23f340df19 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -62,7 +62,7 @@ static const struct MemmapEntry {
[VIRT_PLIC] ={ 0xc00, 0x400 },
[VIRT_UART0] = { 0x1000, 0x100 },
[VIRT_VIRTIO] = { 0x10001000,0x1000 },
-[VIRT_FLASH] = { 0x2000, 0x200 },
+[VIRT_FLASH] = { 0x2000, 0x400 },
[VIRT_DRAM] ={ 0x8000, 0x0 },
[VIRT_PCIE_MMIO] = { 0x4000,0x4000 },
[VIRT_PCIE_PIO] ={ 0x0300,0x0001 },
Reviewed-by: Palmer Dabbelt
I'll include this in my next PR, which should be soon -- I was about to send
it, but figure I should look at my email first :)
Re: [PATCH 2/5] ipmi: Add support to customize OEM functions
On Sun, Oct 27, 2019 at 01:33:47PM -0500, Corey Minyard wrote:
> On Sun, Oct 27, 2019 at 06:47:39PM +0100, David Gibson wrote:
> > On Mon, Oct 21, 2019 at 09:30:17AM -0500, Corey Minyard wrote:
> > > On Mon, Oct 21, 2019 at 03:12:12PM +0200, Cédric Le Goater wrote:
> > > > The routine ipmi_register_oem_netfn() lets external modules register
> > > > command handlers for OEM functions. Required for the PowerNV machine.
> > >
> > > Comments inline.
> > >
> > > >
> > > > Cc: Corey Minyard
> > > > Signed-off-by: Cédric Le Goater
> > > > ---
> > > > include/hw/ipmi/ipmi.h | 36
> > > > hw/ipmi/ipmi_bmc_sim.c | 41 ++---
> > > > 2 files changed, 42 insertions(+), 35 deletions(-)
> > > >
> > > > diff --git a/include/hw/ipmi/ipmi.h b/include/hw/ipmi/ipmi.h
> > > > index 6f2413b39b4a..cb7203b06767 100644
> > > > --- a/include/hw/ipmi/ipmi.h
> > > > +++ b/include/hw/ipmi/ipmi.h
> > > > @@ -265,4 +265,40 @@ int ipmi_bmc_sdr_find(IPMIBmc *b, uint16_t recid,
> > > >const struct ipmi_sdr_compact **sdr, uint16_t
> > > > *nextrec);
> > > > void ipmi_bmc_gen_event(IPMIBmc *b, uint8_t *evt, bool log);
> > > >
> > > > +typedef struct IPMIBmcSim IPMIBmcSim;
> > >
> > > This type isn't very useful outside of the simulator, but changes for
> > > that can come as they are needed. I don't see an easy way to avoid
> > > putting it here.
> > >
> > > > +
> > > > +typedef struct RspBuffer {
> > > > +uint8_t buffer[MAX_IPMI_MSG_SIZE];
> > > > +unsigned int len;
> > > > +} RspBuffer;
> > > > +
> > > > +static inline void rsp_buffer_set_error(RspBuffer *rsp, uint8_t byte)
> > > > +{
> > > > +rsp->buffer[2] = byte;
> > > > +}
> > > > +
> > > > +/* Add a byte to the response. */
> > > > +static inline void rsp_buffer_push(RspBuffer *rsp, uint8_t byte)
> > > > +{
> > > > +if (rsp->len >= sizeof(rsp->buffer)) {
> > > > +rsp_buffer_set_error(rsp, IPMI_CC_REQUEST_DATA_TRUNCATED);
> > > > +return;
> > > > +}
> > > > +rsp->buffer[rsp->len++] = byte;
> > > > +}
> > > > +
> > > > +typedef struct IPMICmdHandler {
> > > > +void (*cmd_handler)(IPMIBmcSim *s,
> > > > +uint8_t *cmd, unsigned int cmd_len,
> > > > +RspBuffer *rsp);
> > > > +unsigned int cmd_len_min;
> > > > +} IPMICmdHandler;
> > > > +
> > > > +typedef struct IPMINetfn {
> > > > +unsigned int cmd_nums;
> > > > +const IPMICmdHandler *cmd_handlers;
> > > > +} IPMINetfn;
> > > > +
> > > > +int ipmi_register_oem_netfn(IPMIBmc *b, const IPMINetfn *netfnd);
> > > > +
> > > > #endif
> > > > diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
> > > > index 71e56f3b13d1..770aace55b08 100644
> > > > --- a/hw/ipmi/ipmi_bmc_sim.c
> > > > +++ b/hw/ipmi/ipmi_bmc_sim.c
> > > > @@ -98,6 +98,7 @@
> > > > #define IPMI_CMD_GET_SEL_TIME 0x48
> > > > #define IPMI_CMD_SET_SEL_TIME 0x49
> > > >
> > > > +#define IPMI_NETFN_OEM0x3a
> > > >
> > > > /* Same as a timespec struct. */
> > > > struct ipmi_time {
> > > > @@ -167,23 +168,8 @@ typedef struct IPMISensor {
> > > > #define MAX_SENSORS 20
> > > > #define IPMI_WATCHDOG_SENSOR 0
> > > >
> > > > -typedef struct IPMIBmcSim IPMIBmcSim;
> > > > -typedef struct RspBuffer RspBuffer;
> > > > -
> > > > #define MAX_NETFNS 64
> > > >
> > > > -typedef struct IPMICmdHandler {
> > > > -void (*cmd_handler)(IPMIBmcSim *s,
> > > > -uint8_t *cmd, unsigned int cmd_len,
> > > > -RspBuffer *rsp);
> > > > -unsigned int cmd_len_min;
> > > > -} IPMICmdHandler;
> > > > -
> > > > -typedef struct IPMINetfn {
> > > > -unsigned int cmd_nums;
> > > > -const IPMICmdHandler *cmd_handlers;
> > > > -} IPMINetfn;
> > > > -
> > > > typedef struct IPMIRcvBufEntry {
> > > > QTAILQ_ENTRY(IPMIRcvBufEntry) entry;
> > > > uint8_t len;
> > > > @@ -279,28 +265,8 @@ struct IPMIBmcSim {
> > > > #define IPMI_BMC_WATCHDOG_ACTION_POWER_DOWN 2
> > > > #define IPMI_BMC_WATCHDOG_ACTION_POWER_CYCLE 3
> > > >
> > > > -struct RspBuffer {
> > > > -uint8_t buffer[MAX_IPMI_MSG_SIZE];
> > > > -unsigned int len;
> > > > -};
> > > > -
> > > > #define RSP_BUFFER_INITIALIZER { }
> > > >
> > > > -static inline void rsp_buffer_set_error(RspBuffer *rsp, uint8_t byte)
> > > > -{
> > > > -rsp->buffer[2] = byte;
> > > > -}
> > > > -
> > > > -/* Add a byte to the response. */
> > > > -static inline void rsp_buffer_push(RspBuffer *rsp, uint8_t byte)
> > > > -{
> > > > -if (rsp->len >= sizeof(rsp->buffer)) {
> > > > -rsp_buffer_set_error(rsp, IPMI_CC_REQUEST_DATA_TRUNCATED);
> > > > -return;
> > > > -}
> > > > -rsp->buffer[rsp->len++] = byte;
> > > > -}
> > > > -
> > > > static inline void rsp_buffer_pushmore(RspBuffer *rsp, uint8_t *bytes,
> > > > unsigned int n)
> > > > {
> > >
Re: [PATCH 1/4] Add a mutex to guarantee single writer to qemu_logfile handle.
Robert Foley writes:
> This is being added in preparation for using RCU with the logfile handle.
> Also added qemu_logfile_init() for initializing the logfile mutex.
>
> Signed-off-by: Robert Foley
> ---
> util/log.c | 23 +++
> 1 file changed, 23 insertions(+)
>
> diff --git a/util/log.c b/util/log.c
> index 1ca13059ee..dff2f98c8c 100644
> --- a/util/log.c
> +++ b/util/log.c
> @@ -24,8 +24,11 @@
> #include "qapi/error.h"
> #include "qemu/cutils.h"
> #include "trace/control.h"
> +#include "qemu/thread.h"
>
> static char *logfilename;
> +static bool qemu_logfile_initialized;
> +static QemuMutex qemu_logfile_mutex;
> FILE *qemu_logfile;
> int qemu_loglevel;
> static int log_append = 0;
> @@ -49,6 +52,14 @@ int qemu_log(const char *fmt, ...)
> return ret;
> }
>
> +static void qemu_logfile_init(void)
> +{
> +if (!qemu_logfile_initialized) {
> +qemu_mutex_init(_logfile_mutex);
> +qemu_logfile_initialized = true;
> +}
> +}
> +
> static bool log_uses_own_buffers;
>
> /* enable or disable low levels log */
> @@ -58,6 +69,12 @@ void qemu_set_log(int log_flags)
> #ifdef CONFIG_TRACE_LOG
> qemu_loglevel |= LOG_TRACE;
> #endif
> +
> +/* Is there a better place to call this to init the logfile subsystem? */
> +if (!qemu_logfile_initialized) {
> +qemu_logfile_init();
> +}
It wouldn't be the worst thing in the world to expose:
qemu_logfile_init()
and make vl.c and main.c call it before the setup. Then you can drop the
flag or even just g_assert(qemu_log_mutex_initialised) in qemu_set_log
and qemu_set_logfile.
In fact you could just use:
static void __attribute__((__constructor__)) qemu_logfile_init(void)
and make the compiler do it for you.
> +qemu_mutex_lock(_logfile_mutex);
> if (!qemu_logfile &&
> (is_daemonized() ? logfilename != NULL : qemu_loglevel)) {
> if (logfilename) {
> @@ -93,6 +110,7 @@ void qemu_set_log(int log_flags)
> log_append = 1;
> }
> }
> +qemu_mutex_unlock(_logfile_mutex);
> if (qemu_logfile &&
> (is_daemonized() ? logfilename == NULL : !qemu_loglevel)) {
> qemu_log_close();
> @@ -114,6 +132,11 @@ void qemu_set_log_filename(const char *filename, Error
> **errp)
> char *pidstr;
> g_free(logfilename);
>
> +/* Is there a better place to call this to init the logfile subsystem? */
> +if (!qemu_logfile_initialized) {
> +qemu_logfile_init();
> +}
> +
> pidstr = strstr(filename, "%");
> if (pidstr) {
> /* We only accept one %d, no other format strings */
--
Alex Bennée
Re: [PATCH v2 2/4] Memory: Enable writeback for given memory region
On Thu, 7 Nov 2019 at 16:57, Alex Bennée wrote: > > > Beata Michalska writes: > > > On Wed, 6 Nov 2019 at 12:20, Richard Henderson > > wrote: > >> qemu_log_mask w/ GUEST_ERROR? How do we expect the length to overflow? > > > > In theory it shouldn't, at least with current usage. > > I guess the probe_access will make sure of that. > > This was more of a precaution to enable catching potential/future misuses > > aka debugging purpose. I can get rid of that it that's playing too > > safe. > > If the internal code might get it wrong and that would be a bug then the > g_assert(), if the values are ultimately from the guest then log with > GUEST_ERROR as Richard suggests. ...or consider asserting at this level and making the target specific calling code sanitize and do the GUEST_ERROR logging (it can do a better job of it because it knows what the target-specific operation that the guest just got wrong was). thanks -- PMM
Re: [PATCH] ppc/pnv: Add a "/qemu" device tree node
On Wed, Nov 06, 2019 at 03:21:29PM +0100, Cédric Le Goater wrote: > It helps skiboot identifying that is running on a QEMU platform. The > compatible string will define the POWERPC processor version. > > Signed-off-by: Cédric Le Goater Applied to ppc-for-4.3. > --- > hw/ppc/pnv.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c > index 627c08e5b985..4c3d5184126a 100644 > --- a/hw/ppc/pnv.c > +++ b/hw/ppc/pnv.c > @@ -479,6 +479,9 @@ static void *pnv_dt_create(MachineState *machine) > fdt = g_malloc0(FDT_MAX_SIZE); > _FDT((fdt_create_empty_tree(fdt, FDT_MAX_SIZE))); > > +/* /qemu node */ > +_FDT((fdt_add_subnode(fdt, 0, "qemu"))); > + > /* Root node */ > _FDT((fdt_setprop_cell(fdt, 0, "#address-cells", 0x2))); > _FDT((fdt_setprop_cell(fdt, 0, "#size-cells", 0x2))); -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
[Bug 1851664] Re: qemu-system-x86_64: "VFIO_MAP_DMA : -28" error when we attache 6 VF's to guest machine
Please provide information how you started QEMU, and some information about your PCI device (e.g. the output of lspci). ** Information type changed from Private Security to Public ** Changed in: qemu Status: New => Incomplete -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1851664 Title: qemu-system-x86_64: "VFIO_MAP_DMA : -28" error when we attache 6 VF's to guest machine Status in QEMU: Incomplete Bug description: We are trying to attach 6 VF's to the guest machine on 4.1.1 qemu emulator. We are observing "VFIO_MAP_DMA : -28" error. We are using w-bits=48 bits while lunching VM. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1851664/+subscriptions
Re: [PATCH v2 2/4] Memory: Enable writeback for given memory region
Beata Michalska writes:
> On Wed, 6 Nov 2019 at 12:20, Richard Henderson
> wrote:
>>
>> On 11/6/19 12:40 AM, Beata Michalska wrote:
>> > +void qemu_ram_writeback(RAMBlock *block, ram_addr_t start, ram_addr_t
>> > length)
>> > +{
>> > +void *addr = ramblock_ptr(block, start);
>> > +
>> > +/*
>> > + * The requested range might spread up to the very end of the block
>> > + */
>> > +if ((start + length) > block->used_length) {
>> > +qemu_log("%s: sync range outside the block boundaries: "
>> > + "start: " RAM_ADDR_FMT " length: " RAM_ADDR_FMT
>> > + " block length: " RAM_ADDR_FMT " Narrowing down ..."
>> > ,
>> > + __func__, start, length, block->used_length);
>> > +length = block->used_length - start;
>> > +}
>>
>> qemu_log_mask w/ GUEST_ERROR? How do we expect the length to overflow?
>
> In theory it shouldn't, at least with current usage.
> I guess the probe_access will make sure of that.
> This was more of a precaution to enable catching potential/future misuses
> aka debugging purpose. I can get rid of that it that's playing too
> safe.
If the internal code might get it wrong and that would be a bug then the
g_assert(), if the values are ultimately from the guest then log with
GUEST_ERROR as Richard suggests.
--
Alex Bennée
Re: [PATCH v3 21/22] iotests: Disable data_file where it cannot be used
On 07.11.19 17:37, Max Reitz wrote: > Signed-off-by: Max Reitz > --- > tests/qemu-iotests/007 | 5 +++-- > tests/qemu-iotests/014 | 2 ++ > tests/qemu-iotests/015 | 5 +++-- > tests/qemu-iotests/026 | 5 - > tests/qemu-iotests/029 | 5 +++-- > tests/qemu-iotests/031 | 6 +++--- > tests/qemu-iotests/036 | 5 +++-- > tests/qemu-iotests/039 | 3 +++ > tests/qemu-iotests/046 | 2 ++ > tests/qemu-iotests/048 | 2 ++ > tests/qemu-iotests/051 | 5 +++-- > tests/qemu-iotests/058 | 5 +++-- > tests/qemu-iotests/060 | 6 -- > tests/qemu-iotests/061 | 6 -- > tests/qemu-iotests/062 | 2 +- > tests/qemu-iotests/066 | 4 +++- > tests/qemu-iotests/067 | 6 -- > tests/qemu-iotests/068 | 5 +++-- > tests/qemu-iotests/071 | 3 +++ > tests/qemu-iotests/073 | 4 > tests/qemu-iotests/074 | 2 ++ > tests/qemu-iotests/080 | 5 +++-- > tests/qemu-iotests/090 | 2 ++ > tests/qemu-iotests/098 | 6 -- > tests/qemu-iotests/099 | 3 ++- > tests/qemu-iotests/103 | 5 +++-- > tests/qemu-iotests/108 | 6 -- > tests/qemu-iotests/112 | 5 +++-- > tests/qemu-iotests/114 | 2 ++ > tests/qemu-iotests/121 | 3 +++ > tests/qemu-iotests/138 | 3 +++ > tests/qemu-iotests/156 | 2 ++ > tests/qemu-iotests/176 | 7 +-- > tests/qemu-iotests/191 | 2 ++ > tests/qemu-iotests/201 | 6 +++--- > tests/qemu-iotests/214 | 3 ++- > tests/qemu-iotests/217 | 3 ++- > tests/qemu-iotests/220 | 5 +++-- > tests/qemu-iotests/243 | 6 -- > tests/qemu-iotests/244 | 5 +++-- > tests/qemu-iotests/250 | 2 ++ > tests/qemu-iotests/261 | 3 ++- > tests/qemu-iotests/267 | 5 +++-- > 43 files changed, 124 insertions(+), 53 deletions(-) [...] > diff --git a/tests/qemu-iotests/067 b/tests/qemu-iotests/067 > index 926c79b37c..3bc6e719eb 100755 > --- a/tests/qemu-iotests/067 > +++ b/tests/qemu-iotests/067 > @@ -32,8 +32,10 @@ status=1 # failure is the default! > > _supported_fmt qcow2 > _supported_proto file > -# Because anything other than 16 would change the output of query-block > -_unsupported_imgopts 'refcount_bits=\([^1]\|.\([^6]\|$\)\)' > +# Because anything other than 16 would change the output of query-block, > +# and external data files would change the output of > +# query-named-block-ndoes s/ndoes/nodes/... signature.asc Description: OpenPGP digital signature
[PATCH v3 18/22] iotests: Make 110 work with data_file
The only difference is that the json:{} filename of the image looks
different. We actually do not care about that filename in this test, we
are only interested in (1) that there is a json:{} filename, and (2)
whether the backing filename can be constructed.
So just filter out the json:{} data, thus making this test pass both
with and without data_file.
Signed-off-by: Max Reitz
Reviewed-by: Maxim Levitsky
---
tests/qemu-iotests/110 | 7 +--
tests/qemu-iotests/110.out | 4 ++--
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/tests/qemu-iotests/110 b/tests/qemu-iotests/110
index f78df0e6e1..139c02c2cf 100755
--- a/tests/qemu-iotests/110
+++ b/tests/qemu-iotests/110
@@ -67,6 +67,7 @@ echo
# Across blkdebug without a config file, you cannot reconstruct filenames, so
# qemu is incapable of knowing the directory of the top image from the filename
# alone. However, using bdrv_dirname(), it should still work.
+# (Filter out the json:{} filename so this test works with external data files)
TEST_IMG="json:{
'driver': '$IMGFMT',
'file': {
@@ -82,7 +83,8 @@ TEST_IMG="json:{
}
]
}
-}" _img_info | _filter_img_info | grep -v 'backing file format'
+}" _img_info | _filter_img_info | grep -v 'backing file format' \
+| _filter_json_filename
echo
echo '=== Backing name is always relative to the backed image ==='
@@ -114,7 +116,8 @@ TEST_IMG="json:{
}
]
}
-}" _img_info | _filter_img_info | grep -v 'backing file format'
+}" _img_info | _filter_img_info | grep -v 'backing file format' \
+| _filter_json_filename
# success, all done
diff --git a/tests/qemu-iotests/110.out b/tests/qemu-iotests/110.out
index f60b26390e..f835553a99 100644
--- a/tests/qemu-iotests/110.out
+++ b/tests/qemu-iotests/110.out
@@ -11,7 +11,7 @@ backing file: t.IMGFMT.base (actual path:
TEST_DIR/t.IMGFMT.base)
=== Non-reconstructable filename ===
-image: json:{"driver": "IMGFMT", "file": {"set-state.0.event": "read_aio",
"image": {"driver": "file", "filename": "TEST_DIR/t.IMGFMT"}, "driver":
"blkdebug", "set-state.0.new_state": 42}}
+image: json:{ /* filtered */ }
file format: IMGFMT
virtual size: 64 MiB (67108864 bytes)
backing file: t.IMGFMT.base (actual path: TEST_DIR/t.IMGFMT.base)
@@ -22,7 +22,7 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
backing_file=t.IMGFMT.b
=== Nodes without a common directory ===
-image: json:{"driver": "IMGFMT", "file": {"children": [{"driver": "file",
"filename": "TEST_DIR/t.IMGFMT"}, {"driver": "file", "filename":
"TEST_DIR/t.IMGFMT.copy"}], "driver": "quorum", "vote-threshold": 1}}
+image: json:{ /* filtered */ }
file format: IMGFMT
virtual size: 64 MiB (67108864 bytes)
backing file: t.IMGFMT.base (cannot determine actual path)
--
2.23.0
Re: [PATCH v2 20/21] iotests: Disable data_file where it cannot be used
On 07.11.19 16:19, Maxim Levitsky wrote: > On Thu, 2019-11-07 at 12:36 +0100, Max Reitz wrote: >> On 06.11.19 16:52, Maxim Levitsky wrote: >>> On Tue, 2019-10-15 at 16:27 +0200, Max Reitz wrote: Signed-off-by: Max Reitz --- tests/qemu-iotests/007 | 5 +++-- tests/qemu-iotests/014 | 2 ++ tests/qemu-iotests/015 | 5 +++-- tests/qemu-iotests/026 | 5 - tests/qemu-iotests/029 | 5 +++-- tests/qemu-iotests/031 | 6 +++--- tests/qemu-iotests/036 | 5 +++-- tests/qemu-iotests/039 | 3 +++ tests/qemu-iotests/046 | 2 ++ tests/qemu-iotests/048 | 2 ++ tests/qemu-iotests/051 | 5 +++-- tests/qemu-iotests/058 | 5 +++-- tests/qemu-iotests/060 | 6 -- tests/qemu-iotests/061 | 6 -- tests/qemu-iotests/062 | 2 +- tests/qemu-iotests/066 | 2 +- tests/qemu-iotests/067 | 6 -- tests/qemu-iotests/068 | 5 +++-- tests/qemu-iotests/071 | 3 +++ tests/qemu-iotests/073 | 2 ++ tests/qemu-iotests/074 | 2 ++ tests/qemu-iotests/080 | 5 +++-- tests/qemu-iotests/090 | 2 ++ tests/qemu-iotests/098 | 6 -- tests/qemu-iotests/099 | 3 ++- tests/qemu-iotests/103 | 5 +++-- tests/qemu-iotests/108 | 6 -- tests/qemu-iotests/112 | 5 +++-- tests/qemu-iotests/114 | 2 ++ tests/qemu-iotests/121 | 3 +++ tests/qemu-iotests/138 | 2 ++ tests/qemu-iotests/156 | 2 ++ tests/qemu-iotests/176 | 7 +-- tests/qemu-iotests/191 | 2 ++ tests/qemu-iotests/201 | 6 +++--- tests/qemu-iotests/214 | 3 ++- tests/qemu-iotests/217 | 3 ++- tests/qemu-iotests/220 | 5 +++-- tests/qemu-iotests/243 | 6 -- tests/qemu-iotests/244 | 5 +++-- tests/qemu-iotests/250 | 2 ++ tests/qemu-iotests/267 | 5 +++-- 42 files changed, 117 insertions(+), 52 deletions(-) >> >> [...] >> diff --git a/tests/qemu-iotests/031 b/tests/qemu-iotests/031 index c44fcf91bb..646ecd593f 100755 --- a/tests/qemu-iotests/031 +++ b/tests/qemu-iotests/031 @@ -40,9 +40,9 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 # This tests qcow2-specific low-level functionality _supported_fmt qcow2 _supported_proto file -# We want to test compat=0.10, which does not support refcount widths -# other than 16 -_unsupported_imgopts 'refcount_bits=\([^1]\|.\([^6]\|$\)\)' +# We want to test compat=0.10, which does not support external data +# files or refcount widths other than 16 +_unsupported_imgopts data_file 'refcount_bits=\([^1]\|.\([^6]\|$\)\)' >>> >>> This is maybe another reason to split this test for compat=0.10 and for >>> compat=1.1 >>> But still can be done later of course. >> >> Hm, but I don’t really think this test is important for external data >> files. There is no I/O. > I guess both yes and no, the external data file is a header extension as well. Yes, but the test already involves a header extension. diff --git a/tests/qemu-iotests/036 b/tests/qemu-iotests/036 index bbaf0ef45b..512598421c 100755 --- a/tests/qemu-iotests/036 +++ b/tests/qemu-iotests/036 @@ -43,8 +43,9 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 # This tests qcow2-specific low-level functionality _supported_fmt qcow2 _supported_proto file -# Only qcow2v3 and later supports feature bits -_unsupported_imgopts 'compat=0.10' +# Only qcow2v3 and later supports feature bits; +# qcow2.py does not support external data files >>> >>> Minor nitpick, maybe tag this with TODO or so. No need to do now. >> >> Hm, well, and the same applies here. (Just not a very important test.) > Same here, in theory external data file is a feature, and it could > 'interact' with other features, but most likely you are right here as well. Well, but the test currently doesn’t involve any known feature bits. It’s mostly about checking what our qcow2 driver does with unknown feature bits. (If it wanted to involve known feature bits, it could have easily used e.g. the dirty feature.) diff --git a/tests/qemu-iotests/048 b/tests/qemu-iotests/048 index a8feb76184..2af6b74b41 100755 --- a/tests/qemu-iotests/048 +++ b/tests/qemu-iotests/048 @@ -49,6 +49,8 @@ _compare() _supported_fmt raw qcow2 qed luks _supported_proto file _supported_os Linux +# Using 'cp' is incompatible with external data files +_unsupported_imgopts data_file >>> >>> You could compare the external files instead in theory *I think*. >>> Another item on some TODO list I guess. >> >> This is a test of qemu-img compare, not of the image format. So it >> doesn’t make much sense to me to compare the external files, and also it >> should be completely sufficient to run this test only without external >> data files. > Yes but on the other hand, its is kind of nice to test that it can compare > correctly > two qcow2 files which have external data
Re: [PATCH 0/2] Acceptance test: update kernel used on m68k/q800 test
- Original Message - > From: "Eric Blake" > To: "Cleber Rosa" , qemu-devel@nongnu.org > Cc: "Peter Maydell" , "Eduardo Habkost" > , "Philippe Mathieu-Daudé" > , "Wainer dos Santos Moschetta" , > "Laurent Vivier" , > "Willian Rampazzo" , "Philippe Mathieu-Daudé" > > Sent: Thursday, November 7, 2019 10:43:08 AM > Subject: Re: [PATCH 0/2] Acceptance test: update kernel used on m68k/q800 test > > On 10/29/19 6:23 PM, Cleber Rosa wrote: > > The boot_linux_console.py:BootLinuxConsole.test_m68k_q800 was very > > recently merged, but between its last review and now, the Kernel > > package used went missing. > > > > meta-question: Why was this series posted in-reply-to the pull request, > rather than as a new top-level thread? I nearly missed it because I > don't expect to see unreviewed patches buried in threading like that. > My workflow would have been to post the series in isolation, then > manually reply to the pull request to mention the message-id of the > related series proposed as a followup. > Hi Eric, That was my attempt to signal that it was a fix to something which had *just* being merged as part of that pull request (though now caused by it). I basically did not know how to act properly, so I thank you for the workflow suggestion. I'll certainly follow it next time. Thanks a lot! - Cleber. > -- > Eric Blake, Principal Software Engineer > Red Hat, Inc. +1-919-301-3226 > Virtualization: qemu.org | libvirt.org >
[PATCH v3 22/22] iotests: Allow check -o data_file
The problem with allowing the data_file option is that you want to use a
different data file per image used in the test. Therefore, we need to
allow patterns like -o data_file='$TEST_IMG.data_file'.
Then, we need to filter it out from qemu-img map, qemu-img create, and
remove the data file in _rm_test_img.
Signed-off-by: Max Reitz
Reviewed-by: Maxim Levitsky
---
tests/qemu-iotests/common.filter | 23 +--
tests/qemu-iotests/common.rc | 22 +-
2 files changed, 42 insertions(+), 3 deletions(-)
diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter
index 8a0169f19a..6902f9d94b 100644
--- a/tests/qemu-iotests/common.filter
+++ b/tests/qemu-iotests/common.filter
@@ -122,7 +122,13 @@ _filter_actual_image_size()
# replace driver-specific options in the "Formatting..." line
_filter_img_create()
{
-$SED -e "s#$REMOTE_TEST_DIR#TEST_DIR#g" \
+data_file_filter=()
+if data_file=$(_get_data_file "$TEST_IMG"); then
+data_file_filter=(-e "s# data_file=$data_file##")
+fi
+
+$SED "${data_file_filter[@]}" \
+-e "s#$REMOTE_TEST_DIR#TEST_DIR#g" \
-e "s#$IMGPROTO:$TEST_DIR#TEST_DIR#g" \
-e "s#$TEST_DIR#TEST_DIR#g" \
-e "s#$SOCK_DIR#SOCK_DIR#g" \
@@ -207,9 +213,22 @@ _filter_img_info()
# human and json output
_filter_qemu_img_map()
{
+# Assuming the data_file value in $IMGOPTS contains a '$TEST_IMG',
+# create a filter that replaces the data file name by $TEST_IMG.
+# Example:
+# In $IMGOPTS: 'data_file=$TEST_IMG.data_file'
+# Then data_file_pattern == '\(.*\).data_file'
+# And data_file_filter == -e 's#\(.*\).data_file#\1#
+data_file_filter=()
+if data_file_pattern=$(_get_data_file '\\(.*\\)'); then
+data_file_filter=(-e "s#$data_file_pattern#\\1#")
+fi
+
$SED -e 's/\([0-9a-fx]* *[0-9a-fx]* *\)[0-9a-fx]* */\1/g' \
-e 's/"offset": [0-9]\+/"offset": OFFSET/g' \
--e 's/Mapped to *//' | _filter_testdir | _filter_imgfmt
+-e 's/Mapped to *//' \
+"${data_file_filter[@]}" \
+| _filter_testdir | _filter_imgfmt
}
_filter_nbd()
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index a623485f96..a07a10a305 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -297,6 +297,20 @@ _stop_nbd_server()
fi
}
+# Gets the data_file value from IMGOPTS and replaces the '$TEST_IMG'
+# pattern by '$1'
+# Caution: The replacement is done with sed, so $1 must be escaped
+# properly. (The delimiter is '#'.)
+_get_data_file()
+{
+if ! echo "$IMGOPTS" | grep -q 'data_file='; then
+return 1
+fi
+
+echo "$IMGOPTS" | sed -e 's/.*data_file=\([^,]*\).*/\1/' \
+| sed -e "s#\\\$TEST_IMG#$1#"
+}
+
_make_test_img()
{
# extra qemu-img options can be added by tests
@@ -317,7 +331,8 @@ _make_test_img()
fi
if [ -n "$IMGOPTS" ]; then
-optstr=$(_optstr_add "$optstr" "$IMGOPTS")
+imgopts_expanded=$(echo "$IMGOPTS" | sed -e
"s#\\\$TEST_IMG#$img_name#")
+optstr=$(_optstr_add "$optstr" "$imgopts_expanded")
fi
if [ -n "$IMGKEYSECRET" ]; then
object_options="--object secret,id=keysec0,data=$IMGKEYSECRET"
@@ -396,6 +411,11 @@ _rm_test_img()
# Remove all the extents for vmdk
"$QEMU_IMG" info "$img" 2>/dev/null | grep 'filename:' | cut -f 2 -d: \
| xargs -I {} rm -f "{}"
+elif [ "$IMGFMT" = "qcow2" ]; then
+# Remove external data file
+if data_file=$(_get_data_file "$img"); then
+rm -f "$data_file"
+fi
fi
rm -f "$img"
}
--
2.23.0
Re: [PATCH v8 0/3] RTC support for QEMU RISC-V virt machine
On Wed, 06 Nov 2019 03:56:29 PST (-0800), Anup Patel wrote: This series adds RTC device to QEMU RISC-V virt machine. We have selected Goldfish RTC device model for this. It's a pretty simple synthetic device with few MMIO registers and no dependency external clock. The driver for Goldfish RTC is already available in Linux so we just need to enable it in Kconfig for RISCV and also update Linux defconfigs. We have tested this series with Linux-5.4-rc4 plus defconfig changes available in 'goldfish_rtc_v2' branch of: https://github.com/avpatel/linux.git Changes since v7: - Fix broken "stdout-path" in "/chosen" DT node of virt machine Changes since v6: - Rebased on latest QEMU master - Addressed all nit comments from Philippe Mathieu-Daude Changes since v5: - Rebased on latest QEMU master - Added maintainer entry for Goldfish RTC Changes since v4: - Fixed typo in trace event usage - Moved goldfish_rtc.h to correct location Changes since v3: - Address all nit comments from Alistair Changes since v2: - Rebased on RTC code refactoring Changes since v1: - Implemented VMState save/restore callbacks Anup Patel (3): hw: rtc: Add Goldfish RTC device riscv: virt: Use Goldfish RTC device MAINTAINERS: Add maintainer entry for Goldfish RTC MAINTAINERS | 8 + hw/riscv/Kconfig | 1 + hw/riscv/virt.c | 16 ++ hw/rtc/Kconfig| 3 + hw/rtc/Makefile.objs | 1 + hw/rtc/goldfish_rtc.c | 285 ++ hw/rtc/trace-events | 4 + include/hw/riscv/virt.h | 2 + include/hw/rtc/goldfish_rtc.h | 46 ++ 9 files changed, 366 insertions(+) create mode 100644 hw/rtc/goldfish_rtc.c create mode 100644 include/hw/rtc/goldfish_rtc.h Thanks. I've updated the patches on my queue, LMK if there are any more changes!
[PATCH v3 14/22] iotests: Avoid qemu-img create
Use _make_test_img whenever possible. This way, we will not ignore
user-specified image options.
Signed-off-by: Max Reitz
Reviewed-by: Maxim Levitsky
---
tests/qemu-iotests/094 | 2 +-
tests/qemu-iotests/111 | 3 +--
tests/qemu-iotests/123 | 2 +-
tests/qemu-iotests/153 | 2 +-
tests/qemu-iotests/200 | 4 ++--
5 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/tests/qemu-iotests/094 b/tests/qemu-iotests/094
index 9343e09492..d645952d54 100755
--- a/tests/qemu-iotests/094
+++ b/tests/qemu-iotests/094
@@ -45,7 +45,7 @@ _supported_proto nbd
_unsupported_imgopts "subformat=monolithicFlat" "subformat=twoGbMaxExtentFlat"
_make_test_img 64M
-$QEMU_IMG create -f $IMGFMT "$TEST_DIR/source.$IMGFMT" 64M | _filter_img_create
+TEST_IMG_FILE="$TEST_DIR/source.$IMGFMT" IMGPROTO=file _make_test_img 64M
_launch_qemu -drive if=none,id=src,file="$TEST_DIR/source.$IMGFMT",format=raw \
-nodefaults
diff --git a/tests/qemu-iotests/111 b/tests/qemu-iotests/111
index 490a5bbcb5..3b43d1bd83 100755
--- a/tests/qemu-iotests/111
+++ b/tests/qemu-iotests/111
@@ -41,8 +41,7 @@ _supported_fmt qed qcow qcow2 vmdk
_supported_proto file
_unsupported_imgopts "subformat=monolithicFlat" "subformat=twoGbMaxExtentFlat"
-$QEMU_IMG create -f $IMGFMT -b "$TEST_IMG.inexistent" "$TEST_IMG" 2>&1 \
-| _filter_testdir | _filter_imgfmt
+_make_test_img -b "$TEST_IMG.inexistent"
# success, all done
echo '*** done'
diff --git a/tests/qemu-iotests/123 b/tests/qemu-iotests/123
index d33950eb54..74d40d0478 100755
--- a/tests/qemu-iotests/123
+++ b/tests/qemu-iotests/123
@@ -44,7 +44,7 @@ _supported_os Linux
SRC_IMG="$TEST_DIR/source.$IMGFMT"
_make_test_img 1M
-$QEMU_IMG create -f $IMGFMT "$SRC_IMG" 1M | _filter_img_create
+TEST_IMG_FILE=$SRC_IMG IMGPROTO=file _make_test_img 1M
$QEMU_IO -c 'write -P 42 0 1M' "$SRC_IMG" | _filter_qemu_io
diff --git a/tests/qemu-iotests/153 b/tests/qemu-iotests/153
index c969a1a16f..e59090259c 100755
--- a/tests/qemu-iotests/153
+++ b/tests/qemu-iotests/153
@@ -98,7 +98,7 @@ for opts1 in "" "read-only=on" "read-only=on,force-share=on";
do
echo
echo "== Creating test image =="
-$QEMU_IMG create -f $IMGFMT "${TEST_IMG}" -b ${TEST_IMG}.base |
_filter_img_create
+_make_test_img -b "${TEST_IMG}.base"
echo
echo "== Launching QEMU, opts: '$opts1' =="
diff --git a/tests/qemu-iotests/200 b/tests/qemu-iotests/200
index 72d431f251..d904885136 100755
--- a/tests/qemu-iotests/200
+++ b/tests/qemu-iotests/200
@@ -46,8 +46,8 @@ _supported_proto file
BACKING_IMG="${TEST_DIR}/backing.img"
TEST_IMG="${TEST_DIR}/test.img"
-${QEMU_IMG} create -f $IMGFMT "${BACKING_IMG}" 512M | _filter_img_create
-${QEMU_IMG} create -f $IMGFMT -F $IMGFMT "${TEST_IMG}" -b "${BACKING_IMG}"
512M | _filter_img_create
+TEST_IMG="$BACKING_IMG" _make_test_img 512M
+_make_test_img -F $IMGFMT -b "$BACKING_IMG" 512M
${QEMU_IO} -c "write -P 0xa5 512 300M" "${BACKING_IMG}" | _filter_qemu_io
--
2.23.0
[PATCH v3 21/22] iotests: Disable data_file where it cannot be used
Signed-off-by: Max Reitz --- tests/qemu-iotests/007 | 5 +++-- tests/qemu-iotests/014 | 2 ++ tests/qemu-iotests/015 | 5 +++-- tests/qemu-iotests/026 | 5 - tests/qemu-iotests/029 | 5 +++-- tests/qemu-iotests/031 | 6 +++--- tests/qemu-iotests/036 | 5 +++-- tests/qemu-iotests/039 | 3 +++ tests/qemu-iotests/046 | 2 ++ tests/qemu-iotests/048 | 2 ++ tests/qemu-iotests/051 | 5 +++-- tests/qemu-iotests/058 | 5 +++-- tests/qemu-iotests/060 | 6 -- tests/qemu-iotests/061 | 6 -- tests/qemu-iotests/062 | 2 +- tests/qemu-iotests/066 | 4 +++- tests/qemu-iotests/067 | 6 -- tests/qemu-iotests/068 | 5 +++-- tests/qemu-iotests/071 | 3 +++ tests/qemu-iotests/073 | 4 tests/qemu-iotests/074 | 2 ++ tests/qemu-iotests/080 | 5 +++-- tests/qemu-iotests/090 | 2 ++ tests/qemu-iotests/098 | 6 -- tests/qemu-iotests/099 | 3 ++- tests/qemu-iotests/103 | 5 +++-- tests/qemu-iotests/108 | 6 -- tests/qemu-iotests/112 | 5 +++-- tests/qemu-iotests/114 | 2 ++ tests/qemu-iotests/121 | 3 +++ tests/qemu-iotests/138 | 3 +++ tests/qemu-iotests/156 | 2 ++ tests/qemu-iotests/176 | 7 +-- tests/qemu-iotests/191 | 2 ++ tests/qemu-iotests/201 | 6 +++--- tests/qemu-iotests/214 | 3 ++- tests/qemu-iotests/217 | 3 ++- tests/qemu-iotests/220 | 5 +++-- tests/qemu-iotests/243 | 6 -- tests/qemu-iotests/244 | 5 +++-- tests/qemu-iotests/250 | 2 ++ tests/qemu-iotests/261 | 3 ++- tests/qemu-iotests/267 | 5 +++-- 43 files changed, 124 insertions(+), 53 deletions(-) diff --git a/tests/qemu-iotests/007 b/tests/qemu-iotests/007 index 7d3544b479..160683adf8 100755 --- a/tests/qemu-iotests/007 +++ b/tests/qemu-iotests/007 @@ -41,8 +41,9 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 _supported_fmt qcow2 _supported_proto generic # refcount_bits must be at least 4 so we can create ten internal snapshots -# (1 bit supports none, 2 bits support two, 4 bits support 14) -_unsupported_imgopts 'refcount_bits=\(1\|2\)[^0-9]' +# (1 bit supports none, 2 bits support two, 4 bits support 14); +# snapshot are generally impossible with external data files +_unsupported_imgopts 'refcount_bits=\(1\|2\)[^0-9]' data_file echo echo "creating image" diff --git a/tests/qemu-iotests/014 b/tests/qemu-iotests/014 index 2f728a1956..e1221c0fff 100755 --- a/tests/qemu-iotests/014 +++ b/tests/qemu-iotests/014 @@ -43,6 +43,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 _supported_fmt qcow2 _supported_proto file _supported_os Linux +# Compression and snapshots do not work with external data files +_unsupported_imgopts data_file TEST_OFFSETS="0 4294967296" TEST_OPS="writev read write readv" diff --git a/tests/qemu-iotests/015 b/tests/qemu-iotests/015 index eec5387f3d..4d8effd0ae 100755 --- a/tests/qemu-iotests/015 +++ b/tests/qemu-iotests/015 @@ -40,8 +40,9 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 # actually any format that supports snapshots _supported_fmt qcow2 _supported_proto generic -# Internal snapshots are (currently) impossible with refcount_bits=1 -_unsupported_imgopts 'refcount_bits=1[^0-9]' +# Internal snapshots are (currently) impossible with refcount_bits=1, +# and generally impossible with external data files +_unsupported_imgopts 'refcount_bits=1[^0-9]' data_file echo echo "creating image" diff --git a/tests/qemu-iotests/026 b/tests/qemu-iotests/026 index 3430029ed6..a4aa74764f 100755 --- a/tests/qemu-iotests/026 +++ b/tests/qemu-iotests/026 @@ -49,7 +49,10 @@ _supported_cache_modes writethrough none # 32 and 64 bits do not work either, however, due to different leaked cluster # count on error. # Thus, the only remaining option is refcount_bits=16. -_unsupported_imgopts 'refcount_bits=\([^1]\|.\([^6]\|$\)\)' +# +# As for data_file, none of the refcount tests can work for it. +_unsupported_imgopts 'refcount_bits=\([^1]\|.\([^6]\|$\)\)' \ +data_file echo "Errors while writing 128 kB" echo diff --git a/tests/qemu-iotests/029 b/tests/qemu-iotests/029 index 9254ede5e5..2161a4b87a 100755 --- a/tests/qemu-iotests/029 +++ b/tests/qemu-iotests/029 @@ -42,8 +42,9 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 _supported_fmt qcow2 _supported_proto generic _unsupported_proto vxhs -# Internal snapshots are (currently) impossible with refcount_bits=1 -_unsupported_imgopts 'refcount_bits=1[^0-9]' +# Internal snapshots are (currently) impossible with refcount_bits=1, +# and generally impossible with external data files +_unsupported_imgopts 'refcount_bits=1[^0-9]' data_file offset_size=24 offset_l1_size=36 diff --git a/tests/qemu-iotests/031 b/tests/qemu-iotests/031 index c44fcf91bb..646ecd593f 100755 --- a/tests/qemu-iotests/031 +++ b/tests/qemu-iotests/031 @@ -40,9 +40,9 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 # This tests qcow2-specific low-level functionality _supported_fmt qcow2 _supported_proto file -# We want to test compat=0.10, which does not support refcount widths -# other than 16 -_unsupported_imgopts 'refcount_bits=\([^1]\|.\([^6]\|$\)\)' +# We want to test
[PATCH v3 17/22] iotests: Make 091 work with data_file
The image end offset as reported by qemu-img check is different when
using an external data file; we do not care about its value here, so we
can just filter it. Incidentally, common.rc already has _check_test_img
for us which does exactly that.
Signed-off-by: Max Reitz
Reviewed-by: Maxim Levitsky
---
tests/qemu-iotests/091 | 2 +-
tests/qemu-iotests/091.out | 2 --
2 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/tests/qemu-iotests/091 b/tests/qemu-iotests/091
index f4b44659ae..0874fa84c8 100755
--- a/tests/qemu-iotests/091
+++ b/tests/qemu-iotests/091
@@ -101,7 +101,7 @@ echo "Check image pattern"
${QEMU_IO} -c "read -P 0x22 0 4M" "${TEST_IMG}" | _filter_testdir |
_filter_qemu_io
echo "Running 'qemu-img check -r all \$TEST_IMG'"
-"${QEMU_IMG}" check -r all "${TEST_IMG}" 2>&1 | _filter_testdir | _filter_qemu
+_check_test_img -r all
echo "*** done"
rm -f $seq.full
diff --git a/tests/qemu-iotests/091.out b/tests/qemu-iotests/091.out
index 5017f8c2d9..5ec7b00f13 100644
--- a/tests/qemu-iotests/091.out
+++ b/tests/qemu-iotests/091.out
@@ -23,6 +23,4 @@ read 4194304/4194304 bytes at offset 0
4 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
Running 'qemu-img check -r all $TEST_IMG'
No errors were found on the image.
-80/16384 = 0.49% allocated, 0.00% fragmented, 0.00% compressed clusters
-Image end offset: 5570560
*** done
--
2.23.0
[PATCH v3 11/22] iotests: Replace IMGOPTS= by -o
Tests should not overwrite all user-supplied image options, but only add to it (which will effectively overwrite conflicting values). Accomplish this by passing options to _make_test_img via -o instead of $IMGOPTS. For some tests, there is no functional change because they already only appended options to IMGOPTS. For these, this patch is just a simplification. For others, this is a change, so they now heed user-specified $IMGOPTS. Some of those tests do not work with all image options, though, so we need to disable them accordingly. Signed-off-by: Max Reitz Reviewed-by: Maxim Levitsky --- tests/qemu-iotests/031 | 9 --- tests/qemu-iotests/039 | 24 ++ tests/qemu-iotests/059 | 18 ++--- tests/qemu-iotests/060 | 6 ++--- tests/qemu-iotests/061 | 57 ++ tests/qemu-iotests/079 | 3 +-- tests/qemu-iotests/106 | 2 +- tests/qemu-iotests/108 | 2 +- tests/qemu-iotests/112 | 32 tests/qemu-iotests/115 | 3 +-- tests/qemu-iotests/121 | 6 ++--- tests/qemu-iotests/125 | 2 +- tests/qemu-iotests/137 | 2 +- tests/qemu-iotests/138 | 3 +-- tests/qemu-iotests/175 | 2 +- tests/qemu-iotests/190 | 2 +- tests/qemu-iotests/191 | 3 +-- tests/qemu-iotests/220 | 4 ++- tests/qemu-iotests/243 | 6 +++-- tests/qemu-iotests/244 | 10 +--- tests/qemu-iotests/250 | 3 +-- tests/qemu-iotests/265 | 2 +- 22 files changed, 100 insertions(+), 101 deletions(-) diff --git a/tests/qemu-iotests/031 b/tests/qemu-iotests/031 index a3c25ec237..c44fcf91bb 100755 --- a/tests/qemu-iotests/031 +++ b/tests/qemu-iotests/031 @@ -40,19 +40,22 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 # This tests qcow2-specific low-level functionality _supported_fmt qcow2 _supported_proto file +# We want to test compat=0.10, which does not support refcount widths +# other than 16 +_unsupported_imgopts 'refcount_bits=\([^1]\|.\([^6]\|$\)\)' CLUSTER_SIZE=65536 # qcow2.py output depends on the exact options used, so override the command # line here as an exception -for IMGOPTS in "compat=0.10" "compat=1.1"; do +for compat in "compat=0.10" "compat=1.1"; do echo -echo = Testing with -o $IMGOPTS = +echo = Testing with -o $compat = echo echo === Create image with unknown header extension === echo -_make_test_img 64M +_make_test_img -o $compat 64M $PYTHON qcow2.py "$TEST_IMG" add-header-ext 0x12345678 "This is a test header extension" $PYTHON qcow2.py "$TEST_IMG" dump-header _check_test_img diff --git a/tests/qemu-iotests/039 b/tests/qemu-iotests/039 index 325da63a4c..99563bf126 100755 --- a/tests/qemu-iotests/039 +++ b/tests/qemu-iotests/039 @@ -50,8 +50,7 @@ size=128M echo echo "== Checking that image is clean on shutdown ==" -IMGOPTS="compat=1.1,lazy_refcounts=on" -_make_test_img $size +_make_test_img -o "compat=1.1,lazy_refcounts=on" $size $QEMU_IO -c "write -P 0x5a 0 512" "$TEST_IMG" | _filter_qemu_io @@ -62,8 +61,7 @@ _check_test_img echo echo "== Creating a dirty image file ==" -IMGOPTS="compat=1.1,lazy_refcounts=on" -_make_test_img $size +_make_test_img -o "compat=1.1,lazy_refcounts=on" $size _NO_VALGRIND \ $QEMU_IO -c "write -P 0x5a 0 512" \ @@ -98,8 +96,7 @@ $QEMU_IO -c "read -P 0x5a 0 512" "$TEST_IMG" | _filter_qemu_io echo echo "== Opening a dirty image read/write should repair it ==" -IMGOPTS="compat=1.1,lazy_refcounts=on" -_make_test_img $size +_make_test_img -o "compat=1.1,lazy_refcounts=on" $size _NO_VALGRIND \ $QEMU_IO -c "write -P 0x5a 0 512" \ @@ -117,8 +114,7 @@ $PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features echo echo "== Creating an image file with lazy_refcounts=off ==" -IMGOPTS="compat=1.1,lazy_refcounts=off" -_make_test_img $size +_make_test_img -o "compat=1.1,lazy_refcounts=off" $size _NO_VALGRIND \ $QEMU_IO -c "write -P 0x5a 0 512" \ @@ -132,11 +128,9 @@ _check_test_img echo echo "== Committing to a backing file with lazy_refcounts=on ==" -IMGOPTS="compat=1.1,lazy_refcounts=on" -TEST_IMG="$TEST_IMG".base _make_test_img $size +TEST_IMG="$TEST_IMG".base _make_test_img -o "compat=1.1,lazy_refcounts=on" $size -IMGOPTS="compat=1.1,lazy_refcounts=on,backing_file=$TEST_IMG.base" -_make_test_img $size +_make_test_img -o "compat=1.1,lazy_refcounts=on,backing_file=$TEST_IMG.base" $size $QEMU_IO -c "write 0 512" "$TEST_IMG" | _filter_qemu_io $QEMU_IMG commit "$TEST_IMG" @@ -151,8 +145,7 @@ TEST_IMG="$TEST_IMG".base _check_test_img echo echo "== Changing lazy_refcounts setting at runtime ==" -IMGOPTS="compat=1.1,lazy_refcounts=off" -_make_test_img $size +_make_test_img -o "compat=1.1,lazy_refcounts=off" $size _NO_VALGRIND \ $QEMU_IO -c "reopen -o lazy-refcounts=on" \ @@ -164,8 +157,7 @@ $QEMU_IO -c "reopen -o lazy-refcounts=on" \ $PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features _check_test_img -IMGOPTS="compat=1.1,lazy_refcounts=on" -_make_test_img $size
[PATCH v3 20/22] iotests: Make 198 work with data_file
We do not care about the json:{} filenames here, so we can just filter
them out and thus make the test work both with and without external data
files.
Signed-off-by: Max Reitz
Reviewed-by: Maxim Levitsky
---
tests/qemu-iotests/198 | 6 --
tests/qemu-iotests/198.out | 4 ++--
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/tests/qemu-iotests/198 b/tests/qemu-iotests/198
index c8f824cfae..fb0d5a29d3 100755
--- a/tests/qemu-iotests/198
+++ b/tests/qemu-iotests/198
@@ -92,13 +92,15 @@ echo
echo "== checking image base =="
$QEMU_IMG info --image-opts $IMGSPECBASE | _filter_img_info --format-specific \
| sed -e "/^disk size:/ D" -e '/refcount bits:/ D' -e '/compat:/ D' \
- -e '/lazy refcounts:/ D' -e '/corrupt:/ D'
+ -e '/lazy refcounts:/ D' -e '/corrupt:/ D' -e '/^\s*data file/ D' \
+| _filter_json_filename
echo
echo "== checking image layer =="
$QEMU_IMG info --image-opts $IMGSPECLAYER | _filter_img_info --format-specific
\
| sed -e "/^disk size:/ D" -e '/refcount bits:/ D' -e '/compat:/ D' \
- -e '/lazy refcounts:/ D' -e '/corrupt:/ D'
+ -e '/lazy refcounts:/ D' -e '/corrupt:/ D' -e '/^\s*data file/ D' \
+| _filter_json_filename
# success, all done
diff --git a/tests/qemu-iotests/198.out b/tests/qemu-iotests/198.out
index e86b175e39..831ce3a289 100644
--- a/tests/qemu-iotests/198.out
+++ b/tests/qemu-iotests/198.out
@@ -32,7 +32,7 @@ read 16777216/16777216 bytes at offset 0
16 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
== checking image base ==
-image: json:{"encrypt.key-secret": "sec0", "driver": "IMGFMT", "file":
{"driver": "file", "filename": "TEST_DIR/t.IMGFMT.base"}}
+image: json:{ /* filtered */ }
file format: IMGFMT
virtual size: 16 MiB (16777216 bytes)
Format specific information:
@@ -74,7 +74,7 @@ Format specific information:
master key iters: 1024
== checking image layer ==
-image: json:{"encrypt.key-secret": "sec1", "driver": "IMGFMT", "file":
{"driver": "file", "filename": "TEST_DIR/t.IMGFMT"}}
+image: json:{ /* filtered */ }
file format: IMGFMT
virtual size: 16 MiB (16777216 bytes)
backing file: TEST_DIR/t.IMGFMT.base
--
2.23.0
[PATCH v3 13/22] iotests: Drop IMGOPTS use in 267
Overwriting IMGOPTS means ignoring all user-supplied options, which is
not what we want. Replace the current IMGOPTS use by a new BACKING_FILE
variable.
Signed-off-by: Max Reitz
Reviewed-by: Maxim Levitsky
---
tests/qemu-iotests/267 | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/tests/qemu-iotests/267 b/tests/qemu-iotests/267
index 170e173c0a..f54262a4ad 100755
--- a/tests/qemu-iotests/267
+++ b/tests/qemu-iotests/267
@@ -68,7 +68,11 @@ size=128M
run_test()
{
-_make_test_img $size
+if [ -n "$BACKING_FILE" ]; then
+_make_test_img -b "$BACKING_FILE" $size
+else
+_make_test_img $size
+fi
printf "savevm snap0\ninfo snapshots\nloadvm snap0\n" | run_qemu "$@" |
_filter_date
}
@@ -119,12 +123,12 @@ echo
TEST_IMG="$TEST_IMG.base" _make_test_img $size
-IMGOPTS="backing_file=$TEST_IMG.base" \
+BACKING_FILE="$TEST_IMG.base" \
run_test -blockdev
driver=file,filename="$TEST_IMG.base",node-name=backing-file \
-blockdev driver=file,filename="$TEST_IMG",node-name=file \
-blockdev driver=$IMGFMT,file=file,backing=backing-file,node-name=fmt
-IMGOPTS="backing_file=$TEST_IMG.base" \
+BACKING_FILE="$TEST_IMG.base" \
run_test -blockdev
driver=file,filename="$TEST_IMG.base",node-name=backing-file \
-blockdev driver=$IMGFMT,file=backing-file,node-name=backing-fmt \
-blockdev driver=file,filename="$TEST_IMG",node-name=file \
@@ -141,7 +145,7 @@ echo
echo "=== -blockdev with NBD server on the backing file ==="
echo
-IMGOPTS="backing_file=$TEST_IMG.base" _make_test_img $size
+_make_test_img -b "$TEST_IMG.base" $size
cat <
[PATCH v3 19/22] iotests: Make 137 work with data_file
When using an external data file, there are no refcounts for data
clusters. We thus have to adjust the corruption test in this patch to
not be based around a data cluster allocation, but the L2 table
allocation (L2 tables are still refcounted with external data files).
Furthermore, we should not print qcow2.py's list of incompatible
features because it differs depending on whether there is an external
data file or not.
With those two changes, the test will work both with and without
external data files (once that options works with the iotests at all).
Signed-off-by: Max Reitz
Reviewed-by: Maxim Levitsky
---
tests/qemu-iotests/137 | 15 +++
tests/qemu-iotests/137.out | 6 ++
2 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/tests/qemu-iotests/137 b/tests/qemu-iotests/137
index 6cf2997577..7ae86892f7 100755
--- a/tests/qemu-iotests/137
+++ b/tests/qemu-iotests/137
@@ -138,14 +138,21 @@ $QEMU_IO \
"$TEST_IMG" 2>&1 | _filter_qemu_io
# The dirty bit must not be set
-$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+# (Filter the external data file bit)
+if $PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features \
+| grep -q '\<0\>'
+then
+echo 'ERROR: Dirty bit set'
+else
+echo 'OK: Dirty bit not set'
+fi
# Similarly we can test whether corruption detection has been enabled:
-# Create L1/L2, overwrite first entry in refcount block, allocate something.
+# Create L1, overwrite refcounts, force allocation of L2 by writing
+# data.
# Disabling the checks should fail, so the corruption must be detected.
_make_test_img 64M
-$QEMU_IO -c "write 0 64k" "$TEST_IMG" | _filter_qemu_io
-poke_file "$TEST_IMG" "$((0x2))" "\x00\x00"
+poke_file "$TEST_IMG" "$((0x2))" "\x00\x00\x00\x00\x00\x00\x00\x00"
$QEMU_IO \
-c "reopen -o overlap-check=none,lazy-refcounts=42" \
-c "write 64k 64k" \
diff --git a/tests/qemu-iotests/137.out b/tests/qemu-iotests/137.out
index bd4523a853..86377c80cd 100644
--- a/tests/qemu-iotests/137.out
+++ b/tests/qemu-iotests/137.out
@@ -36,11 +36,9 @@ qemu-io: Unsupported value 'blubb' for qcow2 option
'overlap-check'. Allowed are
wrote 512/512 bytes at offset 0
512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
./common.rc: Killed ( VALGRIND_QEMU="${VALGRIND_QEMU_IO}"
_qemu_proc_exec "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
-incompatible_features []
+OK: Dirty bit not set
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
-wrote 65536/65536 bytes at offset 0
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
qemu-io: Parameter 'lazy-refcounts' expects 'on' or 'off'
-qcow2: Marking image as corrupt: Preventing invalid write on metadata
(overlaps with qcow2_header); further corruption events will be suppressed
+qcow2: Marking image as corrupt: Preventing invalid allocation of L2 table at
offset 0; further corruption events will be suppressed
write failed: Input/output error
*** done
--
2.23.0
[PATCH v3 10/22] iotests: Inject space into -ocompat=0.10 in 051
It did not matter before, but now that _make_test_img understands -o, we should use it properly here. Signed-off-by: Max Reitz Reviewed-by: Maxim Levitsky --- tests/qemu-iotests/051 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051 index 53bcdbc911..9cd1d60d45 100755 --- a/tests/qemu-iotests/051 +++ b/tests/qemu-iotests/051 @@ -157,7 +157,7 @@ echo echo === With version 2 images enabling lazy refcounts must fail === echo -_make_test_img -ocompat=0.10 $size +_make_test_img -o compat=0.10 $size run_qemu -drive file="$TEST_IMG",format=qcow2,lazy-refcounts=on run_qemu -drive file="$TEST_IMG",format=qcow2,lazy-refcounts=off -- 2.23.0
[PATCH v3 16/22] iotests: Avoid cp/mv of test images
This will not work with external data files, so try to get tests working
without it as far as possible.
Signed-off-by: Max Reitz
Reviewed-by: Maxim Levitsky
---
tests/qemu-iotests/063 | 12
tests/qemu-iotests/063.out | 3 ++-
tests/qemu-iotests/085 | 9 +++--
tests/qemu-iotests/085.out | 8
4 files changed, 13 insertions(+), 19 deletions(-)
diff --git a/tests/qemu-iotests/063 b/tests/qemu-iotests/063
index eef2b8a534..c750b3806e 100755
--- a/tests/qemu-iotests/063
+++ b/tests/qemu-iotests/063
@@ -51,15 +51,13 @@ _unsupported_imgopts "subformat=monolithicFlat" \
_make_test_img 4M
echo "== Testing conversion with -n fails with no target file =="
-# check .orig file does not exist
-rm -f "$TEST_IMG.orig"
if $QEMU_IMG convert -f $IMGFMT -O $IMGFMT -n "$TEST_IMG" "$TEST_IMG.orig"
>/dev/null 2>&1; then
exit 1
fi
echo "== Testing conversion with -n succeeds with a target file =="
-rm -f "$TEST_IMG.orig"
-cp "$TEST_IMG" "$TEST_IMG.orig"
+_rm_test_img "$TEST_IMG.orig"
+TEST_IMG="$TEST_IMG.orig" _make_test_img 4M
if ! $QEMU_IMG convert -f $IMGFMT -O $IMGFMT -n "$TEST_IMG" "$TEST_IMG.orig" ;
then
exit 1
fi
@@ -85,10 +83,8 @@ fi
_check_test_img
echo "== Testing conversion to a smaller file fails =="
-rm -f "$TEST_IMG.orig"
-mv "$TEST_IMG" "$TEST_IMG.orig"
-_make_test_img 2M
-if $QEMU_IMG convert -f $IMGFMT -O $IMGFMT -n "$TEST_IMG.orig" "$TEST_IMG"
>/dev/null 2>&1; then
+TEST_IMG="$TEST_IMG.target" _make_test_img 2M
+if $QEMU_IMG convert -f $IMGFMT -O $IMGFMT -n "$TEST_IMG" "$TEST_IMG.target"
>/dev/null 2>&1; then
exit 1
fi
diff --git a/tests/qemu-iotests/063.out b/tests/qemu-iotests/063.out
index 7b691b2c9e..890b719bf0 100644
--- a/tests/qemu-iotests/063.out
+++ b/tests/qemu-iotests/063.out
@@ -2,11 +2,12 @@ QA output created by 063
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=4194304
== Testing conversion with -n fails with no target file ==
== Testing conversion with -n succeeds with a target file ==
+Formatting 'TEST_DIR/t.IMGFMT.orig', fmt=IMGFMT size=4194304
== Testing conversion to raw is the same after conversion with -n ==
== Testing conversion back to original format ==
No errors were found on the image.
== Testing conversion to a smaller file fails ==
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2097152
+Formatting 'TEST_DIR/t.IMGFMT.target', fmt=IMGFMT size=2097152
== Regression testing for copy offloading bug ==
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576
Formatting 'TEST_DIR/t.IMGFMT.target', fmt=IMGFMT size=1048576
diff --git a/tests/qemu-iotests/085 b/tests/qemu-iotests/085
index bbea1252d2..46981dbb64 100755
--- a/tests/qemu-iotests/085
+++ b/tests/qemu-iotests/085
@@ -105,8 +105,7 @@ add_snapshot_image()
{
base_image="${TEST_DIR}/$((${1}-1))-${snapshot_virt0}"
snapshot_file="${TEST_DIR}/${1}-${snapshot_virt0}"
-_make_test_img -u -b "${base_image}" "$size"
-mv "${TEST_IMG}" "${snapshot_file}"
+TEST_IMG=$snapshot_file _make_test_img -u -b "${base_image}" "$size"
do_blockdev_add "$1" "'backing': null, " "${snapshot_file}"
}
@@ -122,10 +121,8 @@ blockdev_snapshot()
size=128M
-_make_test_img $size
-mv "${TEST_IMG}" "${TEST_IMG}.1"
-_make_test_img $size
-mv "${TEST_IMG}" "${TEST_IMG}.2"
+TEST_IMG="$TEST_IMG.1" _make_test_img $size
+TEST_IMG="$TEST_IMG.2" _make_test_img $size
echo
echo === Running QEMU ===
diff --git a/tests/qemu-iotests/085.out b/tests/qemu-iotests/085.out
index 2a5f256cd3..313198f182 100644
--- a/tests/qemu-iotests/085.out
+++ b/tests/qemu-iotests/085.out
@@ -1,6 +1,6 @@
QA output created by 085
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
+Formatting 'TEST_DIR/t.IMGFMT.1', fmt=IMGFMT size=134217728
+Formatting 'TEST_DIR/t.IMGFMT.2', fmt=IMGFMT size=134217728
=== Running QEMU ===
@@ -55,10 +55,10 @@ Formatting 'TEST_DIR/10-snapshot-v1.qcow2', fmt=qcow2
size=134217728 backing_fil
=== Create a couple of snapshots using blockdev-snapshot ===
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
backing_file=TEST_DIR/10-snapshot-v0.IMGFMT
+Formatting 'TEST_DIR/11-snapshot-v0.IMGFMT', fmt=IMGFMT size=134217728
backing_file=TEST_DIR/10-snapshot-v0.IMGFMT
{"return": {}}
{"return": {}}
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
backing_file=TEST_DIR/11-snapshot-v0.IMGFMT
+Formatting 'TEST_DIR/12-snapshot-v0.IMGFMT', fmt=IMGFMT size=134217728
backing_file=TEST_DIR/11-snapshot-v0.IMGFMT
{"return": {}}
{"return": {}}
--
2.23.0
