Re: [PATCH 1/3] net/colo-compare.c: Create event_bh with the right AioContext
On 2020/4/23 下午3:29, Zhang, Chen wrote: -Original Message- From: Lukas Straub Sent: Wednesday, April 22, 2020 5:40 PM To: Zhang, Chen Cc: qemu-devel ; Li Zhijian ; Jason Wang ; Marc- André Lureau ; Paolo Bonzini Subject: Re: [PATCH 1/3] net/colo-compare.c: Create event_bh with the right AioContext On Wed, 22 Apr 2020 09:03:00 + "Zhang, Chen" wrote: -Original Message- From: Lukas Straub Sent: Wednesday, April 22, 2020 4:43 PM To: Zhang, Chen Cc: qemu-devel ; Li Zhijian ; Jason Wang ; Marc- André Lureau ; Paolo Bonzini Subject: Re: [PATCH 1/3] net/colo-compare.c: Create event_bh with the right AioContext On Wed, 22 Apr 2020 08:29:39 + "Zhang, Chen" wrote: -Original Message- From: Lukas Straub Sent: Thursday, April 9, 2020 2:34 AM To: qemu-devel Cc: Zhang, Chen ; Li Zhijian ; Jason Wang ; Marc- André Lureau ; Paolo Bonzini Subject: [PATCH 1/3] net/colo-compare.c: Create event_bh with the right AioContext qemu_bh_new will set the bh to be executed in the main loop. This causes problems as colo_compare_handle_event assumes that it has exclusive access the queues, which are also accessed in the iothread. It also assumes that it runs in a different thread than the caller and takes the appropriate locks. Create the bh with the AioContext of the iothread to fulfill these assumptions. Looks good for me, I assume it will increase performance. Do you have related data? No, this fixes several crashes because the queues where accessed concurrently from multiple threads. Sorry for my bad wording. Can you describe some details about the crash? Step by step? Maybe I can re-produce and test it for this patch. There is no clear test case. For me the crashes happened after 1-20h of runtime with lots of checkpoints (800ms) and some network traffic. The coredump always showed that two threads where doing operations on the queues simultaneously. Unfortunately, I don't have the coredumps anymore. OK, Although I have not encountered the problem you described. I have test this patch, looks running fine. Reviewed-by: Zhang Chen Thanks Zhang Chen Hi, I encountered PVM crash caused by the race condition issue in v4.2.0. Here is the coredump for reference. ``` warning: core file may not match specified executable file. Core was generated by `qemu-system-x86_64 -name source -enable-kvm -cpu core2duo -m 1024 -global kvm-a'. Program terminated with signal SIGSEGV, Segmentation fault. #0 0x55cb478bcd25 in qemu_hexdump (buf=0x0, fp=0x7f6e9122b680 , prefix=0x55cb47a388f5 "colo-compare spkt", size=1514) at util/hexdump.c:34 34 fprintf(fp, " %02x", (unsigned char)buf[b + i]); [Current thread is 1 (Thread 0x7f6da1ade700 (LWP 6119))] (gdb) where #0 0x55cb478bcd25 in qemu_hexdump (buf=0x0, fp=0x7f6e9122b680 , prefix=0x55cb47a388f5 "colo-compare spkt", size=1514) at util/hexdump.c:34 #1 0x55cb476fa1b5 in colo_compare_tcp (s=0x55cb496429f0, conn=0x7f6e78003e30) at net/colo-compare.c:462 #2 0x55cb476fa8c1 in colo_compare_connection (opaque=0x7f6e78003e30, user_data=0x55cb496429f0) at net/colo-compare.c:687 #3 0x55cb476fb4ab in compare_pri_rs_finalize (pri_rs=0x55cb49642b18) at net/colo-compare.c:1001 #4 0x55cb476eb46f in net_fill_rstate (rs=0x55cb49642b18, buf=0x7f6da1add2c8 "", size=1064) at net/net.c:1764 #5 0x55cb476faafa in compare_pri_chr_in (opaque=0x55cb496429f0, buf=0x7f6da1adc6f0 "`E˧V\210RT", size=4096) at net/colo-compare.c:776 #6 0x55cb47815363 in qemu_chr_be_write_impl (s=0x55cb48c87ec0, buf=0x7f6da1adc6f0 "`E˧V\210RT", len=4096) at chardev/char.c:177 #7 0x55cb478153c7 in qemu_chr_be_write (s=0x55cb48c87ec0, buf=0x7f6da1adc6f0 "`E˧V\210RT", len=4096) at chardev/char.c:189 #8 0x55cb4781e002 in tcp_chr_read (chan=0x55cb48ef7690, cond=G_IO_IN, opaque=0x55cb48c87ec0) at chardev/char-socket.c:525 #9 0x55cb47839655 in qio_channel_fd_source_dispatch (source=0x7f6e78002050, callback=0x55cb4781de53 , user_data=0x55cb48c87ec0) at io/channel-watch.c:84 #10 0x7f6e950e1285 in g_main_context_dispatch () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0 #11 0x7f6e950e1650 in () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0 #12 0x7f6e950e1962 in g_main_loop_run () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0 #13 0x55cb474920ae in iothread_run (opaque=0x55cb48c67f10) at iothread.c:82 #14 0x55cb478a699d in qemu_thread_start (args=0x55cb498035d0) at util/qemu-thread-posix.c:519 #15 0x7f6e912376db in start_thread (arg=0x7f6da1ade700) at pthread_create.c:463 #16 0x7f6e90f6088f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95 (gdb) ``` COLO works well after applying this patch in my tests. Reviewed-by: Derek Su Tested-by: Derek Su Regards, Derek Regards, Lukas Straub Thanks Zhang Chen Regards, Lukas Straub Thanks Zhang Chen Signed-off-by: Lukas Straub --- net/colo-compare.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
Re: [PATCH 0/7] chardev: Reduce system emulation specific code
On 4/23/20 1:21 PM, Philippe Mathieu-Daudé wrote: > chardev cleanup while reviewing 'Refactor machine_init and exit > notifiers' from the multi-process series: > https://www.mail-archive.com/qemu-devel@nongnu.org/msg697510.html > > Elena Ufimtseva (1): > multi-process: Refactor machine_init and exit notifiers > > Philippe Mathieu-Daudé (6): > monitor/misc: Remove unused "chardev/char-mux.h" include > tests/test-char: Remove unused "chardev/char-mux.h" include > chardev: Restrict msmouse / wctablet / testdev to system emulation > chardev: Reduce "char-mux.h" scope, rename it "chardev-internal.h" > chardev: Extract system emulation specific code > stubs: Split machine-init-done as machine-init and machine-notify Reviewed-by: Richard Henderson r~
Re: [RFC PATCH v1 1/7] contrib/vhost-user-blk: add option to simulate disconnect on init
I’m not opposed to adding this kind of debugging functionality to the vhost-user-blk sample. It could be helpful to easily test these cases in the future. That said, I'm not sure how others will feel about adding these kind of debugging capabilities to libvhost-user. Marc-Andre, thoughts? If we go this route I would prefer to add the debugging options to the vhost-user-blk sample in a separate patch. On Thu, Apr 23, 2020 at 09:39:32PM +0300, Dima Stepanov wrote: > > Add "--simulate-disconnect-stage" option for the testing purposes. > This option can be used to test the vhost-user reconnect functionality: > ./vhost-user-blk ... --simulate-disconnect-stage= > In this case the daemon will "crash" in the middle of the VHOST comands > communication. Case nums are as follows: > 1 - make assert in the handler of the SET_VRING_CALL command > 2 - make assert in the handler of the SET_VRING_NUM command > Main purpose is to test QEMU reconnect functionality. Such fail > injection should not lead to QEMU crash and should be handled > successfully. > Also update the "GOptionEntry entries" definition with the final NULL > item according to API. > > Signed-off-by: Dima Stepanov > --- > contrib/libvhost-user/libvhost-user.c | 30 ++ > contrib/libvhost-user/libvhost-user.h | 13 + > contrib/vhost-user-blk/vhost-user-blk.c | 14 +- > 3 files changed, 56 insertions(+), 1 deletion(-)
Re: [RFC PATCH v1 5/7] vhost-user-blk: add mechanism to track the guest notifiers init state
There are some problems with this patch. It doesn't apply cleanly.
Are you sure you’re developing on an up to date master branch?
On Thu, Apr 23, 2020 at 09:39:36PM +0300, Dima Stepanov wrote:
>
> In case of the vhost-user devices the daemon can be killed at any
> moment. Since QEMU supports the reconnet functionality the guest
> notifiers should be reset and disabled after "disconnect" event. The
> most issues were found if the "disconnect" event happened during vhost
> device initialization step.
> The disconnect event leads to the call of the vhost_dev_cleanup()
> routine. Which memset to 0 a vhost device structure. Because of this, if
> device was not started (dev.started == false) and the connection is
> broken, then the set_guest_notifier method will produce assertion error.
> Also connection can be broken after the dev.started field is set to
> true.
> A new notifiers_set field is added to the vhost_dev structure to track
> the state of the guest notifiers during the initialization process.
>
> Signed-off-by: Dima Stepanov
> ---
> hw/block/vhost-user-blk.c | 8
> hw/virtio/vhost.c | 11 +++
> include/hw/virtio/vhost.h | 1 +
> 3 files changed, 16 insertions(+), 4 deletions(-)
> @@ -1449,6 +1456,10 @@ int vhost_dev_drop_guest_notifiers(struct
vhost_dev *hdev,
I can’t find the function vhost_dev_drop_guest_notifiers() in
/hw/virtio/vhost.c, or anywhere in the codebase.
Where does this code come from?
> VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
> int ret;
>
> +if (!hdev->notifiers_set) {
> +return 0;
> +}
> +
> ret = k->set_guest_notifiers(qbus->parent, nvqs, false);
> if (ret < 0) {
> error_report("Error reset guest notifier: %d", -ret);
> diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
> index 4d0d2e2..e3711a7 100644
> --- a/include/hw/virtio/vhost.h
> +++ b/include/hw/virtio/vhost.h
> @@ -90,6 +90,7 @@ struct vhost_dev {
> QLIST_HEAD(, vhost_iommu) iommu_list;
> IOMMUNotifier n;
> const VhostDevConfigOps *config_ops;
> +bool notifiers_set;
> };
>
> int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
> --
> 2.7.4
>
>
Re: [PATCH 0/2] net/colo-compare.c: Expose "max_queue_size" to users and clean up
On 2020/4/11 上午11:38, Zhang Chen wrote: From: Zhang Chen This series make a way to config COLO "max_queue_size" parameters according to user's scenarios and environments and do some clean up for descriptions. Zhang Chen (2): net/colo-compare.c: Expose compare "max_queue_size" to users qemu-options.hx: Clean up and fix typo for colo-compare net/colo-compare.c | 43 ++- qemu-options.hx| 33 + 2 files changed, 59 insertions(+), 17 deletions(-) Queued for 5.1. Thanks
Re: [RFC PATCH 3/3] hw/net/tulip: Set descriptor error bit when lenght is incorrect
On 2020/4/24 上午7:16, Philippe Mathieu-Daudé wrote:
When a frame lenght is incorrect, set the RDES0 'Error Summary'
and 'Frame too long' bits. Then stop the receive process and
trigger an abnormal interrupt. See [4.3.5 Receive Process].
Cc: Li Qiang
Cc: Li Qiang
Cc: Ziming Zhang
Cc: Jason Wang
Cc: Prasad J Pandit
Fixes: 8ffb7265af ("check frame size and r/w data length")
Buglink: https://bugs.launchpad.net/bugs/1874539
Reported-by: Helge Deller
Signed-off-by: Philippe Mathieu-Daudé
Hi Philippe:
It's still unclear to me that how this fixes the stuck. Did you mean
guest trigger the error condition and then recvoer by abnormal interrupt?
If yes, this sounds still like a bug somewhere in the code.
Thanks
---
hw/net/tulip.c | 16
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/hw/net/tulip.c b/hw/net/tulip.c
index 470f635acb..671f79b6f4 100644
--- a/hw/net/tulip.c
+++ b/hw/net/tulip.c
@@ -158,7 +158,7 @@ static void tulip_next_rx_descriptor(TULIPState *s,
s->current_rx_desc &= ~3ULL;
}
-static void tulip_copy_rx_bytes(TULIPState *s, struct tulip_descriptor *desc)
+static int tulip_copy_rx_bytes(TULIPState *s, struct tulip_descriptor *desc)
{
int len1 = (desc->control >> RDES1_BUF1_SIZE_SHIFT) &
RDES1_BUF1_SIZE_MASK;
int len2 = (desc->control >> RDES1_BUF2_SIZE_SHIFT) &
RDES1_BUF2_SIZE_MASK;
@@ -177,7 +177,8 @@ static void tulip_copy_rx_bytes(TULIPState *s, struct
tulip_descriptor *desc)
"(ofs: %u, len:%d, size:%zu)\n",
__func__, s->rx_frame_len, len,
sizeof(s->rx_frame));
-return;
+s->rx_frame_len = 0;
+return -1;
}
pci_dma_write(>dev, desc->buf_addr1, s->rx_frame +
(s->rx_frame_size - s->rx_frame_len), len);
@@ -197,12 +198,15 @@ static void tulip_copy_rx_bytes(TULIPState *s, struct
tulip_descriptor *desc)
"(ofs: %u, len:%d, size:%zu)\n",
__func__, s->rx_frame_len, len,
sizeof(s->rx_frame));
-return;
+s->rx_frame_len = 0;
+return -1;
}
pci_dma_write(>dev, desc->buf_addr2, s->rx_frame +
(s->rx_frame_size - s->rx_frame_len), len);
s->rx_frame_len -= len;
}
+
+return 0;
}
static bool tulip_filter_address(TULIPState *s, const uint8_t *addr)
@@ -274,7 +278,11 @@ static ssize_t tulip_receive(TULIPState *s, const uint8_t
*buf, size_t size)
s->rx_frame_len = s->rx_frame_size;
}
-tulip_copy_rx_bytes(s, );
+if (tulip_copy_rx_bytes(s, )) {
+desc.status |= RDES0_ES | RDES0_TL; /* Error: frame too long */
+s->csr[5] |= CSR5_RPS; /* Receive process stopped */
+tulip_update_int(s);
+}
if (!s->rx_frame_len) {
desc.status |= s->rx_status;
Re: [PATCH] spapr_nvdimm.c: make 'label-size' mandatory
On Tue, Apr 14, 2020 at 08:04:29AM -0300, Daniel Henrique Barboza wrote: > > > On 4/13/20 11:01 PM, David Gibson wrote: > > CCing Xiao, Michael and Igor for generic NVDIMM perspective. > > > > On Mon, Apr 13, 2020 at 05:36:28PM -0300, Daniel Henrique Barboza wrote: > > > The pseries machine does not support NVDIMM modules without label. > > > Attempting to do so, even if the overall block size is aligned with > > > 256MB, will seg fault the guest kernel during NVDIMM probe. This > > > can be avoided by forcing 'label-size' to always be present for > > > sPAPR NVDIMMs. > > > > > > The verification was put before the alignment check because the > > > presence of label-size affects the alignment calculation, so > > > it's not optimal to warn the user about an alignment error, > > > then about the lack of label-size, then about a new alignment > > > error when the user sets a label-size. > > > > > > Signed-off-by: Daniel Henrique Barboza > > > > So, this would get the job done, but it seems a bit inelegant compared > > to having the device default to working settings. I'm looking at how > > this interacts with the generic constraints on label-size. > > > > The generic nvdimm code has a MIN_NAMESPACE_LABEL_SIZE of 128 kiB, and > > values of label-size less than that are rejected. Except that if > > label-size is not set at all, it is left as 0. > > > > Is that intended behaviour? Do x86 (or whatever) NVDIMMs have a label > > of at least 128kiB, unless they have no label at all? Or could we > > make the default label-size 128kiB generically? > > My limited understanding on how NVDIMM works in x86 is that x86 NVDIMMs can > work with and without label, but the label has a minimum size of 128kiB. Ok. Kinda weird, but ok. But.. the thing that bothers me about this is that the guest is crashing in generic code. If the generic code can handle a label-less nvdimm on x86, why is it dying on power. After a bit of poking, I think the answer is that x86 is explicitly treating label_size==0 as "label not supported" and returning explicit errors for the get_label_size/read_label/write_label low level operations, which the generic code is handling. ppc, I think, is instead just returning bogus/meaningless values which the generic code is choking on. So, I really think we should have the spapr_scm driver on the guest side check for the zero sized label case and return similar errors, for robustness. But, I don't care enough to do it myself. -- 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
Re: [PATCH] spapr_nvdimm.c: make 'label-size' mandatory
On Mon, Apr 13, 2020 at 05:36:28PM -0300, Daniel Henrique Barboza wrote:
> The pseries machine does not support NVDIMM modules without label.
> Attempting to do so, even if the overall block size is aligned with
> 256MB, will seg fault the guest kernel during NVDIMM probe. This
> can be avoided by forcing 'label-size' to always be present for
> sPAPR NVDIMMs.
>
> The verification was put before the alignment check because the
> presence of label-size affects the alignment calculation, so
> it's not optimal to warn the user about an alignment error,
> then about the lack of label-size, then about a new alignment
> error when the user sets a label-size.
>
> Signed-off-by: Daniel Henrique Barboza
So, I still think it's kind of bogus that the guest side driver falls
over so messily (more on that elswhere in the thread).
However, regardless of that, it does make sense to enforce the PAPR
restriction that all NVDIMMs have labels. And it fixes the visible
problem with a minimal change.
So, I've applied to ppc-for-5.1. I am going to update the error
messages a little to make it clearer that these are PAPR specific
restrictions.
> ---
> hw/ppc/spapr_nvdimm.c | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/hw/ppc/spapr_nvdimm.c b/hw/ppc/spapr_nvdimm.c
> index 25be8082d7..9abcdcc26b 100644
> --- a/hw/ppc/spapr_nvdimm.c
> +++ b/hw/ppc/spapr_nvdimm.c
> @@ -37,6 +37,12 @@ void spapr_nvdimm_validate_opts(NVDIMMDevice *nvdimm,
> uint64_t size,
> QemuUUID uuid;
> int ret;
>
> +if (object_property_get_int(OBJECT(nvdimm), NVDIMM_LABEL_SIZE_PROP,
> +_abort) == 0) {
> +error_setg(errp, "NVDIMM device requires label-size to be set");
> +return;
> +}
> +
> if (size % SPAPR_MINIMUM_SCM_BLOCK_SIZE) {
> error_setg(errp, "NVDIMM memory size excluding the label area"
> " must be a multiple of %" PRIu64 "MB",
--
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
RE: [PATCH 0/2] net/colo-compare.c: Expose "max_queue_size" to users and clean up
> -Original Message- > From: Jason Wang > Sent: Thursday, April 23, 2020 5:07 PM > To: Zhang, Chen ; qemu-dev de...@nongnu.org> > Cc: Zhang Chen > Subject: Re: [PATCH 0/2] net/colo-compare.c: Expose "max_queue_size" to > users and clean up > > > On 2020/4/23 下午4:59, Zhang, Chen wrote: > > > >> -Original Message- > >> From: Jason Wang > >> Sent: Thursday, April 23, 2020 4:54 PM > >> To: Zhang, Chen ; qemu-dev >> de...@nongnu.org> > >> Cc: Zhang Chen > >> Subject: Re: [PATCH 0/2] net/colo-compare.c: Expose "max_queue_size" > >> to users and clean up > >> > >> > >> On 2020/4/23 下午3:31, Zhang, Chen wrote: > >>> Hi Jason, > >>> > >>> Please review this series when you free. > >>> > >>> Thanks > >>> Zhang Chen > >>> > >> Sure. > >> > >> I wonder maybe it's better e.g you can review and collect the patches > >> that looks good and send them to me periodically? > > OK, I will queue COLO related patch as one series to you. > > Do I need send a pull request? or just a big patch set? > > > I prefer big patch set. OK, I got it. Thanks Zhang Chen > > Thanks > > > > > > Thanks > > Zhang Chen > > > >> Thanks
Re: [PATCH RESEND v6 36/36] multi-process: add configure and usage information
On Thu, Apr 23, 2020 at 11:03 PM Jag Raman wrote: > > > > On Apr 23, 2020, at 9:54 AM, 罗勇刚(Yonggang Luo) > wrote: > > > > Does multi-process support on Windows? > > I found it use mmap and unix socket for inter-process communication, > that may not support under Windows. > > Hi Yonggang, > > We have only tested this on Linux till now. Are you using QEMU with > Windows? > > > And also, can the python script be replaced by C implementation? > > The functionality in the python script would eventually move to libvirt. > The python > script is a temporary measure. > > I suggest use qemu as the driver like clang-cl, we preseve the options currently avaiable in qemu and then starting remote process by qemu directly, so we libvirt need to lauch with qemu > Thank you very much! > — > Jag > > > > > On Thu, Apr 23, 2020 at 12:38 PM wrote: > > From: Elena Ufimtseva > > > > Signed-off-by: Elena Ufimtseva > > Signed-off-by: Jagannathan Raman > > Signed-off-by: John G Johnson > > --- > > MAINTAINERS | 2 + > > docs/multi-process.rst | 85 + > > scripts/mpqemu-launcher-perf-mode.py | 92 > > scripts/mpqemu-launcher.py | 53 > > 4 files changed, 232 insertions(+) > > create mode 100644 docs/multi-process.rst > > create mode 100755 scripts/mpqemu-launcher-perf-mode.py > > create mode 100755 scripts/mpqemu-launcher.py > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index ed48615e15..8ff3bfae6a 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -2880,6 +2880,8 @@ F: remote/iohub.c > > F: remote/remote-opts.h > > F: remote/remote-opts.c > > F: docs/devel/multi-process.rst > > +F: scripts/mpqemu-launcher.py > > +F: scripts/mpqemu-launcher-perf-mode.py > > > > Build and test automation > > - > > diff --git a/docs/multi-process.rst b/docs/multi-process.rst > > new file mode 100644 > > index 00..8387d6c691 > > --- /dev/null > > +++ b/docs/multi-process.rst > > @@ -0,0 +1,85 @@ > > +Multi-process QEMU > > +== > > + > > +This document describes how to configure and use multi-process qemu. > > +For the design document refer to docs/devel/qemu-multiprocess. > > + > > +1) Configuration > > + > > + > > +To enable support for multi-process add --enable-mpqemu > > +to the list of options for the "configure" script. > > + > > + > > +2) Usage > > + > > + > > +Multi-process QEMU requires an orchestrator to launch. Please refer to a > > +light-weight python based orchestrator for mpqemu in > > +scripts/mpqemu-launcher.py to lauch QEMU in multi-process mode. > > + > > +scripts/mpqemu-launcher-perf-mode.py launches in "perf" mode. In this > mode, > > +the same QEMU process connects to multiple remote devices, each > emulated in > > +a separate process. > > + > > +As of now, we only support the emulation of lsi53c895a in a separate > process. > > + > > +Following is a description of command-line used to launch mpqemu. > > + > > +* Orchestrator: > > + > > + - The Orchestrator creates a unix socketpair > > + > > + - It launches the remote process and passes one of the > > +sockets to it via command-line. > > + > > + - It then launches QEMU and specifies the other socket as an option > > +to the Proxy device object > > + > > +* Remote Process: > > + > > + - The first command-line option in the remote process is one of the > > +sockets created by the Orchestrator > > + > > + - The remaining options are no different from how one launches QEMU > with > > +devices. The only other requirement is each PCI device must have a > > +unique ID specified to it. This is needed to pair remote device > with the > > +Proxy object. > > + > > + - Example command-line for the remote process is as follows: > > + > > + /usr/bin/qemu-scsu-dev 4 > \ > > + -device lsi53c895a,id=lsi0 > \ > > + -drive id=drive_image2,file=/build/ol7-nvme-test-1.qcow2 > \ > > + -device scsi-hd,id=drive2,drive=drive_image2,bus=lsi0.0,scsi-id=0 > > + > > +* QEMU: > > + > > + - Since parts of the RAM are shared between QEMU & remote process, a > > +memory-backend-memfd is required to facilitate this, as follows: > > + > > +-object memory-backend-memfd,id=mem,size=2G > > + > > + - A "pci-proxy-dev" device is created for each of the PCI devices > emulated > > +in the remote process. A "socket" sub-option specifies the other > end of > > +unix channel created by orchestrator. The "id" sub-option must be > specified > > +and should be the same as the "id" specified for the remote PCI > device > > + > > + - Example commandline for QEMU is as follows: > > + > > + -device pci-proxy-dev,id=lsi0,socket=3 > > + > > +* Monitor / QMP: > > + > > + - The remote process supports QEMU monitor. It could be specified > using the > > +"-monitor" or "-qmp" command-line options > > + > > + - As an example, one
[RFC PATCH 0/3] hw/net/tulip: Fix LP#1874539
Attempt to fix the launchpad bug filled by Helge: In a qemu-system-hppa system, qemu release v5.0.0-rc, the tulip nic driver is broken. The tulip nic is detected, even getting DHCP info does work. But when trying to download bigger files via network, the tulip driver gets stuck. Philippe Mathieu-Daudé (3): hw/net/tulip: Fix 'Descriptor Error' definition hw/net/tulip: Log descriptor overflows hw/net/tulip: Set descriptor error bit when lenght is incorrect hw/net/tulip.h | 2 +- hw/net/tulip.c | 32 2 files changed, 29 insertions(+), 5 deletions(-) -- 2.21.1
[RFC PATCH 3/3] hw/net/tulip: Set descriptor error bit when lenght is incorrect
When a frame lenght is incorrect, set the RDES0 'Error Summary'
and 'Frame too long' bits. Then stop the receive process and
trigger an abnormal interrupt. See [4.3.5 Receive Process].
Cc: Li Qiang
Cc: Li Qiang
Cc: Ziming Zhang
Cc: Jason Wang
Cc: Prasad J Pandit
Fixes: 8ffb7265af ("check frame size and r/w data length")
Buglink: https://bugs.launchpad.net/bugs/1874539
Reported-by: Helge Deller
Signed-off-by: Philippe Mathieu-Daudé
---
hw/net/tulip.c | 16
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/hw/net/tulip.c b/hw/net/tulip.c
index 470f635acb..671f79b6f4 100644
--- a/hw/net/tulip.c
+++ b/hw/net/tulip.c
@@ -158,7 +158,7 @@ static void tulip_next_rx_descriptor(TULIPState *s,
s->current_rx_desc &= ~3ULL;
}
-static void tulip_copy_rx_bytes(TULIPState *s, struct tulip_descriptor *desc)
+static int tulip_copy_rx_bytes(TULIPState *s, struct tulip_descriptor *desc)
{
int len1 = (desc->control >> RDES1_BUF1_SIZE_SHIFT) & RDES1_BUF1_SIZE_MASK;
int len2 = (desc->control >> RDES1_BUF2_SIZE_SHIFT) & RDES1_BUF2_SIZE_MASK;
@@ -177,7 +177,8 @@ static void tulip_copy_rx_bytes(TULIPState *s, struct
tulip_descriptor *desc)
"(ofs: %u, len:%d, size:%zu)\n",
__func__, s->rx_frame_len, len,
sizeof(s->rx_frame));
-return;
+s->rx_frame_len = 0;
+return -1;
}
pci_dma_write(>dev, desc->buf_addr1, s->rx_frame +
(s->rx_frame_size - s->rx_frame_len), len);
@@ -197,12 +198,15 @@ static void tulip_copy_rx_bytes(TULIPState *s, struct
tulip_descriptor *desc)
"(ofs: %u, len:%d, size:%zu)\n",
__func__, s->rx_frame_len, len,
sizeof(s->rx_frame));
-return;
+s->rx_frame_len = 0;
+return -1;
}
pci_dma_write(>dev, desc->buf_addr2, s->rx_frame +
(s->rx_frame_size - s->rx_frame_len), len);
s->rx_frame_len -= len;
}
+
+return 0;
}
static bool tulip_filter_address(TULIPState *s, const uint8_t *addr)
@@ -274,7 +278,11 @@ static ssize_t tulip_receive(TULIPState *s, const uint8_t
*buf, size_t size)
s->rx_frame_len = s->rx_frame_size;
}
-tulip_copy_rx_bytes(s, );
+if (tulip_copy_rx_bytes(s, )) {
+desc.status |= RDES0_ES | RDES0_TL; /* Error: frame too long */
+s->csr[5] |= CSR5_RPS; /* Receive process stopped */
+tulip_update_int(s);
+}
if (!s->rx_frame_len) {
desc.status |= s->rx_status;
--
2.21.1
[RFC PATCH 1/3] hw/net/tulip: Fix 'Descriptor Error' definition
Bit #14 is "DE" for 'Descriptor Error': When set, indicates a frame truncation caused by a frame that does not fit within the current descriptor buffers, and that the 21143 does not own the next descriptor. [Table 4-1. RDES0 Bit Fields Description] Signed-off-by: Philippe Mathieu-Daudé --- hw/net/tulip.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/net/tulip.h b/hw/net/tulip.h index 97521b21db..5271aad8d5 100644 --- a/hw/net/tulip.h +++ b/hw/net/tulip.h @@ -211,7 +211,7 @@ #define RDES0_RF BIT(11) #define RDES0_DT_SHIFT 12 #define RDES0_DT_MASK3 -#define RDES0_LE BIT(14) +#define RDES0_DE BIT(14) #define RDES0_ES BIT(15) #define RDES0_FL_SHIFT 16 #define RDES0_FL_MASK0x3fff -- 2.21.1
[RFC PATCH 2/3] hw/net/tulip: Log descriptor overflows
Log with GUEST_ERROR what the guest is doing wrong.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/net/tulip.c | 16
1 file changed, 16 insertions(+)
diff --git a/hw/net/tulip.c b/hw/net/tulip.c
index 1295f51d07..470f635acb 100644
--- a/hw/net/tulip.c
+++ b/hw/net/tulip.c
@@ -172,6 +172,11 @@ static void tulip_copy_rx_bytes(TULIPState *s, struct
tulip_descriptor *desc)
}
if (s->rx_frame_len + len > sizeof(s->rx_frame)) {
+qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: descriptor overflow "
+ "(ofs: %u, len:%d, size:%zu)\n",
+ __func__, s->rx_frame_len, len,
+ sizeof(s->rx_frame));
return;
}
pci_dma_write(>dev, desc->buf_addr1, s->rx_frame +
@@ -187,6 +192,11 @@ static void tulip_copy_rx_bytes(TULIPState *s, struct
tulip_descriptor *desc)
}
if (s->rx_frame_len + len > sizeof(s->rx_frame)) {
+qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: descriptor overflow "
+ "(ofs: %u, len:%d, size:%zu)\n",
+ __func__, s->rx_frame_len, len,
+ sizeof(s->rx_frame));
return;
}
pci_dma_write(>dev, desc->buf_addr2, s->rx_frame +
@@ -584,6 +594,9 @@ static int tulip_copy_tx_buffers(TULIPState *s, struct
tulip_descriptor *desc)
int len2 = (desc->control >> TDES1_BUF2_SIZE_SHIFT) & TDES1_BUF2_SIZE_MASK;
if (s->tx_frame_len + len1 > sizeof(s->tx_frame)) {
+qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: descriptor overflow (ofs: %u, len:%d, size:%zu)\n",
+ __func__, s->tx_frame_len, len1, sizeof(s->tx_frame));
return -1;
}
if (len1) {
@@ -593,6 +606,9 @@ static int tulip_copy_tx_buffers(TULIPState *s, struct
tulip_descriptor *desc)
}
if (s->tx_frame_len + len2 > sizeof(s->tx_frame)) {
+qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: descriptor overflow (ofs: %u, len:%d, size:%zu)\n",
+ __func__, s->tx_frame_len, len2, sizeof(s->tx_frame));
return -1;
}
if (len2) {
--
2.21.1
Re: [PATCH v2 13/36] tcg: Use tcg_constant_{i32,i64} with tcg int expanders
On 4/22/20 1:04 PM, Alex Bennée wrote:
>
> Richard Henderson writes:
>
>> Signed-off-by: Richard Henderson
>
> We have a regression. Setting up a build dir with:
>
> ../../configure --disable-tools --disable-docs
> --target-list=sparc-softmmu,sparc64-softmmu
> make -j30 && make check-acceptance
>
> And then running a bisect between HEAD and master:
>
> git bisect run /bin/sh -c "cd builds/bisect && make -j30 &&
> ./tests/venv/bin/avocado run
> ./tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_sparc_ss20"
>
> Fingers:
>
> a4d42b76dd29818e4f393c4c3eb59601b0015b2f is the first bad commit
> commit a4d42b76dd29818e4f393c4c3eb59601b0015b2f
> Author: Richard Henderson
> Date: Tue Apr 21 18:16:59 2020 -0700
>
> tcg: Use tcg_constant_{i32,i64} with tcg int expanders
>
> Signed-off-by: Richard Henderson
> Message-Id: <20200422011722.13287-14-richard.hender...@linaro.org>
Ho hum. I can reproduce this, but after a day of debugging I'm no closer to
figuring out what's wrong than when I started.
I'm going to put this whole section of TEMP_CONST to the side for now.
r~
Re: [PATCH v2 09/36] tcg: Consolidate 3 bits into enum TCGTempKind
On 4/23/20 10:24 AM, Daniel P. Berrangé wrote:
> On Thu, Apr 23, 2020 at 08:40:10AM -0700, Richard Henderson wrote:
>> On 4/23/20 2:00 AM, Philippe Mathieu-Daudé wrote:
> @@ -1885,12 +1896,17 @@ static char *tcg_get_arg_str_ptr(TCGContext *s,
> char *buf, int buf_size,
> {
> int idx = temp_idx(ts);
>
> -if (ts->temp_global) {
> +switch (ts->kind) {
> +case TEMP_FIXED:
> +case TEMP_GLOBAL:
> pstrcpy(buf, buf_size, ts->name);
> -} else if (ts->temp_local) {
> +break;
> +case TEMP_LOCAL:
> snprintf(buf, buf_size, "loc%d", idx - s->nb_globals);
> -} else {
> +break;
> +case TEMP_NORMAL:
> snprintf(buf, buf_size, "tmp%d", idx - s->nb_globals);
> +break;
> }
Hmm, why this switch doesn't have:
default:
g_assert_not_reached();
like the other ones?
>>>
>>> ... then all switch should have a default case, as noticed Aleksandar.
>>
>> There's a bit of a conflict between wanting to use -Werror -Wswitch, and
>> making
>> sure every switch has a default.
>>
>> With the former, you get a compiler error of the form
>>
>> error: enumeration value ‘FOO’ not handled in switch
>>
>> which lets you easily find places that need adjustment enumerators are added.
>
> FYI, -Wswitch-enum can deal with this. This gives a warning about
> missing enum cases, even if there is a default statement:
>
> [quote]
> '-Wswitch-enum'
> Warn whenever a 'switch' statement has an index of enumerated type
> and lacks a 'case' for one or more of the named codes of that
> enumeration. 'case' labels outside the enumeration range also
> provoke warnings when this option is used. The only difference
> between '-Wswitch' and this option is that this option gives a
> warning about an omitted enumeration code even if there is a
> 'default' label.
This warning, IMO, is useless.
All you need is one enumeration with 100 elements -- e.g. TCGOp -- and you
certainly will not want to have to add all enumerators to every switch.
r~
Re: [PATCH RESEND v6 36/36] multi-process: add configure and usage information
On Thu, Apr 23, 2020 at 11:03 PM Jag Raman wrote: > > > > On Apr 23, 2020, at 9:54 AM, 罗勇刚(Yonggang Luo) > wrote: > > > > Does multi-process support on Windows? > > I found it use mmap and unix socket for inter-process communication, > that may not support under Windows. > > Hi Yonggang, > > We have only tested this on Linux till now. Are you using QEMU with > Windows? > Yeap, I am using QEMU with windows. > > > And also, can the python script be replaced by C implementation? > > The functionality in the python script would eventually move to libvirt. > The python > script is a temporary measure. > > Does that means without libvirt, the QEMU can not be called directly? > Thank you very much! > — > Jag > > > > > On Thu, Apr 23, 2020 at 12:38 PM wrote: > > From: Elena Ufimtseva > > > > Signed-off-by: Elena Ufimtseva > > Signed-off-by: Jagannathan Raman > > Signed-off-by: John G Johnson > > --- > > MAINTAINERS | 2 + > > docs/multi-process.rst | 85 + > > scripts/mpqemu-launcher-perf-mode.py | 92 > > scripts/mpqemu-launcher.py | 53 > > 4 files changed, 232 insertions(+) > > create mode 100644 docs/multi-process.rst > > create mode 100755 scripts/mpqemu-launcher-perf-mode.py > > create mode 100755 scripts/mpqemu-launcher.py > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index ed48615e15..8ff3bfae6a 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -2880,6 +2880,8 @@ F: remote/iohub.c > > F: remote/remote-opts.h > > F: remote/remote-opts.c > > F: docs/devel/multi-process.rst > > +F: scripts/mpqemu-launcher.py > > +F: scripts/mpqemu-launcher-perf-mode.py > > > > Build and test automation > > - > > diff --git a/docs/multi-process.rst b/docs/multi-process.rst > > new file mode 100644 > > index 00..8387d6c691 > > --- /dev/null > > +++ b/docs/multi-process.rst > > @@ -0,0 +1,85 @@ > > +Multi-process QEMU > > +== > > + > > +This document describes how to configure and use multi-process qemu. > > +For the design document refer to docs/devel/qemu-multiprocess. > > + > > +1) Configuration > > + > > + > > +To enable support for multi-process add --enable-mpqemu > > +to the list of options for the "configure" script. > > + > > + > > +2) Usage > > + > > + > > +Multi-process QEMU requires an orchestrator to launch. Please refer to a > > +light-weight python based orchestrator for mpqemu in > > +scripts/mpqemu-launcher.py to lauch QEMU in multi-process mode. > > + > > +scripts/mpqemu-launcher-perf-mode.py launches in "perf" mode. In this > mode, > > +the same QEMU process connects to multiple remote devices, each > emulated in > > +a separate process. > > + > > +As of now, we only support the emulation of lsi53c895a in a separate > process. > > + > > +Following is a description of command-line used to launch mpqemu. > > + > > +* Orchestrator: > > + > > + - The Orchestrator creates a unix socketpair > > + > > + - It launches the remote process and passes one of the > > +sockets to it via command-line. > > + > > + - It then launches QEMU and specifies the other socket as an option > > +to the Proxy device object > > + > > +* Remote Process: > > + > > + - The first command-line option in the remote process is one of the > > +sockets created by the Orchestrator > > + > > + - The remaining options are no different from how one launches QEMU > with > > +devices. The only other requirement is each PCI device must have a > > +unique ID specified to it. This is needed to pair remote device > with the > > +Proxy object. > > + > > + - Example command-line for the remote process is as follows: > > + > > + /usr/bin/qemu-scsu-dev 4 > \ > > + -device lsi53c895a,id=lsi0 > \ > > + -drive id=drive_image2,file=/build/ol7-nvme-test-1.qcow2 > \ > > + -device scsi-hd,id=drive2,drive=drive_image2,bus=lsi0.0,scsi-id=0 > > + > > +* QEMU: > > + > > + - Since parts of the RAM are shared between QEMU & remote process, a > > +memory-backend-memfd is required to facilitate this, as follows: > > + > > +-object memory-backend-memfd,id=mem,size=2G > > + > > + - A "pci-proxy-dev" device is created for each of the PCI devices > emulated > > +in the remote process. A "socket" sub-option specifies the other > end of > > +unix channel created by orchestrator. The "id" sub-option must be > specified > > +and should be the same as the "id" specified for the remote PCI > device > > + > > + - Example commandline for QEMU is as follows: > > + > > + -device pci-proxy-dev,id=lsi0,socket=3 > > + > > +* Monitor / QMP: > > + > > + - The remote process supports QEMU monitor. It could be specified > using the > > +"-monitor" or "-qmp" command-line options > > + > > + - As an example, one could connect to the monitor by adding the > following > > +to the
[PATCH RFC 2/3] target/arm: Implement SVE2 AESE, AESD, SM4E
Signed-off-by: Stephen Long
---
target/arm/cpu.h | 5 +
target/arm/helper-sve.h| 4
target/arm/sve.decode | 6 ++
target/arm/sve_helper.c| 25 +
target/arm/translate-sve.c | 16
5 files changed, 56 insertions(+)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index b7c7946771..4dda0cf6c1 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -3870,6 +3870,11 @@ static inline bool isar_feature_aa64_sve2_bitperm(const
ARMISARegisters *id)
return FIELD_EX64(id->id_aa64zfr0, ID_AA64ZFR0, BITPERM) != 0;
}
+static inline bool isar_feature_aa64_sve2_sm4(const ARMISARegisters *id)
+{
+return FIELD_EX64(id->id_aa64zfr0, ID_AA64ZFR0, SM4) != 0;
+}
+
/*
* Feature tests for "does this exist in either 32-bit or 64-bit?"
*/
diff --git a/target/arm/helper-sve.h b/target/arm/helper-sve.h
index 6e8421991c..3da9590da5 100644
--- a/target/arm/helper-sve.h
+++ b/target/arm/helper-sve.h
@@ -2690,3 +2690,7 @@ DEF_HELPER_FLAGS_5(sve2_sqrdcmlah__d, TCG_CALL_NO_RWG,
DEF_HELPER_FLAGS_3(sve2_aesmc, TCG_CALL_NO_RWG, void, ptr, ptr, i32)
DEF_HELPER_FLAGS_3(sve2_aesimc, TCG_CALL_NO_RWG, void, ptr, ptr, i32)
+
+DEF_HELPER_FLAGS_3(sve2_aese, TCG_CALL_NO_RWG, void, ptr, ptr, i32)
+DEF_HELPER_FLAGS_3(sve2_aesd, TCG_CALL_NO_RWG, void, ptr, ptr, i32)
+DEF_HELPER_FLAGS_3(sve2_sm4e, TCG_CALL_NO_RWG, void, ptr, ptr, i32)
diff --git a/target/arm/sve.decode b/target/arm/sve.decode
index a83420e690..4bbf219cb6 100644
--- a/target/arm/sve.decode
+++ b/target/arm/sve.decode
@@ -98,6 +98,7 @@
# Two operand with unused vector element size
@pd_pn_e0 ... rn:4 . rd:4 _esz esz=0
+@pd5_pn5_e0 .. rn:5 rd:5 _esz esz=0
# Two operand
@pd_pn esz:2 .. ... rn:4 . rd:4 _esz
@@ -1397,3 +1398,8 @@ SQRDCMLAH_ 01000100 esz:2 0 rm:5 0011 rot:2 rn:5
rd:5 ra=%reg_movprfx
## SVE2 crypto unary operations
AESMC 01000101 00 1011100 0 0 . @rdn_e0
AESIMC 01000101 00 1011100 1 0 . @rdn_e0
+
+## SVE2 crpyto destructive binary operations
+AESE01000101 00 10001 0 11100 0 . . @pd5_pn5_e0
+AESD01000101 00 10001 0 11100 1 . . @pd5_pn5_e0
+SM4E01000101 00 10001 1 11100 0 . . @pd5_pn5_e0
diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
index f25bb5338d..0581f33fc7 100644
--- a/target/arm/sve_helper.c
+++ b/target/arm/sve_helper.c
@@ -7441,3 +7441,28 @@ void HELPER(NAME)(void *vd, void *vn, uint32_t desc)
\
DO_CRYPTO_AESMC(sve2_aesmc, 0);
DO_CRYPTO_AESMC(sve2_aesimc, 1);
+
+#define DO_CRYPTO_AESE(NAME, DECRYPT) \
+void HELPER(NAME)(void *vd, void *vn, uint32_t desc)\
+{ \
+intptr_t i, opr_sz = simd_oprsz(desc); \
+void *d = vd, *n = vn; \
+for (i = 0; i < opr_sz; i += 16) { \
+HELPER(crypto_aese)(vd + i, vn + i, DECRYPT); \
+}
+}
+
+DO_CRYPTO_AESE(sve2_aese, 0);
+DO_CRYPTO_AESE(sve2_aesd, 1);
+
+#undef DO_CRYPTO_AESE
+#undef DO_CRYPTO_AESMC
+
+void HELPER(sve2_sm4e)(void *vd, void *vn, uint32_t desc)
+{
+intptr_t i, opr_sz = simd_oprsz(desc);
+void *d = vd, *n = vn;
+for (i = 0; i < opr_sz; i += 16) {
+HELPER(crypto_sm4e)(vd + i, vn + i);
+}
+}
diff --git a/target/arm/translate-sve.c b/target/arm/translate-sve.c
index 03463308ca..d991dcdb1c 100644
--- a/target/arm/translate-sve.c
+++ b/target/arm/translate-sve.c
@@ -7900,3 +7900,19 @@ static bool trans_##NAME(DisasContext *s, arg_rr_esz *a)
\
DO_SVE2_AES_CRYPTO(AESMC, aesmc)
DO_SVE2_AES_CRYPTO(AESIMC, aesimc)
+DO_SVE2_AES_CRYPTO(AESE, aese)
+DO_SVE2_AES_CRYPTO(AESD, aesd)
+
+static bool trans_SM4E(DisasContext *s, arg_rr_esz *a)
+{
+if (!dc_isar_feature(aa64_sve2_sm4, s)) {
+return false;
+}
+if (sve_access_check(s)) {
+unsigned vsz = vec_full_reg_size(s);
+tcg_gen_gvec_2_ool(vec_full_reg_offset(s, a->rd),
+ vec_full_reg_offset(s, a->rn),
+ vsz, vsz, 0, gen_helper_sve2_sm4e);
+}
+return true;
+}
--
2.17.1
[PATCH RFC 3/3] target/arm: Implement SVE2 SM4EKEY, RAX1
Signed-off-by: Stephen Long
---
target/arm/helper-sve.h| 3 +++
target/arm/sve.decode | 4
target/arm/sve_helper.c| 21 +
target/arm/translate-sve.c | 30 ++
4 files changed, 58 insertions(+)
diff --git a/target/arm/helper-sve.h b/target/arm/helper-sve.h
index 3da9590da5..07681728ba 100644
--- a/target/arm/helper-sve.h
+++ b/target/arm/helper-sve.h
@@ -2694,3 +2694,6 @@ DEF_HELPER_FLAGS_3(sve2_aesimc, TCG_CALL_NO_RWG, void,
ptr, ptr, i32)
DEF_HELPER_FLAGS_3(sve2_aese, TCG_CALL_NO_RWG, void, ptr, ptr, i32)
DEF_HELPER_FLAGS_3(sve2_aesd, TCG_CALL_NO_RWG, void, ptr, ptr, i32)
DEF_HELPER_FLAGS_3(sve2_sm4e, TCG_CALL_NO_RWG, void, ptr, ptr, i32)
+
+DEF_HELPER_FLAGS_4(sve2_sm4ekey, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_4(sve2_rax1, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
diff --git a/target/arm/sve.decode b/target/arm/sve.decode
index 4bbf219cb6..1e98c6aa2f 100644
--- a/target/arm/sve.decode
+++ b/target/arm/sve.decode
@@ -1403,3 +1403,7 @@ AESIMC 01000101 00 1011100 1 0 .
@rdn_e0
AESE01000101 00 10001 0 11100 0 . . @pd5_pn5_e0
AESD01000101 00 10001 0 11100 1 . . @pd5_pn5_e0
SM4E01000101 00 10001 1 11100 0 . . @pd5_pn5_e0
+
+## SVE2 crypto constructive binary operations
+SM4EKEY 01000101 00 1 . 0 0 . . @rd_rn_rm_e0
+RAX101000101 00 1 . 0 1 . . @rd_rn_rm_e0
diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
index 0581f33fc7..e8299b33ee 100644
--- a/target/arm/sve_helper.c
+++ b/target/arm/sve_helper.c
@@ -7466,3 +7466,24 @@ void HELPER(sve2_sm4e)(void *vd, void *vn, uint32_t desc)
HELPER(crypto_sm4e)(vd + i, vn + i);
}
}
+
+void HELPER(sve2_sm4ekey)(void *vd, void *vn, void *vm, uint32_t desc)
+{
+intptr_t i, opr_sz = simd_oprsz(desc);
+void *d = vd, *n = vn;
+for (i = 0; i < opr_sz; i += 16) {
+HELPER(crypto_sm4ekey)(vd + i, vn + i, vm + i);
+}
+}
+
+void HELPER(sve2_rax1)(void *vd, void *vn, void *vm, uint32_t desc)
+{
+intptr_t i, opr_sz = simd_oprsz(desc) / 8;
+uint64_t *d = vd, *n = vn, *m = vm;
+
+for (i = 0; i < opr_sz; ++i) {
+uint64_t nn = n[i];
+uint64_t mm = m[i];
+d[i] = nn ^ rol64(mm, 1);
+}
+}
diff --git a/target/arm/translate-sve.c b/target/arm/translate-sve.c
index d991dcdb1c..671f09efa7 100644
--- a/target/arm/translate-sve.c
+++ b/target/arm/translate-sve.c
@@ -7916,3 +7916,33 @@ static bool trans_SM4E(DisasContext *s, arg_rr_esz *a)
}
return true;
}
+
+static bool trans_SM4EKEY(DisasContext *s, arg_rrr_esz *a)
+{
+if (!dc_isar_feature(aa64_sve2_sm4, s)) {
+return false;
+}
+if (sve_access_check(s)) {
+unsigned vsz = vec_full_reg_size(s);
+tcg_gen_gvec_3_ool(vec_full_reg_offset(s, a->rd),
+ vec_full_reg_offset(s, a->rn),
+ vec_full_reg_offset(s, a->rm),
+ vsz, vsz, 0, gen_helper_sve2_sm4ekey);
+}
+return true;
+}
+
+static bool trans_RAX1(DisasContext *s, arg_rrr_esz *a)
+{
+if (!dc_isar_feature(aa64_sve2_sm4, s)) {
+return false;
+}
+if (sve_access_check(s)) {
+unsigned vsz = vec_full_reg_size(s);
+tcg_gen_gvec_3_ool(vec_full_reg_offset(s, a->rd),
+ vec_full_reg_offset(s, a->rn),
+ vec_full_reg_offset(s, a->rm),
+ vsz, vsz, 0, gen_helper_sve2_rax1);
+}
+return true;
+}
--
2.17.1
[PATCH RFC 0/3] target/arm: Implement SVE2 Crypto Extensions
Used the helper functions in crypto_helper.c to implement the helper functions for the crypto insns. Stephen Long (3): target/arm: Implement SVE2 AESMC, AESIMC target/arm: Implement SVE2 AESE, AESD, SM4E target/arm: Implement SVE2 SM4EKEY, RAX1 target/arm/cpu.h | 5 +++ target/arm/helper-sve.h| 10 ++ target/arm/sve.decode | 20 target/arm/sve_helper.c| 59 +++ target/arm/translate-sve.c | 64 ++ 5 files changed, 158 insertions(+) -- 2.17.1
[PATCH RFC 1/3] target/arm: Implement SVE2 AESMC, AESIMC
Signed-off-by: Stephen Long
---
target/arm/helper-sve.h| 3 +++
target/arm/sve.decode | 10 ++
target/arm/sve_helper.c| 13 +
target/arm/translate-sve.c | 18 ++
4 files changed, 44 insertions(+)
diff --git a/target/arm/helper-sve.h b/target/arm/helper-sve.h
index f6ae814021..6e8421991c 100644
--- a/target/arm/helper-sve.h
+++ b/target/arm/helper-sve.h
@@ -2687,3 +2687,6 @@ DEF_HELPER_FLAGS_5(sve2_sqrdcmlah__s, TCG_CALL_NO_RWG,
void, ptr, ptr, ptr, ptr, i32)
DEF_HELPER_FLAGS_5(sve2_sqrdcmlah__d, TCG_CALL_NO_RWG,
void, ptr, ptr, ptr, ptr, i32)
+
+DEF_HELPER_FLAGS_3(sve2_aesmc, TCG_CALL_NO_RWG, void, ptr, ptr, i32)
+DEF_HELPER_FLAGS_3(sve2_aesimc, TCG_CALL_NO_RWG, void, ptr, ptr, i32)
diff --git a/target/arm/sve.decode b/target/arm/sve.decode
index 3a2a4a7f1c..a83420e690 100644
--- a/target/arm/sve.decode
+++ b/target/arm/sve.decode
@@ -92,6 +92,10 @@
# Named instruction formats. These are generally used to
# reduce the amount of duplication between instruction patterns.
+# One operand with unused vector element size
+@rdn_e0 .. ... . . rd:5 \
+_esz rn=%reg_movprfx esz=0
+
# Two operand with unused vector element size
@pd_pn_e0 ... rn:4 . rd:4 _esz esz=0
@@ -1387,3 +1391,9 @@ UMLSLT_zzzw 01000100 .. 0 . 010 111 . .
@rda_rn_rm
CMLA_ 01000100 esz:2 0 rm:5 0010 rot:2 rn:5 rd:5 ra=%reg_movprfx
SQRDCMLAH_ 01000100 esz:2 0 rm:5 0011 rot:2 rn:5 rd:5 ra=%reg_movprfx
+
+ SVE2 Crypto Extensions
+
+## SVE2 crypto unary operations
+AESMC 01000101 00 1011100 0 0 . @rdn_e0
+AESIMC 01000101 00 1011100 1 0 . @rdn_e0
diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
index 55e2c32f03..f25bb5338d 100644
--- a/target/arm/sve_helper.c
+++ b/target/arm/sve_helper.c
@@ -7428,3 +7428,16 @@ void HELPER(sve2_xar_s)(void *vd, void *vn, void *vm,
uint32_t desc)
d[i] = ror32(n[i] ^ m[i], shr);
}
}
+
+#define DO_CRYPTO_AESMC(NAME, DECRYPT) \
+void HELPER(NAME)(void *vd, void *vn, uint32_t desc)\
+{ \
+intptr_t i, opr_sz = simd_oprsz(desc); \
+void *d = vd, *n = vn; \
+for (i = 0; i < opr_sz; i += 16) { \
+HELPER(crypto_aesmc)(vd + i, vn + i, DECRYPT); \
+}
+}
+
+DO_CRYPTO_AESMC(sve2_aesmc, 0);
+DO_CRYPTO_AESMC(sve2_aesimc, 1);
diff --git a/target/arm/translate-sve.c b/target/arm/translate-sve.c
index 20eb588cb3..03463308ca 100644
--- a/target/arm/translate-sve.c
+++ b/target/arm/translate-sve.c
@@ -7882,3 +7882,21 @@ static bool trans_SQRDCMLAH_(DisasContext *s,
arg_CMLA_ *a)
};
return do_sve2__fn(s, a->rd, a->rn, a->rm, a->ra, fns[a->esz], a->rot);
}
+
+#define DO_SVE2_AES_CRYPTO(NAME, name) \
+static bool trans_##NAME(DisasContext *s, arg_rr_esz *a)\
+{ \
+if (!dc_isar_feature(aa64_sve2_aes, s)) { \
+return false; \
+} \
+if (sve_access_check(s)) { \
+unsigned vsz = vec_full_reg_size(s);\
+tcg_gen_gvec_2_ool(vec_full_reg_offset(s, a->rd), \
+ vec_full_reg_offset(s, a->rn), \
+ vsz, vsz, 0, gen_helper_sve2_##name);\
+} \
+return true;\
+}
+
+DO_SVE2_AES_CRYPTO(AESMC, aesmc)
+DO_SVE2_AES_CRYPTO(AESIMC, aesimc)
--
2.17.1
Re: [PATCH v2 0/3] qcow2: Allow resize of images with internal snapshots
Patchew URL: https://patchew.org/QEMU/20200423221707.477404-1-ebl...@redhat.com/ Hi, This series failed the docker-mingw@fedora 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 export ARCH=x86_64 make docker-image-fedora V=1 NETWORK=1 time make docker-test-mingw@fedora J=14 NETWORK=1 === TEST SCRIPT END === CC block/blklogwrites.o CC block/block-backend.o /tmp/qemu-test/src/block/qcow.c: In function 'qcow_co_create': /tmp/qemu-test/src/block/qcow.c:933:12: error: 'ret' may be used uninitialized in this function [-Werror=maybe-uninitialized] return ret; ^~~ cc1: all warnings being treated as errors make: *** [/tmp/qemu-test/src/rules.mak:69: block/qcow.o] Error 1 make: *** Waiting for unfinished jobs /tmp/qemu-test/src/block/qcow2.c: In function 'qcow2_co_create': /tmp/qemu-test/src/block/qcow2.c:3574:12: error: 'ret' may be used uninitialized in this function [-Werror=maybe-uninitialized] return ret; ^~~ cc1: all warnings being treated as errors make: *** [/tmp/qemu-test/src/rules.mak:69: block/qcow2.o] Error 1 /tmp/qemu-test/src/block/parallels.c: In function 'parallels_co_create': /tmp/qemu-test/src/block/parallels.c:604:12: error: 'ret' may be used uninitialized in this function [-Werror=maybe-uninitialized] return ret; ^~~ cc1: all warnings being treated as errors make: *** [/tmp/qemu-test/src/rules.mak:69: block/parallels.o] Error 1 Traceback (most recent call last): File "./tests/docker/docker.py", line 664, in sys.exit(main()) --- raise CalledProcessError(retcode, cmd) subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=954b57f8849a4ba29a2929c47e5b334c', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-6j971jca/src/docker-src.2020-04-23-18.32.52.29764:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-mingw']' returned non-zero exit status 2. filter=--filter=label=com.qemu.instance.uuid=954b57f8849a4ba29a2929c47e5b334c make[1]: *** [docker-run] Error 1 make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-6j971jca/src' make: *** [docker-run-test-mingw@fedora] Error 2 real2m18.893s user0m8.916s The full log is available at http://patchew.org/logs/20200423221707.477404-1-ebl...@redhat.com/testing.docker-mingw@fedora/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [PATCH v2 0/3] qcow2: Allow resize of images with internal snapshots
Patchew URL: https://patchew.org/QEMU/20200423221707.477404-1-ebl...@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 === CC block/qed-check.o CC block/vhdx.o /tmp/qemu-test/src/block/qcow.c: In function 'qcow_co_create': /tmp/qemu-test/src/block/qcow.c:824:9: error: 'ret' may be used uninitialized in this function [-Werror=maybe-uninitialized] int ret; ^ cc1: all warnings being treated as errors make: *** [block/qcow.o] Error 1 make: *** Waiting for unfinished jobs /tmp/qemu-test/src/block/qcow2.c: In function 'qcow2_co_create': /tmp/qemu-test/src/block/qcow2.c:3288:9: error: 'ret' may be used uninitialized in this function [-Werror=maybe-uninitialized] int ret; ^ cc1: all warnings being treated as errors make: *** [block/qcow2.o] Error 1 Traceback (most recent call last): File "./tests/docker/docker.py", line 664, in sys.exit(main()) --- raise CalledProcessError(retcode, cmd) subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=712c8b139ba4499b8f42495e5e5ec257', '-u', '1001', '--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/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-e7ishnmm/src/docker-src.2020-04-23-18.29.54.23207:/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=712c8b139ba4499b8f42495e5e5ec257 make[1]: *** [docker-run] Error 1 make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-e7ishnmm/src' make: *** [docker-run-test-quick@centos7] Error 2 real2m8.365s user0m8.548s The full log is available at http://patchew.org/logs/20200423221707.477404-1-ebl...@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 v2 0/3] qcow2: Allow resize of images with internal snapshots
Patchew URL: https://patchew.org/QEMU/20200423221707.477404-1-ebl...@redhat.com/
Hi,
This series failed the asan 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
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===
CC block/qcow2-cluster.o
CC block/qcow2-snapshot.o
CC block/qcow2-cache.o
/tmp/qemu-test/src/block/qcow.c:854:9: error: variable 'ret' is used
uninitialized whenever 'if' condition is true
[-Werror,-Wsometimes-uninitialized]
if (!qcow_blk) {
^
/tmp/qemu-test/src/block/qcow.c:933:12: note: uninitialized use occurs here
---
^
= 0
1 error generated.
make: *** [/tmp/qemu-test/src/rules.mak:69: block/qcow.o] Error 1
make: *** Waiting for unfinished jobs
/tmp/qemu-test/src/block/qcow2.c:3409:9: error: variable 'ret' is used
uninitialized whenever 'if' condition is true
[-Werror,-Wsometimes-uninitialized]
if (!blk) {
^~~~
/tmp/qemu-test/src/block/qcow2.c:3574:12: note: uninitialized use occurs here
---
^
= 0
1 error generated.
make: *** [/tmp/qemu-test/src/rules.mak:69: block/qcow2.o] Error 1
Traceback (most recent call last):
File "./tests/docker/docker.py", line 664, in
sys.exit(main())
---
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run',
'--label', 'com.qemu.instance.uuid=dd087ef7353844bcb604dfa47c6b0963', '-u',
'1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e',
'TARGET_LIST=x86_64-softmmu', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e',
'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache',
'-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v',
'/var/tmp/patchew-tester-tmp-_0udd3nv/src/docker-src.2020-04-23-18.24.05.17686:/var/tmp/qemu:z,ro',
'qemu:fedora', '/var/tmp/qemu/run', 'test-debug']' returned non-zero exit
status 2.
filter=--filter=label=com.qemu.instance.uuid=dd087ef7353844bcb604dfa47c6b0963
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-_0udd3nv/src'
make: *** [docker-run-test-debug@fedora] Error 2
real3m59.908s
user0m8.451s
The full log is available at
http://patchew.org/logs/20200423221707.477404-1-ebl...@redhat.com/testing.asan/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com
[PATCH v2 2/3] qcow2: Allow resize of images with internal snapshots
We originally refused to allow resize of images with internal
snapshots because the v2 image format did not require the tracking of
snapshot size, making it impossible to safely revert to a snapshot
with a different size than the current view of the image. But the
snapshot size tracking was rectified in v3, and our recent fixes to
qemu-img amend (see 0a85af35) guarantee that we always have a valid
snapshot size. Thus, we no longer need to artificially limit image
resizes, but it does become one more thing that would prevent a
downgrade back to v2. And now that we support different-sized
snapshots, it's also easy to fix reverting to a snapshot to apply the
new size.
Upgrade iotest 61 to cover this (we previously had NO coverage of
refusal to resize while snapshots exist). Note that the amend process
can fail but still have effects: in particular, since we break things
into upgrade, resize, downgrade, a failure during resize does not roll
back changes made during upgrade, nor does failure in downgrade roll
back a resize. But this situation is pre-existing even without this
patch; and without journaling, the best we could do is minimize the
chance of partial failure by collecting all changes prior to doing any
writes - which adds a lot of complexity but could still fail with EIO.
On the other hand, we are careful that even if we have partial
modification but then fail, the image is left viable (that is, we are
careful to sequence things so that after each successful cluster
write, there may be transient leaked clusters but no corrupt
metadata). And complicating the code to make it more transaction-like
is not worth the effort: a user can always request multiple 'qemu-img
amend' changing one thing each, if they need finer-grained control
over detecting the first failure than what they get by letting qemu
decide how to sequence multiple changes.
Signed-off-by: Eric Blake
---
block/qcow2-snapshot.c | 20
block/qcow2.c | 25 ++---
tests/qemu-iotests/061 | 35 +++
tests/qemu-iotests/061.out | 28
4 files changed, 101 insertions(+), 7 deletions(-)
diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index 82c32d4c9b08..df16424fd952 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -23,6 +23,7 @@
*/
#include "qemu/osdep.h"
+#include "sysemu/block-backend.h"
#include "qapi/error.h"
#include "qcow2.h"
#include "qemu/bswap.h"
@@ -775,10 +776,21 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char
*snapshot_id)
}
if (sn->disk_size != bs->total_sectors * BDRV_SECTOR_SIZE) {
-error_report("qcow2: Loading snapshots with different disk "
-"size is not implemented");
-ret = -ENOTSUP;
-goto fail;
+BlockBackend *blk = blk_new_with_bs(bs, BLK_PERM_RESIZE, BLK_PERM_ALL,
+_err);
+if (!blk) {
+error_report_err(local_err);
+ret = -ENOTSUP;
+goto fail;
+}
+
+ret = blk_truncate(blk, sn->disk_size, true, PREALLOC_MODE_OFF,
+ _err);
+blk_unref(blk);
+if (ret < 0) {
+error_report_err(local_err);
+goto fail;
+}
}
/*
diff --git a/block/qcow2.c b/block/qcow2.c
index 1ce72041978b..34888a793354 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3987,9 +3987,12 @@ static int coroutine_fn
qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
qemu_co_mutex_lock(>lock);
-/* cannot proceed if image has snapshots */
-if (s->nb_snapshots) {
-error_setg(errp, "Can't resize an image which has snapshots");
+/*
+ * Even though we store snapshot size for all images, it was not
+ * required until v3, so it is not safe to proceed for v2.
+ */
+if (s->nb_snapshots && s->qcow_version < 3) {
+error_setg(errp, "Can't resize a v2 image which has snapshots");
ret = -ENOTSUP;
goto fail;
}
@@ -4951,6 +4954,7 @@ static int qcow2_downgrade(BlockDriverState *bs, int
target_version,
BDRVQcow2State *s = bs->opaque;
int current_version = s->qcow_version;
int ret;
+int i;
/* This is qcow2_downgrade(), not qcow2_upgrade() */
assert(target_version < current_version);
@@ -4968,6 +4972,21 @@ static int qcow2_downgrade(BlockDriverState *bs, int
target_version,
return -ENOTSUP;
}
+/*
+ * If any internal snapshot has a different size than the current
+ * image size, or VM state size that exceeds 32 bits, downgrading
+ * is unsafe. Even though we would still use v3-compliant output
+ * to preserve that data, other v2 programs might not realize
+ * those optional fields are important.
+ */
+for (i = 0; i < s->nb_snapshots; i++) {
+if (s->snapshots[i].vm_state_size > UINT32_MAX ||
+
[PATCH v2 1/3] block: Add blk_new_with_bs() helper
There are several callers that need to create a new block backend from
an existing BDS; make the task slightly easier with a common helper
routine.
Suggested-by: Max Reitz
Signed-off-by: Eric Blake
---
include/sysemu/block-backend.h | 2 ++
block/block-backend.c | 23 +++
block/crypto.c | 8 +++-
block/parallels.c | 7 +++
block/qcow.c | 7 +++
block/qcow2.c | 15 ++-
block/qed.c| 7 +++
block/sheepdog.c | 9 -
block/vdi.c| 7 +++
block/vhdx.c | 7 +++
block/vmdk.c | 9 -
block/vpc.c| 7 +++
blockdev.c | 8 +++-
blockjob.c | 7 ++-
14 files changed, 65 insertions(+), 58 deletions(-)
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 9bbdbd63d743..d37c1244dd9c 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -77,6 +77,8 @@ typedef struct BlockBackendPublic {
} BlockBackendPublic;
BlockBackend *blk_new(AioContext *ctx, uint64_t perm, uint64_t shared_perm);
+BlockBackend *blk_new_with_bs(BlockDriverState *bs, uint64_t perm,
+ uint64_t shared_perm, Error **errp);
BlockBackend *blk_new_open(const char *filename, const char *reference,
QDict *options, int flags, Error **errp);
int blk_get_refcnt(BlockBackend *blk);
diff --git a/block/block-backend.c b/block/block-backend.c
index 38ae41382652..1e082ce2a0f4 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -355,6 +355,29 @@ BlockBackend *blk_new(AioContext *ctx, uint64_t perm,
uint64_t shared_perm)
return blk;
}
+/*
+ * Create a new BlockBackend connected to an existing BlockDriverState.
+ *
+ * @perm is a bitmasks of BLK_PERM_* constants which describes the permissions
+ * to request for a block driver node that is attached to this BlockBackend.
+ * @shared_perm is a bitmask which describes which permissions may be granted
+ * to other users of the attached node.
+ * Both sets of permissions can be changed later using blk_set_perm().
+ *
+ * Return the new BlockBackend on success, null on failure.
+ */
+BlockBackend *blk_new_with_bs(BlockDriverState *bs, uint64_t perm,
+ uint64_t shared_perm, Error **errp)
+{
+BlockBackend *blk = blk_new(bdrv_get_aio_context(bs), perm, shared_perm);
+
+if (blk_insert_bs(blk, bs, errp) < 0) {
+blk_unref(blk);
+return NULL;
+}
+return blk;
+}
+
/*
* Creates a new BlockBackend, opens a new BlockDriverState, and connects both.
* The new BlockBackend is in the main AioContext.
diff --git a/block/crypto.c b/block/crypto.c
index d577f89659fa..1cb8ae17ffde 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -261,11 +261,9 @@ static int block_crypto_co_create_generic(BlockDriverState
*bs,
QCryptoBlock *crypto = NULL;
struct BlockCryptoCreateData data;
-blk = blk_new(bdrv_get_aio_context(bs),
- BLK_PERM_WRITE | BLK_PERM_RESIZE, BLK_PERM_ALL);
-
-ret = blk_insert_bs(blk, bs, errp);
-if (ret < 0) {
+blk = blk_new_with_bs(bs, BLK_PERM_WRITE | BLK_PERM_RESIZE, BLK_PERM_ALL,
+ errp);
+if (!blk) {
goto cleanup;
}
diff --git a/block/parallels.c b/block/parallels.c
index 6d4ed77f165f..4019557ceee3 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -559,10 +559,9 @@ static int coroutine_fn
parallels_co_create(BlockdevCreateOptions* opts,
return -EIO;
}
-blk = blk_new(bdrv_get_aio_context(bs),
- BLK_PERM_WRITE | BLK_PERM_RESIZE, BLK_PERM_ALL);
-ret = blk_insert_bs(blk, bs, errp);
-if (ret < 0) {
+blk = blk_new_with_bs(bs, BLK_PERM_WRITE | BLK_PERM_RESIZE, BLK_PERM_ALL,
+ errp);
+if (!blk) {
goto out;
}
blk_set_allow_write_beyond_eof(blk, true);
diff --git a/block/qcow.c b/block/qcow.c
index 8973e4e565a1..d9f26c515a49 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -849,10 +849,9 @@ static int coroutine_fn
qcow_co_create(BlockdevCreateOptions *opts,
return -EIO;
}
-qcow_blk = blk_new(bdrv_get_aio_context(bs),
- BLK_PERM_WRITE | BLK_PERM_RESIZE, BLK_PERM_ALL);
-ret = blk_insert_bs(qcow_blk, bs, errp);
-if (ret < 0) {
+qcow_blk = blk_new_with_bs(bs, BLK_PERM_WRITE | BLK_PERM_RESIZE,
+ BLK_PERM_ALL, errp);
+if (!qcow_blk) {
goto exit;
}
blk_set_allow_write_beyond_eof(qcow_blk, true);
diff --git a/block/qcow2.c b/block/qcow2.c
index b524b0c53f84..1ce72041978b 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3404,10 +3404,9 @@ qcow2_co_create(BlockdevCreateOptions *create_options,
Error **errp)
}
/* Create
[PATCH v2 0/3] qcow2: Allow resize of images with internal snapshots
In v2: - new patch 1 [Max] - split off and reword unrelated change into patch 3 [Max] - improve the test: grep for items of interest, check $? [Max] - improve commit message explaining partial failure [Max] Eric Blake (3): block: Add blk_new_with_bs() helper qcow2: Allow resize of images with internal snapshots qcow2: Tweak comment about bitmaps vs. resize include/sysemu/block-backend.h | 2 ++ block/block-backend.c | 23 +++ block/crypto.c | 8 +++ block/parallels.c | 7 +++--- block/qcow.c | 7 +++--- block/qcow2-snapshot.c | 20 block/qcow2.c | 42 +++--- block/qed.c| 7 +++--- block/sheepdog.c | 9 block/vdi.c| 7 +++--- block/vhdx.c | 7 +++--- block/vmdk.c | 9 block/vpc.c| 7 +++--- blockdev.c | 8 +++ blockjob.c | 7 ++ tests/qemu-iotests/061 | 35 tests/qemu-iotests/061.out | 28 +++ 17 files changed, 167 insertions(+), 66 deletions(-) -- 2.26.2
[PATCH v2 3/3] qcow2: Tweak comment about bitmaps vs. resize
Our comment did not actually match the code. Rewrite the comment to
be less sensitive to any future changes to qcow2-bitmap.c that might
implement scenarios that we currently reject.
Signed-off-by: Eric Blake
---
block/qcow2.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index 34888a793354..6b6d1c3fa8b9 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3997,7 +3997,7 @@ static int coroutine_fn
qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
goto fail;
}
-/* cannot proceed if image has bitmaps */
+/* See qcow2-bitmap.c for which bitmap scenarios prevent a resize. */
if (qcow2_truncate_bitmaps_check(bs, errp)) {
ret = -ENOTSUP;
goto fail;
--
2.26.2
[Bug 1874539] Re: tulip driver broken in v5.0.0-rc4
Commit 8ffb7265af does make the code safer, but broke the device model. Instead of setting the error bits when the frame length is incorrect (too big), it simply discards it. The guest is not notified of the error and keeps waiting. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1874539 Title: tulip driver broken in v5.0.0-rc4 Status in QEMU: New Bug description: In a qemu-system-hppa system, qemu release v5.0.0-rc, the tulip nic driver is broken. The tulip nic is detected, even getting DHCP info does work. But when trying to download bigger files via network, the tulip driver gets stuck. For example when trying to download a 100MB file: root@debian:~# wget https://speed.hetzner.de/100MB.bin --2020-04-23 20:26:43-- https://speed.hetzner.de/100MB.bin Resolving speed.hetzner.de (speed.hetzner.de)... 88.198.248.254, 2a01:4f8:0:59ed::2 Connecting to speed.hetzner.de (speed.hetzner.de)|88.198.248.254|:443... connected. When reverting this commit, everything works again: commit 8ffb7265af64ec81748335ec8f20e7ab542c3850 Author: Prasad J Pandit Date: Tue Mar 24 22:57:22 2020 +0530 PATCH: net: tulip: check frame size and r/w data length To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1874539/+subscriptions
Re: [PATCH 4/7] chardev: Reduce "char-mux.h" scope, rename it "chardev-internal.h"
On Thu, Apr 23, 2020 at 10:24 PM Philippe Mathieu-Daudé
wrote:
>
> No file out of chardev/ requires access to this header,
> restrict its scope.
>
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Marc-André Lureau
> ---
> include/chardev/char-mux.h => chardev/chardev-internal.h | 7 ---
> chardev/char-fe.c| 2 +-
> chardev/char-mux.c | 2 +-
> chardev/char.c | 2 +-
> 4 files changed, 7 insertions(+), 6 deletions(-)
> rename include/chardev/char-mux.h => chardev/chardev-internal.h (96%)
>
> diff --git a/include/chardev/char-mux.h b/chardev/chardev-internal.h
> similarity index 96%
> rename from include/chardev/char-mux.h
> rename to chardev/chardev-internal.h
> index 417fe32eed..e0264ac349 100644
> --- a/include/chardev/char-mux.h
> +++ b/chardev/chardev-internal.h
> @@ -1,5 +1,5 @@
> /*
> - * QEMU System Emulator
> + * QEMU Character device internals
> *
> * Copyright (c) 2003-2008 Fabrice Bellard
> *
> @@ -21,8 +21,8 @@
> * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> * THE SOFTWARE.
> */
> -#ifndef CHAR_MUX_H
> -#define CHAR_MUX_H
> +#ifndef CHARDEV_INTERNAL_H
> +#define CHARDEV_INTERNAL_H
>
> #include "chardev/char.h"
> #include "chardev/char-fe.h"
> @@ -30,6 +30,7 @@
> #define MAX_MUX 4
> #define MUX_BUFFER_SIZE 32 /* Must be a power of 2. */
> #define MUX_BUFFER_MASK (MUX_BUFFER_SIZE - 1)
> +
> typedef struct MuxChardev {
> Chardev parent;
> CharBackend *backends[MAX_MUX];
> diff --git a/chardev/char-fe.c b/chardev/char-fe.c
> index f3530a90e6..474715c5a9 100644
> --- a/chardev/char-fe.c
> +++ b/chardev/char-fe.c
> @@ -29,7 +29,7 @@
>
> #include "chardev/char-fe.h"
> #include "chardev/char-io.h"
> -#include "chardev/char-mux.h"
> +#include "chardev-internal.h"
>
> int qemu_chr_fe_write(CharBackend *be, const uint8_t *buf, int len)
> {
> diff --git a/chardev/char-mux.c b/chardev/char-mux.c
> index 46c44af67c..6f980bb836 100644
> --- a/chardev/char-mux.c
> +++ b/chardev/char-mux.c
> @@ -29,7 +29,7 @@
> #include "chardev/char.h"
> #include "sysemu/block-backend.h"
> #include "sysemu/sysemu.h"
> -#include "chardev/char-mux.h"
> +#include "chardev-internal.h"
>
> /* MUX driver for serial I/O splitting */
>
> diff --git a/chardev/char.c b/chardev/char.c
> index e77564060d..b672a41150 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -39,7 +39,7 @@
> #include "qemu/option.h"
> #include "qemu/id.h"
>
> -#include "chardev/char-mux.h"
> +#include "chardev-internal.h"
>
> /***/
> /* character device */
> --
> 2.21.1
>
>
--
Marc-André Lureau
Re: [PATCH 5/7] chardev: Extract system emulation specific code
On Thu, Apr 23, 2020 at 10:24 PM Philippe Mathieu-Daudé
wrote:
>
> Split out code only used during system emulation,
> to reduce code pulled in user emulation and tools.
>
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Marc-André Lureau
> ---
> chardev/chardev-internal.h | 3 ++
> chardev/char.c | 35 +--
> chardev/chardev-sysemu.c | 69 ++
> chardev/Makefile.objs | 1 +
> 4 files changed, 74 insertions(+), 34 deletions(-)
> create mode 100644 chardev/chardev-sysemu.c
>
> diff --git a/chardev/chardev-internal.h b/chardev/chardev-internal.h
> index e0264ac349..f4d0429763 100644
> --- a/chardev/chardev-internal.h
> +++ b/chardev/chardev-internal.h
> @@ -26,6 +26,7 @@
>
> #include "chardev/char.h"
> #include "chardev/char-fe.h"
> +#include "qom/object.h"
>
> #define MAX_MUX 4
> #define MUX_BUFFER_SIZE 32 /* Must be a power of 2. */
> @@ -59,4 +60,6 @@ typedef struct MuxChardev {
> void mux_set_focus(Chardev *chr, int focus);
> void mux_chr_send_all_event(Chardev *chr, QEMUChrEvent event);
>
> +Object *get_chardevs_root(void);
> +
> #endif /* CHAR_MUX_H */
> diff --git a/chardev/char.c b/chardev/char.c
> index b672a41150..555bb0448e 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -44,7 +44,7 @@
> /***/
> /* character device */
>
> -static Object *get_chardevs_root(void)
> +Object *get_chardevs_root(void)
> {
> return container_get(object_get_root(), "/chardevs");
> }
> @@ -300,33 +300,6 @@ static const TypeInfo char_type_info = {
> .class_init = char_class_init,
> };
>
> -static int chardev_machine_done_notify_one(Object *child, void *opaque)
> -{
> -Chardev *chr = (Chardev *)child;
> -ChardevClass *class = CHARDEV_GET_CLASS(chr);
> -
> -if (class->chr_machine_done) {
> -return class->chr_machine_done(chr);
> -}
> -
> -return 0;
> -}
> -
> -static void chardev_machine_done_hook(Notifier *notifier, void *unused)
> -{
> -int ret = object_child_foreach(get_chardevs_root(),
> - chardev_machine_done_notify_one, NULL);
> -
> -if (ret) {
> -error_report("Failed to call chardev machine_done hooks");
> -exit(1);
> -}
> -}
> -
> -static Notifier chardev_machine_done_notify = {
> -.notify = chardev_machine_done_hook,
> -};
> -
> static bool qemu_chr_is_busy(Chardev *s)
> {
> if (CHARDEV_IS_MUX(s)) {
> @@ -1187,12 +1160,6 @@ void qemu_chr_cleanup(void)
> static void register_types(void)
> {
> type_register_static(_type_info);
> -
> -/* this must be done after machine init, since we register FEs with muxes
> - * as part of realize functions like serial_isa_realizefn when -nographic
> - * is specified
> - */
> -qemu_add_machine_init_done_notifier(_machine_done_notify);
> }
>
> type_init(register_types);
> diff --git a/chardev/chardev-sysemu.c b/chardev/chardev-sysemu.c
> new file mode 100644
> index 00..eecdc615ee
> --- /dev/null
> +++ b/chardev/chardev-sysemu.c
> @@ -0,0 +1,69 @@
> +/*
> + * QEMU System Emulator
> + *
> + * Copyright (c) 2003-2008 Fabrice Bellard
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> copy
> + * of this software and associated documentation files (the "Software"), to
> deal
> + * in the Software without restriction, including without limitation the
> rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "sysemu/sysemu.h"
> +#include "chardev/char.h"
> +#include "qemu/error-report.h"
> +#include "chardev-internal.h"
> +
> +static int chardev_machine_done_notify_one(Object *child, void *opaque)
> +{
> +Chardev *chr = (Chardev *)child;
> +ChardevClass *class = CHARDEV_GET_CLASS(chr);
> +
> +if (class->chr_machine_done) {
> +return class->chr_machine_done(chr);
> +}
> +
> +return 0;
> +}
> +
> +static void chardev_machine_done_hook(Notifier *notifier, void *unused)
> +{
> +int ret = object_child_foreach(get_chardevs_root(),
> +
Re: [PATCH 0/5] QEMU Gating CI
On 4/23/20 7:13 PM, Daniel P. Berrangé wrote: On Thu, Apr 23, 2020 at 01:04:13PM -0400, Cleber Rosa wrote: - Original Message - From: "Peter Maydell" To: "Markus Armbruster" Cc: "Fam Zheng" , "Thomas Huth" , "Beraldo Leal" , "Erik Skultety" , "Alex Bennée" , "Wainer Moschetta" , "QEMU Developers" , "Wainer dos Santos Moschetta" , "Willian Rampazzo" , "Cleber Rosa" , "Philippe Mathieu-Daudé" , "Eduardo Habkost" Sent: Tuesday, April 21, 2020 8:53:49 AM Subject: Re: [PATCH 0/5] QEMU Gating CI On Thu, 19 Mar 2020 at 16:33, Markus Armbruster wrote: Peter Maydell writes: I think we should start by getting the gitlab setup working for the basic "x86 configs" first. Then we can try adding a runner for s390 (that one's logistically easiest because it is a project machine, not one owned by me personally or by Linaro) once the basic framework is working, and expand from there. Makes sense to me. Next steps to get this off the ground: * Red Hat provides runner(s) for x86 stuff we care about. * If that doesn't cover 'basic "x86 configs" in your judgement, we fill the gaps as described below under "Expand from there". * Add an s390 runner using the project machine you mentioned. * Expand from there: identify the remaining gaps, map them to people / organizations interested in them, and solicit contributions from these guys. A note on contributions: we need both hardware and people. By people I mean maintainers for the infrastructure, the tools and all the runners. Cleber & team are willing to serve for the infrastructure, the tools and the Red Hat runners. So, with 5.0 nearly out the door it seems like a good time to check in on this thread again to ask where we are progress-wise with this. My impression is that this patchset provides most of the scripting and config side of the first step, so what we need is for RH to provide an x86 runner machine and tell the gitlab CI it exists. I appreciate that the whole coronavirus and working-from-home situation will have upended everybody's plans, especially when actual hardware might be involved, but how's it going ? Hi Peter, You hit the nail in the head here. We were affected indeed with our ability to move some machines from one lab to another (across the country), but we're actively working on it. For x86, do we really need to be using custom runners ? With GitLab if someone forks the repo to their personal namespace, they cannot use any custom runners setup by the origin project. So if we use custom runners for x86, people forking won't be able to run the GitLab CI jobs. As a sub-system maintainer I wouldn't like this, because I ideally want to be able to run the same jobs on my staging tree, that Peter will run at merge time for the PULL request I send. Thus my strong preference would be to use the GitLab runners in every scenario where they are viable to use. Only use custom runners in the cases where GitLab runners are clearly inadequate for our needs. Based on what we've setup in GitLab for libvirt, the shared runners they have work fine for x86. Just need the environments you are testing to be provided as Docker containers (you can actually build and cache the container images during your CI job too). IOW, any Linux distro build and test jobs should be able to use shared runners on x86, and likewise mingw builds. Custom runners should only be needed if the jobs need todo *BSD / macOS builds, and/or have access to specific hardware devices for some reason. Thanks to insist with that point Daniel. I'd rather see every configuration reproducible, so if we loose a hardware sponsor, we can find another one and start another runner. Also note, if it is not easy to reproduce a runner, it will be very hard to debug a reported build/test error. A non-reproducible runner can not be used as gating, because if they fail it is not acceptable to lock the project development process. In some cases custom runners are acceptable. These runners won't be "gating" but can post informative log and status. [*] Specific hardware that is not easily available. - Alistair at last KVM forum talked about a RISCV board (to test host TCG) - Aleksandar said at last KVM forum Wavecomp could plug a CI20 MIPS (to test host TCG) - Lemote seems interested to setup some Loongson MIPSr6 board (to test interaction with KVM) [*] To run code requiring accepting License Agreements [*] To run non Free / Open Source code Owner of these runners take the responsibility to provide enough time/information about reported bugs, or to debug them themselves. Now the problem is GitLab runner is not natively available on the architectures listed in this mail, so custom setup is required. A dumb script running ssh to a machine also works (tested) but lot of manual tuning/maintenance expected.
Re: [PATCH] elf_ops: Don't try to g_mapped_file_unref(NULL)
On Thu, 23 Apr 2020 at 21:25, Philippe Mathieu-Daudé wrote: > > On 4/23/20 10:20 PM, Peter Maydell wrote: > > This will fix the assertion; for the specific case of the generic > > loader it will then fall back from "guess this is an ELF file" to > > "maybe it's a uImage or a hex file" and eventually to "just load as > > a raw data file". > Reviewed-by: Philippe Mathieu-Daudé Thanks; as a side note "let me just load this as a raw data file" is not a very user-friendly way to report issues with the ELF file like "seems to be truncated" or "has no program headers". Not sure what to do about that... thanks -- PMM
Re: [PATCH 2/7] tests/test-char: Remove unused "chardev/char-mux.h" include
On Thu, Apr 23, 2020 at 10:22 PM Philippe Mathieu-Daudé wrote: > > This test never required "chardev/char-mux.h", remove it. > > Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Marc-André Lureau > --- > tests/test-char.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/tests/test-char.c b/tests/test-char.c > index 3afc9b1b8d..f08a39790e 100644 > --- a/tests/test-char.c > +++ b/tests/test-char.c > @@ -6,7 +6,6 @@ > #include "qemu/option.h" > #include "qemu/sockets.h" > #include "chardev/char-fe.h" > -#include "chardev/char-mux.h" > #include "sysemu/sysemu.h" > #include "qapi/error.h" > #include "qapi/qapi-commands-char.h" > -- > 2.21.1 > > -- Marc-André Lureau
Re: [PATCH 3/7] chardev: Restrict msmouse / wctablet / testdev to system emulation
On Thu, Apr 23, 2020 at 10:22 PM Philippe Mathieu-Daudé wrote: > > The msmouse / wctablet / testdev character devices are only > used by system emulation. Remove them from user mode and tools. > > Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Marc-André Lureau > --- > chardev/Makefile.objs | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/chardev/Makefile.objs b/chardev/Makefile.objs > index d68e1347f9..15ee7f47da 100644 > --- a/chardev/Makefile.objs > +++ b/chardev/Makefile.objs > @@ -17,7 +17,7 @@ chardev-obj-y += char-udp.o > chardev-obj-$(CONFIG_WIN32) += char-win.o > chardev-obj-$(CONFIG_WIN32) += char-win-stdio.o > > -common-obj-y += msmouse.o wctablet.o testdev.o > +common-obj-$(CONFIG_SOFTMMU) += msmouse.o wctablet.o testdev.o > common-obj-$(CONFIG_BRLAPI) += baum.o > baum.o-cflags := $(SDL_CFLAGS) > baum.o-libs := $(BRLAPI_LIBS) > -- > 2.21.1 > > -- Marc-André Lureau
Re: [PATCH 6/7] stubs: Split machine-init-done as machine-init and machine-notify
On Thu, Apr 23, 2020 at 10:22 PM Philippe Mathieu-Daudé
wrote:
>
> As the machine notify handlers are only used in system emulation,
> split the current file in two, and only build the notifier when
> system emulation is used.
>
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Marc-André Lureau
> ---
> stubs/machine-init.c| 4
> stubs/{machine-init-done.c => machine-notify.c} | 2 --
> stubs/Makefile.objs | 3 ++-
> 3 files changed, 6 insertions(+), 3 deletions(-)
> create mode 100644 stubs/machine-init.c
> rename stubs/{machine-init-done.c => machine-notify.c} (78%)
>
> diff --git a/stubs/machine-init.c b/stubs/machine-init.c
> new file mode 100644
> index 00..7622930ee0
> --- /dev/null
> +++ b/stubs/machine-init.c
> @@ -0,0 +1,4 @@
> +#include "qemu/osdep.h"
> +#include "sysemu/sysemu.h"
> +
> +bool machine_init_done = true;
> diff --git a/stubs/machine-init-done.c b/stubs/machine-notify.c
> similarity index 78%
> rename from stubs/machine-init-done.c
> rename to stubs/machine-notify.c
> index cd8e81392d..d164ecccb9 100644
> --- a/stubs/machine-init-done.c
> +++ b/stubs/machine-notify.c
> @@ -1,8 +1,6 @@
> #include "qemu/osdep.h"
> #include "sysemu/sysemu.h"
>
> -bool machine_init_done = true;
> -
> void qemu_add_machine_init_done_notifier(Notifier *notify)
> {
> }
> diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
> index 45be5dc0ed..765659a3f9 100644
> --- a/stubs/Makefile.objs
> +++ b/stubs/Makefile.objs
> @@ -15,7 +15,8 @@ stub-obj-y += iothread-lock.o
> stub-obj-y += is-daemonized.o
> stub-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
> stub-obj-$(CONFIG_LINUX_IO_URING) += io_uring.o
> -stub-obj-y += machine-init-done.o
> +stub-obj-y += machine-init.o
> +stub-obj-$(CONFIG_SOFTMMU) += machine-notify.o
> stub-obj-y += migr-blocker.o
> stub-obj-y += change-state-handler.o
> stub-obj-y += monitor.o
> --
> 2.21.1
>
>
--
Marc-André Lureau
Re: [PATCH 1/7] monitor/misc: Remove unused "chardev/char-mux.h" include
On Thu, Apr 23, 2020 at 10:23 PM Philippe Mathieu-Daudé wrote: > > monitor/misc.c never required "chardev/char-mux.h", remove it. > > Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Marc-André Lureau > --- > monitor/misc.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/monitor/misc.c b/monitor/misc.c > index 6c45fa490f..5d68026a7f 100644 > --- a/monitor/misc.c > +++ b/monitor/misc.c > @@ -33,7 +33,6 @@ > #include "exec/gdbstub.h" > #include "net/net.h" > #include "net/slirp.h" > -#include "chardev/char-mux.h" > #include "ui/qemu-spice.h" > #include "qemu/config-file.h" > #include "qemu/ctype.h" > -- > 2.21.1 > > -- Marc-André Lureau
Re: Need BT support in qemu for Zephyr
On 4/23/20 4:40 PM, Dan Christian wrote: > I'm new to trying to use qemu+bt (via btproxy) and haven't managed to > get anything working. > > btproxy is launched first, then Zephyr launches qemu via it's west > tool. The commands look like: > sudo tools/btproxy -u -i 0 -d # in a separate window > x86_64-pokysdk-linux/usr/bin/qemu-system-aarch64 -cpu cortex-a53 > -nographic -machine virt - > net none -pidfile qemu.pid -chardev stdio,id=con,mux=on -serial > chardev:con -mon chardev=con,mode=readline -serial > unix:/tmp/bt-server-bredr -kernel > /home/dchristian/share-fb/zephyrproject/zephy > r/build/zephyr/zephyr.elf > > It's failing with a device busy when btproxy tries to bind to the > adapter. I don't think this is a quemu issue. > > Zephyr packages it's own qemu: QEMU emulator version 4.2.0 (v4.2.0-dirty) If you just want to test/use BT, you might be luckier with an older version of QEMU supposed to work with BT, such QEMU v1.0. You won't have support for Cortex-A53 although.
[Bug 1874539] [NEW] tulip driver broken in v5.0.0-rc4
Public bug reported: In a qemu-system-hppa system, qemu release v5.0.0-rc, the tulip nic driver is broken. The tulip nic is detected, even getting DHCP info does work. But when trying to download bigger files via network, the tulip driver gets stuck. For example when trying to download a 100MB file: root@debian:~# wget https://speed.hetzner.de/100MB.bin --2020-04-23 20:26:43-- https://speed.hetzner.de/100MB.bin Resolving speed.hetzner.de (speed.hetzner.de)... 88.198.248.254, 2a01:4f8:0:59ed::2 Connecting to speed.hetzner.de (speed.hetzner.de)|88.198.248.254|:443... connected. When reverting this commit, everything works again: commit 8ffb7265af64ec81748335ec8f20e7ab542c3850 Author: Prasad J Pandit Date: Tue Mar 24 22:57:22 2020 +0530 PATCH: net: tulip: check frame size and r/w data length ** Affects: qemu Importance: Undecided Status: New -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1874539 Title: tulip driver broken in v5.0.0-rc4 Status in QEMU: New Bug description: In a qemu-system-hppa system, qemu release v5.0.0-rc, the tulip nic driver is broken. The tulip nic is detected, even getting DHCP info does work. But when trying to download bigger files via network, the tulip driver gets stuck. For example when trying to download a 100MB file: root@debian:~# wget https://speed.hetzner.de/100MB.bin --2020-04-23 20:26:43-- https://speed.hetzner.de/100MB.bin Resolving speed.hetzner.de (speed.hetzner.de)... 88.198.248.254, 2a01:4f8:0:59ed::2 Connecting to speed.hetzner.de (speed.hetzner.de)|88.198.248.254|:443... connected. When reverting this commit, everything works again: commit 8ffb7265af64ec81748335ec8f20e7ab542c3850 Author: Prasad J Pandit Date: Tue Mar 24 22:57:22 2020 +0530 PATCH: net: tulip: check frame size and r/w data length To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1874539/+subscriptions
Re: [PATCH v1 09/14] gdbstub: eliminate gdbserver_fd global
On 4/23/20 7:05 PM, Alex Bennée wrote:
We don't really need to track this fd beyond the initial creation of
the socket. We already know if the system has been initialised by
virtue of the gdbserver_state so lets remove it. This makes the later
re-factoring easier.
Signed-off-by: Alex Bennée
---
gdbstub.c | 23 ++-
1 file changed, 10 insertions(+), 13 deletions(-)
diff --git a/gdbstub.c b/gdbstub.c
index 171e150950..8c53cc0e1c 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -398,8 +398,6 @@ static void reset_gdbserver_state(void)
bool gdb_has_xml;
#ifdef CONFIG_USER_ONLY
-/* XXX: This is not thread safe. Do we care? */
-static int gdbserver_fd = -1;
static int get_char(void)
{
@@ -2964,7 +2962,7 @@ void gdb_exit(CPUArchState *env, int code)
return;
}
#ifdef CONFIG_USER_ONLY
- if (gdbserver_fd < 0 || gdbserver_state.fd < 0) {
+ if (gdbserver_state.fd < 0) {
return;
}
#endif
@@ -3011,7 +3009,7 @@ gdb_handlesig(CPUState *cpu, int sig)
char buf[256];
int n;
-if (gdbserver_fd < 0 || gdbserver_state.fd < 0) {
+if (!gdbserver_state.init || gdbserver_state.fd < 0) {
return sig;
}
@@ -3060,7 +3058,7 @@ void gdb_signalled(CPUArchState *env, int sig)
{
char buf[4];
-if (gdbserver_fd < 0 || gdbserver_state.fd < 0) {
+if (!gdbserver_state.init || gdbserver_state.fd < 0) {
return;
}
@@ -3068,7 +3066,7 @@ void gdb_signalled(CPUArchState *env, int sig)
put_packet(buf);
}
-static bool gdb_accept(void)
+static bool gdb_accept(int gdb_fd)
{
struct sockaddr_in sockaddr;
socklen_t len;
@@ -3076,7 +3074,7 @@ static bool gdb_accept(void)
for(;;) {
len = sizeof(sockaddr);
-fd = accept(gdbserver_fd, (struct sockaddr *), );
+fd = accept(gdb_fd, (struct sockaddr *), );
if (fd < 0 && errno != EINTR) {
perror("accept");
return false;
@@ -3137,13 +3135,12 @@ static int gdbserver_open(int port)
int gdbserver_start(int port)
{
-gdbserver_fd = gdbserver_open(port);
-if (gdbserver_fd < 0)
+int gdb_fd = gdbserver_open(port);
+if (gdb_fd < 0)
return -1;
/* accept connections */
-if (!gdb_accept()) {
-close(gdbserver_fd);
-gdbserver_fd = -1;
+if (!gdb_accept(gdb_fd)) {
+close(gdb_fd);
return -1;
}
return 0;
@@ -3152,7 +3149,7 @@ int gdbserver_start(int port)
/* Disable gdb stub for child processes. */
void gdbserver_fork(CPUState *cpu)
{
-if (gdbserver_fd < 0 || gdbserver_state.fd < 0) {
+if (!gdbserver_state.init || gdbserver_state.fd < 0) {
return;
}
close(gdbserver_state.fd);
Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v1 14/14] .travis.yml: drop MacOSX
On 4/23/20 7:05 PM, Alex Bennée wrote:
This keeps breaking on Travis so lets just fall back to the Cirrus CI
builds which seem to be better maintained. Fix up the comments while
we are doing this as we never had a windows build.
I certainly vouch this.
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Alex Bennée
---
.travis.yml | 28 +---
1 file changed, 1 insertion(+), 27 deletions(-)
diff --git a/.travis.yml b/.travis.yml
index a4c3c6c805..49267b73b3 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -9,9 +9,8 @@ compiler:
cache:
# There is one cache per branch and compiler version.
# characteristics of each job are used to identify the cache:
- # - OS name (currently, linux, osx, or windows)
+ # - OS name (currently only linux)
# - OS distribution (for Linux, xenial, trusty, or precise)
- # - macOS image name (e.g., xcode7.2)
# - Names and values of visible environment variables set in .travis.yml or
Settings panel
timeout: 1200
ccache: true
@@ -271,31 +270,6 @@ jobs:
- TEST_CMD=""
-# MacOSX builds - cirrus.yml also tests some MacOS builds including latest Xcode
-
-- name: "OSX Xcode 10.3"
- env:
-- BASE_CONFIG="--disable-docs --enable-tools"
--
CONFIG="--target-list=i386-softmmu,ppc-softmmu,ppc64-softmmu,m68k-softmmu,x86_64-softmmu"
- os: osx
- osx_image: xcode10.3
- compiler: clang
- addons:
-homebrew:
- packages:
-- ccache
-- glib
-- pixman
-- gnu-sed
-- python
- update: true
- before_script:
-- brew link --overwrite python
-- export PATH="/usr/local/opt/ccache/libexec:$PATH"
-- mkdir -p ${BUILD_DIR} && cd ${BUILD_DIR}
-- ${SRC_DIR}/configure ${BASE_CONFIG} ${CONFIG} || { cat config.log &&
exit 1; }
-
-
# Python builds
- name: "GCC Python 3.5 (x86_64-softmmu)"
env:
Re: [PATCH v1 05/14] configure: favour gdb-multiarch if we have it
On 4/23/20 7:05 PM, Alex Bennée wrote: As gdb will generally be talking to "foreign" guests lets use that if we can. Otherwise the chances of gdb barfing are considerably higher. Signed-off-by: Alex Bennée --- configure | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure b/configure index 23b5e93752..c58787100f 100755 --- a/configure +++ b/configure @@ -303,7 +303,7 @@ libs_qga="" debug_info="yes" stack_protector="" use_containers="yes" -gdb_bin=$(command -v "gdb") +gdb_bin=$(command -v "gdb-multiarch" || command -v "gdb") Good one! Reviewed-by: Philippe Mathieu-Daudé if test -e "$source_path/.git" then
Re: [PATCH v1 13/14] .travis.yml: show free disk space at end of run
On 4/23/20 7:05 PM, Alex Bennée wrote: Signed-off-by: Alex Bennée --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 2fd63eceaa..a4c3c6c805 100644 --- a/.travis.yml +++ b/.travis.yml @@ -113,6 +113,7 @@ script: $(exit $BUILD_RC); fi after_script: + - df -h Well, I suppose it helps to realize we didn't break the runner, it was already dying... Reviewed-by: Philippe Mathieu-Daudé - if command -v ccache ; then ccache --show-stats ; fi
Re: [PATCH] elf_ops: Don't try to g_mapped_file_unref(NULL)
On 4/23/20 10:20 PM, Peter Maydell wrote:
Calling g_mapped_file_unref() on a NULL pointer is not valid, and
glib will assert if you try it.
$ qemu-system-arm -M virt -display none -device loader,file=/tmp/bad.elf
qemu-system-arm: -device loader,file=/tmp/bad.elf: GLib: g_mapped_file_unref:
assertion 'file != NULL' failed
(One way to produce an ELF file that fails like this is to copy just
the first 16 bytes of a valid ELF file; this is sufficient to fool
the code in load_elf_ram_sym() into thinking it's an ELF file and
calling load_elf32() or load_elf64().)
The failure-exit path in load_elf can be reached from various points
in execution, and for some of those we haven't yet called
g_mapped_file_new_from_fd(). Add a condition to the unref call so we
only call it if we successfully created the GMappedFile to start with.
This will fix the assertion; for the specific case of the generic
loader it will then fall back from "guess this is an ELF file" to
"maybe it's a uImage or a hex file" and eventually to "just load as
a raw data file".
Reported-by: Randy Yates
Signed-off-by: Peter Maydell
Reviewed-by: Philippe Mathieu-Daudé
---
include/hw/elf_ops.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h
index e0bb47bb678..398a4a2c85b 100644
--- a/include/hw/elf_ops.h
+++ b/include/hw/elf_ops.h
@@ -606,7 +606,9 @@ static int glue(load_elf, SZ)(const char *name, int fd,
*highaddr = (uint64_t)(elf_sword)high;
ret = total_size;
fail:
-g_mapped_file_unref(mapped_file);
+if (mapped_file) {
+g_mapped_file_unref(mapped_file);
+}
g_free(phdr);
return ret;
}
[PATCH 5/7] chardev: Extract system emulation specific code
Split out code only used during system emulation,
to reduce code pulled in user emulation and tools.
Signed-off-by: Philippe Mathieu-Daudé
---
chardev/chardev-internal.h | 3 ++
chardev/char.c | 35 +--
chardev/chardev-sysemu.c | 69 ++
chardev/Makefile.objs | 1 +
4 files changed, 74 insertions(+), 34 deletions(-)
create mode 100644 chardev/chardev-sysemu.c
diff --git a/chardev/chardev-internal.h b/chardev/chardev-internal.h
index e0264ac349..f4d0429763 100644
--- a/chardev/chardev-internal.h
+++ b/chardev/chardev-internal.h
@@ -26,6 +26,7 @@
#include "chardev/char.h"
#include "chardev/char-fe.h"
+#include "qom/object.h"
#define MAX_MUX 4
#define MUX_BUFFER_SIZE 32 /* Must be a power of 2. */
@@ -59,4 +60,6 @@ typedef struct MuxChardev {
void mux_set_focus(Chardev *chr, int focus);
void mux_chr_send_all_event(Chardev *chr, QEMUChrEvent event);
+Object *get_chardevs_root(void);
+
#endif /* CHAR_MUX_H */
diff --git a/chardev/char.c b/chardev/char.c
index b672a41150..555bb0448e 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -44,7 +44,7 @@
/***/
/* character device */
-static Object *get_chardevs_root(void)
+Object *get_chardevs_root(void)
{
return container_get(object_get_root(), "/chardevs");
}
@@ -300,33 +300,6 @@ static const TypeInfo char_type_info = {
.class_init = char_class_init,
};
-static int chardev_machine_done_notify_one(Object *child, void *opaque)
-{
-Chardev *chr = (Chardev *)child;
-ChardevClass *class = CHARDEV_GET_CLASS(chr);
-
-if (class->chr_machine_done) {
-return class->chr_machine_done(chr);
-}
-
-return 0;
-}
-
-static void chardev_machine_done_hook(Notifier *notifier, void *unused)
-{
-int ret = object_child_foreach(get_chardevs_root(),
- chardev_machine_done_notify_one, NULL);
-
-if (ret) {
-error_report("Failed to call chardev machine_done hooks");
-exit(1);
-}
-}
-
-static Notifier chardev_machine_done_notify = {
-.notify = chardev_machine_done_hook,
-};
-
static bool qemu_chr_is_busy(Chardev *s)
{
if (CHARDEV_IS_MUX(s)) {
@@ -1187,12 +1160,6 @@ void qemu_chr_cleanup(void)
static void register_types(void)
{
type_register_static(_type_info);
-
-/* this must be done after machine init, since we register FEs with muxes
- * as part of realize functions like serial_isa_realizefn when -nographic
- * is specified
- */
-qemu_add_machine_init_done_notifier(_machine_done_notify);
}
type_init(register_types);
diff --git a/chardev/chardev-sysemu.c b/chardev/chardev-sysemu.c
new file mode 100644
index 00..eecdc615ee
--- /dev/null
+++ b/chardev/chardev-sysemu.c
@@ -0,0 +1,69 @@
+/*
+ * QEMU System Emulator
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "sysemu/sysemu.h"
+#include "chardev/char.h"
+#include "qemu/error-report.h"
+#include "chardev-internal.h"
+
+static int chardev_machine_done_notify_one(Object *child, void *opaque)
+{
+Chardev *chr = (Chardev *)child;
+ChardevClass *class = CHARDEV_GET_CLASS(chr);
+
+if (class->chr_machine_done) {
+return class->chr_machine_done(chr);
+}
+
+return 0;
+}
+
+static void chardev_machine_done_hook(Notifier *notifier, void *unused)
+{
+int ret = object_child_foreach(get_chardevs_root(),
+ chardev_machine_done_notify_one, NULL);
+
+if (ret) {
+error_report("Failed to call chardev machine_done hooks");
+exit(1);
+}
+}
+
+
+static Notifier chardev_machine_done_notify = {
+.notify = chardev_machine_done_hook,
+};
+
+static void register_types(void)
+{
+/*
+ * This must be done after machine init, since we register FEs with muxes
+
[PATCH 7/7] multi-process: Refactor machine_init and exit notifiers
From: Elena Ufimtseva
Relocate machine_int and exit notifiers into common code
Signed-off-by: Elena Ufimtseva
Signed-off-by: John G Johnson
Signed-off-by: Jagannathan Raman
[PMD: Removed NotifierList machine_init_done_notifiers stub]
Signed-off-by: Philippe Mathieu-Daudé
---
Makefile.objs | 1 +
include/sysemu/sysemu.h | 2 ++
softmmu/vl.c| 42 -
stubs/machine-notify.c | 4 +++
util/machine-notify.c | 69 +
MAINTAINERS | 1 +
6 files changed, 77 insertions(+), 42 deletions(-)
create mode 100644 util/machine-notify.c
diff --git a/Makefile.objs b/Makefile.objs
index a7c967633a..bfb9271862 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -79,6 +79,7 @@ qemu-seccomp.o-libs := $(SECCOMP_LIBS)
common-obj-$(CONFIG_FDT) += device_tree.o
common-obj-y += qapi/
+common-obj-y += util/machine-notify.o
endif # CONFIG_SOFTMMU
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index ef81302e1a..2438dd7bea 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -17,11 +17,13 @@ extern bool qemu_uuid_set;
void qemu_add_exit_notifier(Notifier *notify);
void qemu_remove_exit_notifier(Notifier *notify);
+void qemu_run_exit_notifiers(void);
extern bool machine_init_done;
void qemu_add_machine_init_done_notifier(Notifier *notify);
void qemu_remove_machine_init_done_notifier(Notifier *notify);
+void qemu_run_machine_init_done_notifiers(void);
extern int autostart;
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 32c0047889..39cbb6b50d 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -172,12 +172,6 @@ int icount_align_option;
QemuUUID qemu_uuid;
bool qemu_uuid_set;
-static NotifierList exit_notifiers =
-NOTIFIER_LIST_INITIALIZER(exit_notifiers);
-
-static NotifierList machine_init_done_notifiers =
-NOTIFIER_LIST_INITIALIZER(machine_init_done_notifiers);
-
bool xen_allowed;
uint32_t xen_domid;
enum xen_mode xen_mode = XEN_EMULATE;
@@ -2325,21 +2319,6 @@ static MachineClass *machine_parse(const char *name,
GSList *machines)
return mc;
}
-void qemu_add_exit_notifier(Notifier *notify)
-{
-notifier_list_add(_notifiers, notify);
-}
-
-void qemu_remove_exit_notifier(Notifier *notify)
-{
-notifier_remove(notify);
-}
-
-static void qemu_run_exit_notifiers(void)
-{
-notifier_list_notify(_notifiers, NULL);
-}
-
static const char *pid_file;
static Notifier qemu_unlink_pidfile_notifier;
@@ -2350,27 +2329,6 @@ static void qemu_unlink_pidfile(Notifier *n, void *data)
}
}
-bool machine_init_done;
-
-void qemu_add_machine_init_done_notifier(Notifier *notify)
-{
-notifier_list_add(_init_done_notifiers, notify);
-if (machine_init_done) {
-notify->notify(notify, NULL);
-}
-}
-
-void qemu_remove_machine_init_done_notifier(Notifier *notify)
-{
-notifier_remove(notify);
-}
-
-static void qemu_run_machine_init_done_notifiers(void)
-{
-machine_init_done = true;
-notifier_list_notify(_init_done_notifiers, NULL);
-}
-
static const QEMUOption *lookup_opt(int argc, char **argv,
const char **poptarg, int *poptind)
{
diff --git a/stubs/machine-notify.c b/stubs/machine-notify.c
index d164ecccb9..71eba45b0f 100644
--- a/stubs/machine-notify.c
+++ b/stubs/machine-notify.c
@@ -4,3 +4,7 @@
void qemu_add_machine_init_done_notifier(Notifier *notify)
{
}
+
+void qemu_remove_machine_init_done_notifier(Notifier *notify)
+{
+}
diff --git a/util/machine-notify.c b/util/machine-notify.c
new file mode 100644
index 00..718af79335
--- /dev/null
+++ b/util/machine-notify.c
@@ -0,0 +1,69 @@
+/*
+ * Machine notifiers.
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/notify.h"
+#include "sysemu/sysemu.h"
+
+static NotifierList machine_init_done_notifiers =
+
[PATCH 4/7] chardev: Reduce "char-mux.h" scope, rename it "chardev-internal.h"
No file out of chardev/ requires access to this header,
restrict its scope.
Signed-off-by: Philippe Mathieu-Daudé
---
include/chardev/char-mux.h => chardev/chardev-internal.h | 7 ---
chardev/char-fe.c| 2 +-
chardev/char-mux.c | 2 +-
chardev/char.c | 2 +-
4 files changed, 7 insertions(+), 6 deletions(-)
rename include/chardev/char-mux.h => chardev/chardev-internal.h (96%)
diff --git a/include/chardev/char-mux.h b/chardev/chardev-internal.h
similarity index 96%
rename from include/chardev/char-mux.h
rename to chardev/chardev-internal.h
index 417fe32eed..e0264ac349 100644
--- a/include/chardev/char-mux.h
+++ b/chardev/chardev-internal.h
@@ -1,5 +1,5 @@
/*
- * QEMU System Emulator
+ * QEMU Character device internals
*
* Copyright (c) 2003-2008 Fabrice Bellard
*
@@ -21,8 +21,8 @@
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/
-#ifndef CHAR_MUX_H
-#define CHAR_MUX_H
+#ifndef CHARDEV_INTERNAL_H
+#define CHARDEV_INTERNAL_H
#include "chardev/char.h"
#include "chardev/char-fe.h"
@@ -30,6 +30,7 @@
#define MAX_MUX 4
#define MUX_BUFFER_SIZE 32 /* Must be a power of 2. */
#define MUX_BUFFER_MASK (MUX_BUFFER_SIZE - 1)
+
typedef struct MuxChardev {
Chardev parent;
CharBackend *backends[MAX_MUX];
diff --git a/chardev/char-fe.c b/chardev/char-fe.c
index f3530a90e6..474715c5a9 100644
--- a/chardev/char-fe.c
+++ b/chardev/char-fe.c
@@ -29,7 +29,7 @@
#include "chardev/char-fe.h"
#include "chardev/char-io.h"
-#include "chardev/char-mux.h"
+#include "chardev-internal.h"
int qemu_chr_fe_write(CharBackend *be, const uint8_t *buf, int len)
{
diff --git a/chardev/char-mux.c b/chardev/char-mux.c
index 46c44af67c..6f980bb836 100644
--- a/chardev/char-mux.c
+++ b/chardev/char-mux.c
@@ -29,7 +29,7 @@
#include "chardev/char.h"
#include "sysemu/block-backend.h"
#include "sysemu/sysemu.h"
-#include "chardev/char-mux.h"
+#include "chardev-internal.h"
/* MUX driver for serial I/O splitting */
diff --git a/chardev/char.c b/chardev/char.c
index e77564060d..b672a41150 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -39,7 +39,7 @@
#include "qemu/option.h"
#include "qemu/id.h"
-#include "chardev/char-mux.h"
+#include "chardev-internal.h"
/***/
/* character device */
--
2.21.1
[PATCH 1/7] monitor/misc: Remove unused "chardev/char-mux.h" include
monitor/misc.c never required "chardev/char-mux.h", remove it. Signed-off-by: Philippe Mathieu-Daudé --- monitor/misc.c | 1 - 1 file changed, 1 deletion(-) diff --git a/monitor/misc.c b/monitor/misc.c index 6c45fa490f..5d68026a7f 100644 --- a/monitor/misc.c +++ b/monitor/misc.c @@ -33,7 +33,6 @@ #include "exec/gdbstub.h" #include "net/net.h" #include "net/slirp.h" -#include "chardev/char-mux.h" #include "ui/qemu-spice.h" #include "qemu/config-file.h" #include "qemu/ctype.h" -- 2.21.1
[PATCH 6/7] stubs: Split machine-init-done as machine-init and machine-notify
As the machine notify handlers are only used in system emulation,
split the current file in two, and only build the notifier when
system emulation is used.
Signed-off-by: Philippe Mathieu-Daudé
---
stubs/machine-init.c| 4
stubs/{machine-init-done.c => machine-notify.c} | 2 --
stubs/Makefile.objs | 3 ++-
3 files changed, 6 insertions(+), 3 deletions(-)
create mode 100644 stubs/machine-init.c
rename stubs/{machine-init-done.c => machine-notify.c} (78%)
diff --git a/stubs/machine-init.c b/stubs/machine-init.c
new file mode 100644
index 00..7622930ee0
--- /dev/null
+++ b/stubs/machine-init.c
@@ -0,0 +1,4 @@
+#include "qemu/osdep.h"
+#include "sysemu/sysemu.h"
+
+bool machine_init_done = true;
diff --git a/stubs/machine-init-done.c b/stubs/machine-notify.c
similarity index 78%
rename from stubs/machine-init-done.c
rename to stubs/machine-notify.c
index cd8e81392d..d164ecccb9 100644
--- a/stubs/machine-init-done.c
+++ b/stubs/machine-notify.c
@@ -1,8 +1,6 @@
#include "qemu/osdep.h"
#include "sysemu/sysemu.h"
-bool machine_init_done = true;
-
void qemu_add_machine_init_done_notifier(Notifier *notify)
{
}
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index 45be5dc0ed..765659a3f9 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -15,7 +15,8 @@ stub-obj-y += iothread-lock.o
stub-obj-y += is-daemonized.o
stub-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
stub-obj-$(CONFIG_LINUX_IO_URING) += io_uring.o
-stub-obj-y += machine-init-done.o
+stub-obj-y += machine-init.o
+stub-obj-$(CONFIG_SOFTMMU) += machine-notify.o
stub-obj-y += migr-blocker.o
stub-obj-y += change-state-handler.o
stub-obj-y += monitor.o
--
2.21.1
[PATCH 3/7] chardev: Restrict msmouse / wctablet / testdev to system emulation
The msmouse / wctablet / testdev character devices are only used by system emulation. Remove them from user mode and tools. Signed-off-by: Philippe Mathieu-Daudé --- chardev/Makefile.objs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/chardev/Makefile.objs b/chardev/Makefile.objs index d68e1347f9..15ee7f47da 100644 --- a/chardev/Makefile.objs +++ b/chardev/Makefile.objs @@ -17,7 +17,7 @@ chardev-obj-y += char-udp.o chardev-obj-$(CONFIG_WIN32) += char-win.o chardev-obj-$(CONFIG_WIN32) += char-win-stdio.o -common-obj-y += msmouse.o wctablet.o testdev.o +common-obj-$(CONFIG_SOFTMMU) += msmouse.o wctablet.o testdev.o common-obj-$(CONFIG_BRLAPI) += baum.o baum.o-cflags := $(SDL_CFLAGS) baum.o-libs := $(BRLAPI_LIBS) -- 2.21.1
[PATCH 2/7] tests/test-char: Remove unused "chardev/char-mux.h" include
This test never required "chardev/char-mux.h", remove it. Signed-off-by: Philippe Mathieu-Daudé --- tests/test-char.c | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test-char.c b/tests/test-char.c index 3afc9b1b8d..f08a39790e 100644 --- a/tests/test-char.c +++ b/tests/test-char.c @@ -6,7 +6,6 @@ #include "qemu/option.h" #include "qemu/sockets.h" #include "chardev/char-fe.h" -#include "chardev/char-mux.h" #include "sysemu/sysemu.h" #include "qapi/error.h" #include "qapi/qapi-commands-char.h" -- 2.21.1
[PATCH 0/7] chardev: Reduce system emulation specific code
chardev cleanup while reviewing 'Refactor machine_init and exit
notifiers' from the multi-process series:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg697510.html
Elena Ufimtseva (1):
multi-process: Refactor machine_init and exit notifiers
Philippe Mathieu-Daudé (6):
monitor/misc: Remove unused "chardev/char-mux.h" include
tests/test-char: Remove unused "chardev/char-mux.h" include
chardev: Restrict msmouse / wctablet / testdev to system emulation
chardev: Reduce "char-mux.h" scope, rename it "chardev-internal.h"
chardev: Extract system emulation specific code
stubs: Split machine-init-done as machine-init and machine-notify
Makefile.objs | 1 +
.../char-mux.h => chardev/chardev-internal.h | 10 ++-
include/sysemu/sysemu.h | 2 +
chardev/char-fe.c | 2 +-
chardev/char-mux.c| 2 +-
chardev/char.c| 37 +-
chardev/chardev-sysemu.c | 69 +++
monitor/misc.c| 1 -
softmmu/vl.c | 42 ---
stubs/machine-init.c | 4 ++
.../{machine-init-done.c => machine-notify.c} | 6 +-
tests/test-char.c | 1 -
util/machine-notify.c | 69 +++
MAINTAINERS | 1 +
chardev/Makefile.objs | 3 +-
stubs/Makefile.objs | 3 +-
16 files changed, 165 insertions(+), 88 deletions(-)
rename include/chardev/char-mux.h => chardev/chardev-internal.h (93%)
create mode 100644 chardev/chardev-sysemu.c
create mode 100644 stubs/machine-init.c
rename stubs/{machine-init-done.c => machine-notify.c} (63%)
create mode 100644 util/machine-notify.c
--
2.21.1
[PATCH] elf_ops: Don't try to g_mapped_file_unref(NULL)
Calling g_mapped_file_unref() on a NULL pointer is not valid, and
glib will assert if you try it.
$ qemu-system-arm -M virt -display none -device loader,file=/tmp/bad.elf
qemu-system-arm: -device loader,file=/tmp/bad.elf: GLib: g_mapped_file_unref:
assertion 'file != NULL' failed
(One way to produce an ELF file that fails like this is to copy just
the first 16 bytes of a valid ELF file; this is sufficient to fool
the code in load_elf_ram_sym() into thinking it's an ELF file and
calling load_elf32() or load_elf64().)
The failure-exit path in load_elf can be reached from various points
in execution, and for some of those we haven't yet called
g_mapped_file_new_from_fd(). Add a condition to the unref call so we
only call it if we successfully created the GMappedFile to start with.
This will fix the assertion; for the specific case of the generic
loader it will then fall back from "guess this is an ELF file" to
"maybe it's a uImage or a hex file" and eventually to "just load as
a raw data file".
Reported-by: Randy Yates
Signed-off-by: Peter Maydell
---
include/hw/elf_ops.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h
index e0bb47bb678..398a4a2c85b 100644
--- a/include/hw/elf_ops.h
+++ b/include/hw/elf_ops.h
@@ -606,7 +606,9 @@ static int glue(load_elf, SZ)(const char *name, int fd,
*highaddr = (uint64_t)(elf_sword)high;
ret = total_size;
fail:
-g_mapped_file_unref(mapped_file);
+if (mapped_file) {
+g_mapped_file_unref(mapped_file);
+}
g_free(phdr);
return ret;
}
--
2.20.1
Re: [RFC PATCH v1 2/7] char-socket: return -1 in case of disconnect during tcp_chr_write
Hi
On Thu, Apr 23, 2020 at 8:43 PM Dima Stepanov wrote:
>
> During testing of the vhost-user-blk reconnect functionality the qemu
> SIGSEGV was triggered:
> start qemu as:
> x86_64-softmmu/qemu-system-x86_64 -m 1024M -M q35 \
>-object
> memory-backend-file,id=ram-node0,size=1024M,mem-path=/dev/shm/qemu,share=on \
>-numa node,cpus=0,memdev=ram-node0 \
>-chardev socket,id=chardev0,path=./vhost.sock,noserver,reconnect=1 \
>-device vhost-user-blk-pci,chardev=chardev0,num-queues=4 --enable-kvm
> start vhost-user-blk daemon:
> ./vhost-user-blk -s ./vhost.sock -b test-img.raw
>
> If vhost-user-blk will be killed during the vhost initialization
> process, for instance after getting VHOST_SET_VRING_CALL command, then
> QEMU will fail with the following backtrace:
>
> Thread 1 "qemu-system-x86" received signal SIGSEGV, Segmentation fault.
> 0x559272bb in vhost_user_read (dev=0x7fffef2d53e0, msg=0x7fffd5b0)
> at ./hw/virtio/vhost-user.c:260
> 260 CharBackend *chr = u->user->chr;
>
> #0 0x559272bb in vhost_user_read (dev=0x7fffef2d53e0,
> msg=0x7fffd5b0)
> at ./hw/virtio/vhost-user.c:260
> #1 0x5592acb8 in vhost_user_get_config (dev=0x7fffef2d53e0,
> config=0x7fffef2d5394 "", config_len=60)
> at ./hw/virtio/vhost-user.c:1645
> #2 0x55925525 in vhost_dev_get_config (hdev=0x7fffef2d53e0,
> config=0x7fffef2d5394 "", config_len=60)
> at ./hw/virtio/vhost.c:1490
> #3 0x558cc46b in vhost_user_blk_device_realize (dev=0x7fffef2d51a0,
> errp=0x7fffd8f0)
> at ./hw/block/vhost-user-blk.c:429
> #4 0x55920090 in virtio_device_realize (dev=0x7fffef2d51a0,
> errp=0x7fffd948)
> at ./hw/virtio/virtio.c:3615
> #5 0x55a9779c in device_set_realized (obj=0x7fffef2d51a0,
> value=true, errp=0x7fffdb88)
> at ./hw/core/qdev.c:891
> ...
>
> The problem is that vhost_user_write doesn't get an error after
> disconnect and try to call vhost_user_read(). The tcp_chr_write()
> routine should return -1 in case of disconnect. Indicate the EIO error
> if this routine is called in the disconnected state.
>
> Signed-off-by: Dima Stepanov
> ---
> chardev/char-socket.c | 8 +---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> index 185fe38..c128cca 100644
> --- a/chardev/char-socket.c
> +++ b/chardev/char-socket.c
> @@ -175,14 +175,16 @@ static int tcp_chr_write(Chardev *chr, const uint8_t
> *buf, int len)
> if (ret < 0 && errno != EAGAIN) {
> if (tcp_chr_read_poll(chr) <= 0) {
> tcp_chr_disconnect_locked(chr);
> -return len;
> +/* Return an error since we made a disconnect. */
> +return ret;
Looks ok, but this return was introduced in commit
b0a335e351103bf92f3f9d0bd5759311be8156ac ("qemu-char: socket backend:
disconnect on write error"). It doesn't say why it didn't return -1
though. Anton, could you review? thanks
> } /* else let the read handler finish it properly */
> }
>
> return ret;
> } else {
> -/* XXX: indicate an error ? */
> -return len;
> +/* Indicate an error. */
> +errno = EIO;
> +return -1;
> }
> }
>
> --
> 2.7.4
>
>
--
Marc-André Lureau
Re: [PATCH] linux-user/riscv: fix up struct target_ucontext definition
On 4/22/20 6:55 PM, LIU Zhiwei wrote: > As far as I know, Guo Ren and Greentime are supporting RVV on Linux, based on > the v0.7.1 QEMU implementation. > The main problem is that VLEN is not a fixed number. Neither is the SVE vector length fixed. That's one of the reasons I pointed you at the AArch64 sigcontext for ideas. > When the Linux kernel released with RVV, I will push a new sigcontext > structure > here. Yep. r~
Re: [RFC PATCH v1 3/7] char-socket: initialize reconnect timer only if close is emitted
Hi
On Thu, Apr 23, 2020 at 8:41 PM Dima Stepanov wrote:
>
> During vhost-user reconnect functionality testing the following assert
> was hit:
> qemu-system-x86_64: chardev/char-socket.c:125:
> qemu_chr_socket_restart_timer: Assertion `!s->reconnect_timer' failed.
> Aborted (core dumped)
That looks related to "[PATCH 3/4] char-socket: avoid double call
tcp_chr_free_connection"
> This is observed only if the connection is closed by the vhost-user-blk
> daemon during the initialization routine. In this case the
> tcp_chr_disconnect_locked() routine is called twice. First time it is
> called in the tcp_chr_write() routine, after getting the SIGPIPE signal.
> Second time it is called when vhost_user_blk_connect() routine return
> error. In general it looks correct, because the initialization routine
> can return error in many cases.
> The tcp_chr_disconnect_locked() routine could be fixed. The timer will
> be restarted only if the close event is emitted.
>
> Signed-off-by: Dima Stepanov
> ---
> chardev/char-socket.c | 10 +-
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> index c128cca..83ca4d9 100644
> --- a/chardev/char-socket.c
> +++ b/chardev/char-socket.c
> @@ -476,7 +476,7 @@ static void update_disconnected_filename(SocketChardev *s)
> static void tcp_chr_disconnect_locked(Chardev *chr)
> {
> SocketChardev *s = SOCKET_CHARDEV(chr);
> -bool emit_close = s->state == TCP_CHARDEV_STATE_CONNECTED;
> +bool was_connected = s->state == TCP_CHARDEV_STATE_CONNECTED;
>
> tcp_chr_free_connection(chr);
>
> @@ -485,11 +485,11 @@ static void tcp_chr_disconnect_locked(Chardev *chr)
>chr, NULL, chr->gcontext);
> }
> update_disconnected_filename(s);
> -if (emit_close) {
> +if (was_connected) {
> qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
> -}
> -if (s->reconnect_time) {
> -qemu_chr_socket_restart_timer(chr);
> +if (s->reconnect_time) {
> +qemu_chr_socket_restart_timer(chr);
> +}
> }
> }
>
> --
> 2.7.4
>
>
--
Marc-André Lureau
[RFC PATCH v1 6/7] vhost: check vring address before calling unmap
Since disconnect can happen at any time during initialization not all
vring buffers (for instance used vring) can be intialized successfully.
If the buffer was not initialized then vhost_memory_unmap call will lead
to SIGSEGV. Add checks for the vring address value before calling unmap.
Also add assert() in the vhost_memory_unmap() routine.
Signed-off-by: Dima Stepanov
---
hw/virtio/vhost.c | 27 +--
1 file changed, 21 insertions(+), 6 deletions(-)
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index ddbdc53..3ee50c4 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -314,6 +314,8 @@ static void vhost_memory_unmap(struct vhost_dev *dev, void
*buffer,
hwaddr len, int is_write,
hwaddr access_len)
{
+assert(buffer);
+
if (!vhost_dev_has_iommu(dev)) {
cpu_physical_memory_unmap(buffer, len, is_write, access_len);
}
@@ -1132,12 +1134,25 @@ static void vhost_virtqueue_stop(struct vhost_dev *dev,
vhost_vq_index);
}
-vhost_memory_unmap(dev, vq->used, virtio_queue_get_used_size(vdev, idx),
- 1, virtio_queue_get_used_size(vdev, idx));
-vhost_memory_unmap(dev, vq->avail, virtio_queue_get_avail_size(vdev, idx),
- 0, virtio_queue_get_avail_size(vdev, idx));
-vhost_memory_unmap(dev, vq->desc, virtio_queue_get_desc_size(vdev, idx),
- 0, virtio_queue_get_desc_size(vdev, idx));
+/*
+ * Since the vhost-user disconnect can happen during initialization
+ * check if vring was initialized, before making unmap.
+ */
+if (vq->used) {
+vhost_memory_unmap(dev, vq->used,
+ virtio_queue_get_used_size(vdev, idx),
+ 1, virtio_queue_get_used_size(vdev, idx));
+}
+if (vq->avail) {
+vhost_memory_unmap(dev, vq->avail,
+ virtio_queue_get_avail_size(vdev, idx),
+ 0, virtio_queue_get_avail_size(vdev, idx));
+}
+if (vq->desc) {
+vhost_memory_unmap(dev, vq->desc,
+ virtio_queue_get_desc_size(vdev, idx),
+ 0, virtio_queue_get_desc_size(vdev, idx));
+}
}
static void vhost_eventfd_add(MemoryListener *listener,
--
2.7.4
[RFC PATCH v1 7/7] vhost: add device started check in migration set log
If vhost-user daemon is used as a backend for the vhost device, then we
should consider a possibility of disconnect at any moment. If such
disconnect happened in the vhost_migration_log() routine the vhost
device structure will be clean up.
At the start of the vhost_migration_log() function there is a check:
if (!dev->started) {
dev->log_enabled = enable;
return 0;
}
To be consistent with this check add the same check after calling the
vhost_dev_set_log() routine. This in general help not to break a
migration due the assert() message. But it looks like that this code
should be revised to handle these errors more carefully.
In case of vhost-user device backend the fail paths should consider the
state of the device. In this case we should skip some function calls
during rollback on the error paths, so not to get the NULL dereference
errors.
Signed-off-by: Dima Stepanov
---
hw/virtio/vhost.c | 39 +++
1 file changed, 35 insertions(+), 4 deletions(-)
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 3ee50c4..d5ab96d 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -787,6 +787,17 @@ static int vhost_dev_set_features(struct vhost_dev *dev,
static int vhost_dev_set_log(struct vhost_dev *dev, bool enable_log)
{
int r, i, idx;
+
+if (!dev->started) {
+/*
+ * If vhost-user daemon is used as a backend for the
+ * device and the connection is broken, then the vhost_dev
+ * structure will be reset all its values to 0.
+ * Add additional check for the device state.
+ */
+return -1;
+}
+
r = vhost_dev_set_features(dev, enable_log);
if (r < 0) {
goto err_features;
@@ -801,12 +812,19 @@ static int vhost_dev_set_log(struct vhost_dev *dev, bool
enable_log)
}
return 0;
err_vq:
-for (; i >= 0; --i) {
+/*
+ * Disconnect with the vhost-user daemon can lead to the
+ * vhost_dev_cleanup() call which will clean up vhost_dev
+ * structure.
+ */
+for (; dev->started && (i >= 0); --i) {
idx = dev->vhost_ops->vhost_get_vq_index(dev, dev->vq_index + i);
vhost_virtqueue_set_addr(dev, dev->vqs + i, idx,
dev->log_enabled);
}
-vhost_dev_set_features(dev, dev->log_enabled);
+if (dev->started) {
+vhost_dev_set_features(dev, dev->log_enabled);
+}
err_features:
return r;
}
@@ -832,7 +850,15 @@ static int vhost_migration_log(MemoryListener *listener,
int enable)
} else {
vhost_dev_log_resize(dev, vhost_get_log_size(dev));
r = vhost_dev_set_log(dev, true);
-if (r < 0) {
+/*
+ * The dev log resize can fail, because of disconnect
+ * with the vhost-user-blk daemon. Check the device
+ * state before calling the vhost_dev_set_log()
+ * function.
+ * Don't return error if device isn't started to be
+ * consistent with the check above.
+ */
+if (dev->started && r < 0) {
return r;
}
}
@@ -1739,7 +1765,12 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice
*vdev)
fail_log:
vhost_log_put(hdev, false);
fail_vq:
-while (--i >= 0) {
+/*
+ * Disconnect with the vhost-user daemon can lead to the
+ * vhost_dev_cleanup() call which will clean up vhost_dev
+ * structure.
+ */
+while ((--i >= 0) && (hdev->started)) {
vhost_virtqueue_stop(hdev,
vdev,
hdev->vqs + i,
--
2.7.4
Re: [PATCH 13/13] qom: Simplify object_property_get_enum()
On 4/23/20 11:00 AM, Markus Armbruster wrote: Signed-off-by: Markus Armbruster --- qom/object.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
[RFC PATCH v1 2/7] char-socket: return -1 in case of disconnect during tcp_chr_write
During testing of the vhost-user-blk reconnect functionality the qemu
SIGSEGV was triggered:
start qemu as:
x86_64-softmmu/qemu-system-x86_64 -m 1024M -M q35 \
-object
memory-backend-file,id=ram-node0,size=1024M,mem-path=/dev/shm/qemu,share=on \
-numa node,cpus=0,memdev=ram-node0 \
-chardev socket,id=chardev0,path=./vhost.sock,noserver,reconnect=1 \
-device vhost-user-blk-pci,chardev=chardev0,num-queues=4 --enable-kvm
start vhost-user-blk daemon:
./vhost-user-blk -s ./vhost.sock -b test-img.raw
If vhost-user-blk will be killed during the vhost initialization
process, for instance after getting VHOST_SET_VRING_CALL command, then
QEMU will fail with the following backtrace:
Thread 1 "qemu-system-x86" received signal SIGSEGV, Segmentation fault.
0x559272bb in vhost_user_read (dev=0x7fffef2d53e0, msg=0x7fffd5b0)
at ./hw/virtio/vhost-user.c:260
260 CharBackend *chr = u->user->chr;
#0 0x559272bb in vhost_user_read (dev=0x7fffef2d53e0,
msg=0x7fffd5b0)
at ./hw/virtio/vhost-user.c:260
#1 0x5592acb8 in vhost_user_get_config (dev=0x7fffef2d53e0,
config=0x7fffef2d5394 "", config_len=60)
at ./hw/virtio/vhost-user.c:1645
#2 0x55925525 in vhost_dev_get_config (hdev=0x7fffef2d53e0,
config=0x7fffef2d5394 "", config_len=60)
at ./hw/virtio/vhost.c:1490
#3 0x558cc46b in vhost_user_blk_device_realize (dev=0x7fffef2d51a0,
errp=0x7fffd8f0)
at ./hw/block/vhost-user-blk.c:429
#4 0x55920090 in virtio_device_realize (dev=0x7fffef2d51a0,
errp=0x7fffd948)
at ./hw/virtio/virtio.c:3615
#5 0x55a9779c in device_set_realized (obj=0x7fffef2d51a0, value=true,
errp=0x7fffdb88)
at ./hw/core/qdev.c:891
...
The problem is that vhost_user_write doesn't get an error after
disconnect and try to call vhost_user_read(). The tcp_chr_write()
routine should return -1 in case of disconnect. Indicate the EIO error
if this routine is called in the disconnected state.
Signed-off-by: Dima Stepanov
---
chardev/char-socket.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 185fe38..c128cca 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -175,14 +175,16 @@ static int tcp_chr_write(Chardev *chr, const uint8_t
*buf, int len)
if (ret < 0 && errno != EAGAIN) {
if (tcp_chr_read_poll(chr) <= 0) {
tcp_chr_disconnect_locked(chr);
-return len;
+/* Return an error since we made a disconnect. */
+return ret;
} /* else let the read handler finish it properly */
}
return ret;
} else {
-/* XXX: indicate an error ? */
-return len;
+/* Indicate an error. */
+errno = EIO;
+return -1;
}
}
--
2.7.4
[RFC PATCH v1 4/7] vhost: introduce wrappers to set guest notifiers for virtio device
Introduce new wrappers to set/reset guest notifiers for the virtio
device in the vhost device module:
vhost_dev_assign_guest_notifiers
->set_guest_notifiers(..., ..., true);
vhost_dev_drop_guest_notifiers
->set_guest_notifiers(..., ..., false);
This is a preliminary step to refactor code, so the set_guest_notifiers
methods could be called based on the vhost device state.
Update all vhost used devices to use these wrappers instead of direct
method call.
Signed-off-by: Dima Stepanov
---
backends/cryptodev-vhost.c | 26 +++---
backends/vhost-user.c | 16 +---
hw/block/vhost-user-blk.c | 15 +--
hw/net/vhost_net.c | 30 +-
hw/scsi/vhost-scsi-common.c | 15 +--
hw/virtio/vhost-user-fs.c | 17 +++--
hw/virtio/vhost-vsock.c | 18 --
hw/virtio/vhost.c | 38 ++
hw/virtio/virtio.c | 13 +
include/hw/virtio/vhost.h | 4
include/hw/virtio/virtio.h | 1 +
11 files changed, 118 insertions(+), 75 deletions(-)
diff --git a/backends/cryptodev-vhost.c b/backends/cryptodev-vhost.c
index 8337c9a..4522195 100644
--- a/backends/cryptodev-vhost.c
+++ b/backends/cryptodev-vhost.c
@@ -169,16 +169,13 @@ vhost_set_vring_enable(CryptoDevBackendClient *cc,
int cryptodev_vhost_start(VirtIODevice *dev, int total_queues)
{
VirtIOCrypto *vcrypto = VIRTIO_CRYPTO(dev);
-BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(dev)));
-VirtioBusState *vbus = VIRTIO_BUS(qbus);
-VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus);
int r, e;
int i;
CryptoDevBackend *b = vcrypto->cryptodev;
CryptoDevBackendVhost *vhost_crypto;
CryptoDevBackendClient *cc;
-if (!k->set_guest_notifiers) {
+if (!virtio_device_guest_notifiers_initialized(dev)) {
error_report("binding does not support guest notifiers");
return -ENOSYS;
}
@@ -198,9 +195,13 @@ int cryptodev_vhost_start(VirtIODevice *dev, int
total_queues)
}
}
-r = k->set_guest_notifiers(qbus->parent, total_queues, true);
+/*
+ * Since all the states are handled by one vhost device,
+ * use the first one in array.
+ */
+vhost_crypto = cryptodev_get_vhost(b->conf.peers.ccs[0], b, 0);
+r = vhost_dev_assign_guest_notifiers(_crypto->dev, dev,
total_queues);
if (r < 0) {
-error_report("error binding guest notifier: %d", -r);
goto err;
}
@@ -232,7 +233,8 @@ err_start:
vhost_crypto = cryptodev_get_vhost(cc, b, i);
cryptodev_vhost_stop_one(vhost_crypto, dev);
}
-e = k->set_guest_notifiers(qbus->parent, total_queues, false);
+vhost_crypto = cryptodev_get_vhost(b->conf.peers.ccs[0], b, 0);
+e = vhost_dev_drop_guest_notifiers(_crypto->dev, dev, total_queues);
if (e < 0) {
error_report("vhost guest notifier cleanup failed: %d", e);
}
@@ -242,9 +244,6 @@ err:
void cryptodev_vhost_stop(VirtIODevice *dev, int total_queues)
{
-BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(dev)));
-VirtioBusState *vbus = VIRTIO_BUS(qbus);
-VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus);
VirtIOCrypto *vcrypto = VIRTIO_CRYPTO(dev);
CryptoDevBackend *b = vcrypto->cryptodev;
CryptoDevBackendVhost *vhost_crypto;
@@ -259,7 +258,12 @@ void cryptodev_vhost_stop(VirtIODevice *dev, int
total_queues)
cryptodev_vhost_stop_one(vhost_crypto, dev);
}
-r = k->set_guest_notifiers(qbus->parent, total_queues, false);
+/*
+ * Since all the states are handled by one vhost device,
+ * use the first one in array.
+ */
+vhost_crypto = cryptodev_get_vhost(b->conf.peers.ccs[0], b, 0);
+r = vhost_dev_drop_guest_notifiers(_crypto->dev, dev, total_queues);
if (r < 0) {
error_report("vhost guest notifier cleanup failed: %d", r);
}
diff --git a/backends/vhost-user.c b/backends/vhost-user.c
index 2bf3406..e116bc6 100644
--- a/backends/vhost-user.c
+++ b/backends/vhost-user.c
@@ -60,15 +60,13 @@ vhost_user_backend_dev_init(VhostUserBackend *b,
VirtIODevice *vdev,
void
vhost_user_backend_start(VhostUserBackend *b)
{
-BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(b->vdev)));
-VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
int ret, i ;
if (b->started) {
return;
}
-if (!k->set_guest_notifiers) {
+if (!virtio_device_guest_notifiers_initialized(b->vdev)) {
error_report("binding does not support guest notifiers");
return;
}
@@ -78,9 +76,8 @@ vhost_user_backend_start(VhostUserBackend *b)
return;
}
-ret = k->set_guest_notifiers(qbus->parent, b->dev.nvqs, true);
+ret = vhost_dev_assign_guest_notifiers(>dev, b->vdev, b->dev.nvqs);
if (ret < 0) {
-error_report("Error binding guest notifier");
goto err_host_notifiers;
}
@@ -104,7 +101,7 @@
[RFC PATCH v1 1/7] contrib/vhost-user-blk: add option to simulate disconnect on init
Add "--simulate-disconnect-stage" option for the testing purposes.
This option can be used to test the vhost-user reconnect functionality:
./vhost-user-blk ... --simulate-disconnect-stage=
In this case the daemon will "crash" in the middle of the VHOST comands
communication. Case nums are as follows:
1 - make assert in the handler of the SET_VRING_CALL command
2 - make assert in the handler of the SET_VRING_NUM command
Main purpose is to test QEMU reconnect functionality. Such fail
injection should not lead to QEMU crash and should be handled
successfully.
Also update the "GOptionEntry entries" definition with the final NULL
item according to API.
Signed-off-by: Dima Stepanov
---
contrib/libvhost-user/libvhost-user.c | 30 ++
contrib/libvhost-user/libvhost-user.h | 13 +
contrib/vhost-user-blk/vhost-user-blk.c | 14 +-
3 files changed, 56 insertions(+), 1 deletion(-)
diff --git a/contrib/libvhost-user/libvhost-user.c
b/contrib/libvhost-user/libvhost-user.c
index 3bca996..5215214 100644
--- a/contrib/libvhost-user/libvhost-user.c
+++ b/contrib/libvhost-user/libvhost-user.c
@@ -73,6 +73,14 @@
#define VHOST_USER_VERSION 1
#define LIBVHOST_USER_DEBUG 0
+/*
+ * Inject fail in different places in daemon. This will trigger different
+ * paths in QEMU. Main purpose is to test the reconnect functionality
+ * during vhost initialization step.
+ */
+#define VHOST_SDISCONNECT_SET_VRING_CALL 1
+#define VHOST_SDISCONNECT_SET_VRING_NUM 2
+
#define DPRINT(...) \
do {\
if (LIBVHOST_USER_DEBUG) { \
@@ -861,6 +869,11 @@ vu_set_vring_num_exec(VuDev *dev, VhostUserMsg *vmsg)
DPRINT("State.index: %d\n", index);
DPRINT("State.num: %d\n", num);
dev->vq[index].vring.num = num;
+if (dev->simulate_init_disconnect == VHOST_SDISCONNECT_SET_VRING_NUM) {
+DPRINT("Simulate vhost daemon crash during initialization.\n");
+assert(0);
+return false;
+}
return false;
}
@@ -1161,6 +1174,13 @@ vu_set_vring_call_exec(VuDev *dev, VhostUserMsg *vmsg)
DPRINT("u64: 0x%016"PRIx64"\n", vmsg->payload.u64);
+/* Simulate crash during initialization. */
+if (dev->simulate_init_disconnect == VHOST_SDISCONNECT_SET_VRING_CALL) {
+DPRINT("Simulate vhost daemon crash during initialization.\n");
+assert(0);
+return false;
+}
+
if (!vu_check_queue_msg_file(dev, vmsg)) {
return false;
}
@@ -2073,6 +2093,16 @@ vu_queue_empty(VuDev *dev, VuVirtq *vq)
return vring_avail_idx(vq) == vq->last_avail_idx;
}
+/*
+ * Set the flag to simulate the vhost-user daemon crash during
+ * initialization. This is used to test reconnect functionality.
+ */
+void
+vu_simulate_init_disconnect(VuDev *dev, int should_simulate)
+{
+dev->simulate_init_disconnect = should_simulate;
+}
+
static bool
vring_notify(VuDev *dev, VuVirtq *vq)
{
diff --git a/contrib/libvhost-user/libvhost-user.h
b/contrib/libvhost-user/libvhost-user.h
index f30394f..9f75e86 100644
--- a/contrib/libvhost-user/libvhost-user.h
+++ b/contrib/libvhost-user/libvhost-user.h
@@ -388,6 +388,9 @@ struct VuDev {
/* Postcopy data */
int postcopy_ufd;
bool postcopy_listening;
+
+/* Fields to simulate test cases. */
+int simulate_init_disconnect;
};
typedef struct VuVirtqElement {
@@ -645,4 +648,14 @@ void vu_queue_get_avail_bytes(VuDev *vdev, VuVirtq *vq,
unsigned int *in_bytes,
bool vu_queue_avail_bytes(VuDev *dev, VuVirtq *vq, unsigned int in_bytes,
unsigned int out_bytes);
+/**
+ * vu_simulate_init_disconnect:
+ * @dev: a VuDev context
+ * @should_simulate: expected simulation behaviour (true or false)
+ *
+ * Set the flag to simulate the vhost-user daemon crash during
+ * initialization. This is used to test reconnect functionality.
+ */
+void vu_simulate_init_disconnect(VuDev *dev, int should_simulate);
+
#endif /* LIBVHOST_USER_H */
diff --git a/contrib/vhost-user-blk/vhost-user-blk.c
b/contrib/vhost-user-blk/vhost-user-blk.c
index 6fd91c7..6ac37ca 100644
--- a/contrib/vhost-user-blk/vhost-user-blk.c
+++ b/contrib/vhost-user-blk/vhost-user-blk.c
@@ -581,6 +581,7 @@ static char *opt_socket_path;
static char *opt_blk_file;
static gboolean opt_print_caps;
static gboolean opt_read_only;
+static gboolean opt_simulate_disconnect;
static GOptionEntry entries[] = {
{ "print-capabilities", 'c', 0, G_OPTION_ARG_NONE, _print_caps,
@@ -592,7 +593,14 @@ static GOptionEntry entries[] = {
{"blk-file", 'b', 0, G_OPTION_ARG_FILENAME, _blk_file,
"block device or file path", "PATH"},
{ "read-only", 'r', 0, G_OPTION_ARG_NONE, _read_only,
- "Enable read-only", NULL }
+ "Enable read-only", NULL },
+{ "simulate-disconnect-stage", 0, 0, G_OPTION_ARG_INT,
+ _simulate_disconnect,
+ "Simulate disconnect during initialization for the testing
[RFC PATCH v1 5/7] vhost-user-blk: add mechanism to track the guest notifiers init state
In case of the vhost-user devices the daemon can be killed at any
moment. Since QEMU supports the reconnet functionality the guest
notifiers should be reset and disabled after "disconnect" event. The
most issues were found if the "disconnect" event happened during vhost
device initialization step.
The disconnect event leads to the call of the vhost_dev_cleanup()
routine. Which memset to 0 a vhost device structure. Because of this, if
device was not started (dev.started == false) and the connection is
broken, then the set_guest_notifier method will produce assertion error.
Also connection can be broken after the dev.started field is set to
true.
A new notifiers_set field is added to the vhost_dev structure to track
the state of the guest notifiers during the initialization process.
Signed-off-by: Dima Stepanov
---
hw/block/vhost-user-blk.c | 8
hw/virtio/vhost.c | 11 +++
include/hw/virtio/vhost.h | 1 +
3 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index 70d7842..5a3de0f 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -175,7 +175,9 @@ static void vhost_user_blk_stop(VirtIODevice *vdev)
return;
}
-vhost_dev_stop(>dev, vdev);
+if (s->dev.started) {
+vhost_dev_stop(>dev, vdev);
+}
ret = vhost_dev_drop_guest_notifiers(>dev, vdev, s->dev.nvqs);
if (ret < 0) {
@@ -337,9 +339,7 @@ static void vhost_user_blk_disconnect(DeviceState *dev)
}
s->connected = false;
-if (s->dev.started) {
-vhost_user_blk_stop(vdev);
-}
+vhost_user_blk_stop(vdev);
vhost_dev_cleanup(>dev);
}
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index fa3da9c..ddbdc53 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1380,6 +1380,7 @@ int vhost_dev_enable_notifiers(struct vhost_dev *hdev,
VirtIODevice *vdev)
goto fail_vq;
}
}
+hdev->notifiers_set = true;
return 0;
fail_vq:
@@ -1407,6 +1408,10 @@ void vhost_dev_disable_notifiers(struct vhost_dev *hdev,
VirtIODevice *vdev)
BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
int i, r;
+if (!hdev->notifiers_set) {
+return;
+}
+
for (i = 0; i < hdev->nvqs; ++i) {
r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index + i,
false);
@@ -1417,6 +1422,8 @@ void vhost_dev_disable_notifiers(struct vhost_dev *hdev,
VirtIODevice *vdev)
virtio_bus_cleanup_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index + i);
}
virtio_device_release_ioeventfd(vdev);
+
+hdev->notifiers_set = false;
}
/*
@@ -1449,6 +1456,10 @@ int vhost_dev_drop_guest_notifiers(struct vhost_dev
*hdev,
VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
int ret;
+if (!hdev->notifiers_set) {
+return 0;
+}
+
ret = k->set_guest_notifiers(qbus->parent, nvqs, false);
if (ret < 0) {
error_report("Error reset guest notifier: %d", -ret);
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index 4d0d2e2..e3711a7 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -90,6 +90,7 @@ struct vhost_dev {
QLIST_HEAD(, vhost_iommu) iommu_list;
IOMMUNotifier n;
const VhostDevConfigOps *config_ops;
+bool notifiers_set;
};
int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
--
2.7.4
[RFC PATCH v1 0/7] vhost-user reconnect issues during vhost initialization
During vhost-user reconnect functionality we hit several issues, if vhost-user-blk daemon is "crashed" or made disconnect during vhost initialization. The general scenario is as follows: - vhost start routine is called - vhost write failed due to SIGPIPE - this call the disconnect routine and vhost_dev_cleanup routine which set to 0 all the field of the vhost_dev structure - return back to vhost start routine with the error - on the fail path vhost start routine tries to rollback the changes by using vhost_dev struct fields which were already reset - sometimes this leads to SIGSEGV, sometimes to SIGABRT Before revising the vhost-user initialization code, we suggest adding the sanity checks to be aware of the possible disconnect event and that the vhost_dev structure can be in "uninitialized" state. The vhost-user-blk daemon is updated with the additional "--simulate-disconnect-stage=CASENUM" argument to simulate disconnect during VHOST device initialization. For instance: 1. $ ./vhost-user-blk -s ./vhost.sock -b test-img.raw --simulate-disconnect-stage=1 This command will simulate disconnect in the SET_VRING_CALL handler. In this case the vhost device in QEMU is not set the started field to true. 2. $ ./vhost-user-blk -s ./vhost.sock -b test-img.raw --simulate-disconnect-stage=2 This command will simulate disconnect in the SET_VRING_NUM handler. In this case the started field is set to true. These two cases test different QEMU parts. Also to trigger different code paths disconnect should be simulated in two ways: - before any successful initialization - make successful initialization once and try to simulate disconnects Also we catch SIGABRT on the migration start if vhost-user daemon disconnected during vhost-user set log commands communication. Dima Stepanov (7): contrib/vhost-user-blk: add option to simulate disconnect on init char-socket: return -1 in case of disconnect during tcp_chr_write char-socket: initialize reconnect timer only if close is emitted vhost: introduce wrappers to set guest notifiers for virtio device vhost-user-blk: add mechanism to track the guest notifiers init state vhost: check vring address before calling unmap vhost: add device started check in migration set log backends/cryptodev-vhost.c | 26 +--- backends/vhost-user.c | 16 ++--- chardev/char-socket.c | 18 ++--- contrib/libvhost-user/libvhost-user.c | 30 + contrib/libvhost-user/libvhost-user.h | 13 contrib/vhost-user-blk/vhost-user-blk.c | 14 +++- hw/block/vhost-user-blk.c | 23 +++ hw/net/vhost_net.c | 30 + hw/scsi/vhost-scsi-common.c | 15 ++--- hw/virtio/vhost-user-fs.c | 17 ++--- hw/virtio/vhost-vsock.c | 18 +++-- hw/virtio/vhost.c | 115 +--- hw/virtio/virtio.c | 13 include/hw/virtio/vhost.h | 5 ++ include/hw/virtio/virtio.h | 1 + 15 files changed, 256 insertions(+), 98 deletions(-) -- 2.7.4
[RFC PATCH v1 3/7] char-socket: initialize reconnect timer only if close is emitted
During vhost-user reconnect functionality testing the following assert
was hit:
qemu-system-x86_64: chardev/char-socket.c:125:
qemu_chr_socket_restart_timer: Assertion `!s->reconnect_timer' failed.
Aborted (core dumped)
This is observed only if the connection is closed by the vhost-user-blk
daemon during the initialization routine. In this case the
tcp_chr_disconnect_locked() routine is called twice. First time it is
called in the tcp_chr_write() routine, after getting the SIGPIPE signal.
Second time it is called when vhost_user_blk_connect() routine return
error. In general it looks correct, because the initialization routine
can return error in many cases.
The tcp_chr_disconnect_locked() routine could be fixed. The timer will
be restarted only if the close event is emitted.
Signed-off-by: Dima Stepanov
---
chardev/char-socket.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index c128cca..83ca4d9 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -476,7 +476,7 @@ static void update_disconnected_filename(SocketChardev *s)
static void tcp_chr_disconnect_locked(Chardev *chr)
{
SocketChardev *s = SOCKET_CHARDEV(chr);
-bool emit_close = s->state == TCP_CHARDEV_STATE_CONNECTED;
+bool was_connected = s->state == TCP_CHARDEV_STATE_CONNECTED;
tcp_chr_free_connection(chr);
@@ -485,11 +485,11 @@ static void tcp_chr_disconnect_locked(Chardev *chr)
chr, NULL, chr->gcontext);
}
update_disconnected_filename(s);
-if (emit_close) {
+if (was_connected) {
qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
-}
-if (s->reconnect_time) {
-qemu_chr_socket_restart_timer(chr);
+if (s->reconnect_time) {
+qemu_chr_socket_restart_timer(chr);
+}
}
}
--
2.7.4
Re: [PATCH 12/13] qapi: Only input visitors can actually fail
On 4/23/20 11:00 AM, Markus Armbruster wrote: The previous few commits have made this more obvious, and removed the one exception. Time to clarify the documentation, and drop dead error checking. Signed-off-by: Markus Armbruster --- I guess the only other failures an output visitor might encounter are out-of-memory, or invalid contents (such as a NaN in a 'number', or invalid encoding in a string...), and aborting on those errors at the point they happen rather than trying to return errp makes sense. It makes for a nice conceptual result: input parsing fails on unexpected input, but output visitors should never be used on invalid data. Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH 10/13] qapi: Clean up visitor's recovery from input with invalid type
On 4/23/20 1:06 PM, Eric Blake wrote:
On 4/23/20 11:00 AM, Markus Armbruster wrote:
An alternate type's visit_type_FOO() fails when it runs into an
invalid ->type. If it's an input visit, we then need to free the the
object we got from visit_start_alternate(). We do that with
qapi_free_FOO(), which uses the dealloc visitor.
Trouble is that object is in a bad state: its ->type is invalid. So
the dealloc visitor will run into the same error again, and the error
recovery skips deallocating the alternate's (invalid) alternative.
This is a roundabout way to g_free() the alternate.
Simplify: replace the qapi_free_FOO() by g_free().
Signed-off-by: Markus Armbruster
---
scripts/qapi/visit.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Required looking at what gets generated into qapi_free_FOO() as well as
when visit_start_alternate() can fail, but makes sense.
Reviewed-by: Eric Blake
Actually, I'm having second thoughts. As an example, look at the generated:
void visit_type_BlockDirtyBitmapMergeSource(Visitor *v, const char *name,
BlockDirtyBitmapMergeSource **obj, Error **errp)
{
Error *err = NULL;
visit_start_alternate(v, name, (GenericAlternate **)obj, sizeof(**obj),
);
if (err) {
goto out;
}
if (!*obj) {
goto out_obj;
[1]
}
switch ((*obj)->type) {
case QTYPE_QSTRING:
visit_type_str(v, name, &(*obj)->u.local, );
[2]
break;
case QTYPE_QDICT:
visit_start_struct(v, name, NULL, 0, );
if (err) {
break;
[3]
}
visit_type_BlockDirtyBitmap_members(v, &(*obj)->u.external, );
if (!err) {
visit_check_struct(v, );
[4]
}
visit_end_struct(v, NULL);
break;
case QTYPE_NONE:
abort();
default:
error_setg(, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
"BlockDirtyBitmapMergeSource");
[5]
}
out_obj:
visit_end_alternate(v, (void **)obj);
if (err && visit_is_input(v)) {
qapi_free_BlockDirtyBitmapMergeSource(*obj);
If we got here, we must have failed at any of the points mentioned above.
If [1], visit_start_alternate() failed, but *obj is NULL and both
qapi_free_FOO(NULL) and g_free(NULL) are safe.
If [2], visit_type_str() failed, so *obj is allocated but the embedded
string (here, u.local) was left NULL. qapi_free_FOO() then does nothing
further than g_free(obj).
If [3], visit_start_struct() failed, the embedded dict (here,
u.external) was left NULL. qapi_free_FOO() then does nothing further
than g_free(obj).
If [5], we have the wrong ->type. As pointed out by this commit,
qapi_free_FOO() does nothing further than g_free(obj).
But what happens in [4]? Here, the embedded dict was allocated, but we
then failed while parsing its members. That leaves us in a
partially-allocated state, and g_free(NULL) does NOT recursively visit
that partial allocation. I think this patch is prone to a memory leak
unless you _also_ patch things to free any dict branch on failure
(perhaps during the QTYPE_QDICT case label, rather than here at the end).
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Re: [PATCH for 5.1 v1 00/14] guest_base, gdbstub and Travis
Patchew URL: https://patchew.org/QEMU/20200423170557.31106-1-alex.ben...@linaro.org/ 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 === Cleared directory 'slirp' Submodule 'slirp' (https://git.qemu.org/git/libslirp.git) unregistered for path 'slirp' Submodule 'ui/keycodemapdb' (https://git.qemu.org/git/keycodemapdb.git) unregistered for path 'ui/keycodemapdb' make[1]: *** [/var/tmp/patchew-tester-tmp-rhig99s_/src/docker-src.2020-04-23-14.15.56.12776] Error 1 make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-rhig99s_/src' make: *** [docker-run-test-quick@centos7] Error 2 real2m0.927s user0m5.275s The full log is available at http://patchew.org/logs/20200423170557.31106-1-alex.ben...@linaro.org/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 for 5.1 v1 00/14] guest_base, gdbstub and Travis
Patchew URL:
https://patchew.org/QEMU/20200423170557.31106-1-alex.ben...@linaro.org/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Subject: [PATCH for 5.1 v1 00/14] guest_base, gdbstub and Travis
Message-id: 20200423170557.31106-1-alex.ben...@linaro.org
Type: series
=== 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
From https://github.com/patchew-project/qemu
- [tag update] patchew/20200423150127.142609-1-kw...@redhat.com ->
patchew/20200423150127.142609-1-kw...@redhat.com
- [tag update] patchew/20200423160036.7048-1-arm...@redhat.com ->
patchew/20200423160036.7048-1-arm...@redhat.com
Switched to a new branch 'test'
689e6e8 .travis.yml: drop MacOSX
f7c8828 .travis.yml: show free disk space at end of run
c2d0ee7 tests/tcg: add a multiarch linux-user gdb test
5002abd tests/guest-debug: use the unix socket for linux-user tests
98d25bf gdbstub/linux-user: support debugging over a unix socket
37544e1 gdbstub: eliminate gdbserver_fd global
852d87f tests/tcg: drop inferior.was_attached() test
72e9645 tests/tcg: better trap gdb failures
8a161b1 gdbstub: Introduce gdb_get_float64() to get 64-bit float registers
d0f142f configure: favour gdb-multiarch if we have it
bc6da7bb .gitignore: include common build sub-directories
ad7e15b accel/tcg: Relax va restrictions on 64-bit guests
55ee255 exec/cpu-all: Use bool for have_guest_base
55dbacd linux-user: completely re-write init_guest_space
=== OUTPUT BEGIN ===
1/14 Checking commit 55dbacd6b444 (linux-user: completely re-write
init_guest_space)
2/14 Checking commit 55ee255da7f6 (exec/cpu-all: Use bool for have_guest_base)
3/14 Checking commit ad7e15b42dc2 (accel/tcg: Relax va restrictions on 64-bit
guests)
ERROR: Macros with complex values should be enclosed in parenthesis
#91: FILE: include/exec/cpu-all.h:182:
+# define GUEST_ADDR_MAX_ ~0ul
total: 1 errors, 0 warnings, 88 lines checked
Patch 3/14 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
4/14 Checking commit bc6da7bb0a03 (.gitignore: include common build
sub-directories)
5/14 Checking commit d0f142fa8cfd (configure: favour gdb-multiarch if we have
it)
6/14 Checking commit 8a161b12a736 (gdbstub: Introduce gdb_get_float64() to get
64-bit float registers)
7/14 Checking commit 72e964519511 (tests/tcg: better trap gdb failures)
8/14 Checking commit 852d87fc501d (tests/tcg: drop inferior.was_attached() test)
9/14 Checking commit 37544e18de5c (gdbstub: eliminate gdbserver_fd global)
ERROR: suspect code indent for conditional statements (2, 6)
#33: FILE: gdbstub.c:2965:
+ if (gdbserver_state.fd < 0) {
return;
ERROR: braces {} are necessary for all arms of this statement
#80: FILE: gdbstub.c:3139:
+if (gdb_fd < 0)
[...]
total: 2 errors, 0 warnings, 73 lines checked
Patch 9/14 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
10/14 Checking commit 98d25bf64760 (gdbstub/linux-user: support debugging over
a unix socket)
ERROR: suspect code indent for conditional statements (2, 6)
#67: FILE: gdbstub.c:2966:
+ if (gdbserver_state.socket_path) {
+ unlink(gdbserver_state.socket_path);
ERROR: space required before the open parenthesis '('
#101: FILE: gdbstub.c:3087:
+for(;;) {
ERROR: spaces required around that '-' (ctx:VxV)
#128: FILE: gdbstub.c:3114:
+pstrcpy(sockaddr.sun_path, sizeof(sockaddr.sun_path)-1, path);
^
total: 3 errors, 0 warnings, 228 lines checked
Patch 10/14 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
11/14 Checking commit 5002abd35782 (tests/guest-debug: use the unix socket for
linux-user tests)
WARNING: line over 80 characters
#38: FILE: tests/guest-debug/run-test.py:53:
+cmd = "%s %s -g %s %s" % (args.qemu, args.qargs, socket_name,
args.binary)
total: 0 errors, 1 warnings, 34 lines checked
Patch 11/14 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
12/14 Checking commit c2d0ee78fa1d (tests/tcg: add a multiarch linux-user gdb
test)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#75:
new file mode 100644
total: 0 errors, 1 warnings, 124 lines checked
Patch 12/14 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
13/14 Checking commit f7c88287e116 (.travis.yml: show free disk space at
Re: [PATCH v3 1/9] block: Avoid dead assignment
On 22.04.20 15:31, Philippe Mathieu-Daudé wrote: > Fix warning reported by Clang static code analyzer: > > block.c:3167:5: warning: Value stored to 'ret' is never read > ret = bdrv_fill_options(, filename, , _err); > ^ ~ > > Fixes: 462f5bcf6 > Reported-by: Clang Static Analyzer > Suggested-by: Markus Armbruster > Reviewed-by: Alistair Francis > Signed-off-by: Philippe Mathieu-Daudé > --- > block.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Max Reitz signature.asc Description: OpenPGP digital signature
Re: [PATCH v3 2/9] blockdev: Remove dead assignment
On 22.04.20 15:31, Philippe Mathieu-Daudé wrote: > Fix warning reported by Clang static code analyzer: > > CC blockdev.o > blockdev.c:2744:5: warning: Value stored to 'ret' is never read > ret = blk_truncate(blk, size, false, PREALLOC_MODE_OFF, errp); > ^ ~~~ > > Reported-by: Clang Static Analyzer > Signed-off-by: Philippe Mathieu-Daudé > --- > blockdev.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Max Reitz signature.asc Description: OpenPGP digital signature
Re: [PATCH 11/13] qapi: Assert non-input visitors see only valid alternate tags
On 4/23/20 11:00 AM, Markus Armbruster wrote: An alternate type's visit_type_FOO() fails when it runs into an invalid ->type. This is appropriate with an input visitor: visit_start_alternate() sets ->type according to the input, and bad input can lead to bad ->type. It should never happen with an output, clone or dealloc visitor: if it did, the alternate being output, cloned or deallocated would be messed up beyond repair. Assert that. Signed-off-by: Markus Armbruster --- scripts/qapi/visit.py | 1 + 1 file changed, 1 insertion(+) Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH 10/13] qapi: Clean up visitor's recovery from input with invalid type
On 4/23/20 11:00 AM, Markus Armbruster wrote: An alternate type's visit_type_FOO() fails when it runs into an invalid ->type. If it's an input visit, we then need to free the the object we got from visit_start_alternate(). We do that with qapi_free_FOO(), which uses the dealloc visitor. Trouble is that object is in a bad state: its ->type is invalid. So the dealloc visitor will run into the same error again, and the error recovery skips deallocating the alternate's (invalid) alternative. This is a roundabout way to g_free() the alternate. Simplify: replace the qapi_free_FOO() by g_free(). Signed-off-by: Markus Armbruster --- scripts/qapi/visit.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Required looking at what gets generated into qapi_free_FOO() as well as when visit_start_alternate() can fail, but makes sense. Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH v6 07/10] block: truncate: Don't make backing file data visible
On 23.04.20 17:01, Kevin Wolf wrote:
> When extending the size of an image that has a backing file larger than
> its old size, make sure that the backing file data doesn't become
> visible in the guest, but the added area is properly zeroed out.
>
> Consider the following scenario where the overlay is shorter than its
> backing file:
>
> base.qcow2:
> overlay.qcow2:
>
> When resizing (extending) overlay.qcow2, the new blocks should not stay
> unallocated and make the additional As from base.qcow2 visible like
> before this patch, but zeros should be read.
>
> A similar case happens with the various variants of a commit job when an
> intermediate file is short (- for unallocated):
>
> base.qcow2: A-A-
> mid.qcow2: BB-B
> top.qcow2: C--C--C-
>
> After commit top.qcow2 to mid.qcow2, the following happens:
>
> mid.qcow2: CB-C00C0 (correct result)
> mid.qcow2: CB-C--C- (before this fix)
>
> Without the fix, blocks that previously read as zeros on top.qcow2
> suddenly turn into A.
>
> Signed-off-by: Kevin Wolf
> Reviewed-by: Alberto Garcia
Seems like Berto gave you a rather broad R-b in v4. :)
> ---
> block/io.c | 24
> 1 file changed, 24 insertions(+)
>
> diff --git a/block/io.c b/block/io.c
> index 795075954e..f618db3499 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -3394,6 +3394,30 @@ int coroutine_fn bdrv_co_truncate(BdrvChild *child,
> int64_t offset, bool exact,
> goto out;
> }
>
> +/*
> + * If the image has a backing file that is large enough that it would
> + * provide data for the new area, we cannot leave it unallocated because
> + * then the backing file content would become visible. Instead, zero-fill
> + * the new area.
> + *
> + * Note that if the image has a backing file, but was opened without the
> + * backing file, taking care of keeping things consistent with that
> backing
> + * file is the user's responsibility.
> + */
> +if (new_bytes && bs->backing) {
> +int64_t backing_len;
> +
> +backing_len = bdrv_getlength(backing_bs(bs));
> +if (backing_len < 0) {
> +ret = backing_len;
Shouldn’t we set errp?
Max
> +goto out;
> +}
> +
> +if (backing_len > old_size) {
> +flags |= BDRV_REQ_ZERO_WRITE;
> +}
> +}
> +
> if (drv->bdrv_co_truncate) {
> if (flags & ~bs->supported_truncate_flags) {
> error_setg(errp, "Block driver does not support requested
> flags");
>
signature.asc
Description: OpenPGP digital signature
[PATCH RFC] target/arm: Implement SVE2 SPLICE, EXT
Signed-off-by: Stephen Long
I'm not sure I can just use the SVE helper functions for the SVE2
variants of EXT and SPLICE.
---
target/arm/sve.decode | 8
target/arm/translate-sve.c | 39 +-
2 files changed, 42 insertions(+), 5 deletions(-)
diff --git a/target/arm/sve.decode b/target/arm/sve.decode
index 3a2a4a7f1c..004cbb4191 100644
--- a/target/arm/sve.decode
+++ b/target/arm/sve.decode
@@ -1387,3 +1387,11 @@ UMLSLT_zzzw 01000100 .. 0 . 010 111 . .
@rda_rn_rm
CMLA_ 01000100 esz:2 0 rm:5 0010 rot:2 rn:5 rd:5 ra=%reg_movprfx
SQRDCMLAH_ 01000100 esz:2 0 rm:5 0011 rot:2 rn:5 rd:5 ra=%reg_movprfx
+
+### SVE2 vector splice (predicated, constructive)
+
+SPLICE_zpz 0101 .. 101 101 100 ... . . @rd_pg_rn
+
+### SVE2 extract vector (immediate offset)
+EXT_zzi 0101 011 . 000 ... rn:5 rd:5 \
+ imm=%imm8_16_10
diff --git a/target/arm/translate-sve.c b/target/arm/translate-sve.c
index 20eb588cb3..8c34785449 100644
--- a/target/arm/translate-sve.c
+++ b/target/arm/translate-sve.c
@@ -2269,18 +2269,18 @@ static bool trans_CPY_z_i(DisasContext *s, arg_CPY_z_i
*a)
*** SVE Permute Extract Group
*/
-static bool trans_EXT(DisasContext *s, arg_EXT *a)
+static bool do_EXT(DisasContext *s, int rd, int rn, int rm, int imm)
{
if (!sve_access_check(s)) {
return true;
}
unsigned vsz = vec_full_reg_size(s);
-unsigned n_ofs = a->imm >= vsz ? 0 : a->imm;
+unsigned n_ofs = imm >= vsz ? 0 : imm;
unsigned n_siz = vsz - n_ofs;
-unsigned d = vec_full_reg_offset(s, a->rd);
-unsigned n = vec_full_reg_offset(s, a->rn);
-unsigned m = vec_full_reg_offset(s, a->rm);
+unsigned d = vec_full_reg_offset(s, rd);
+unsigned n = vec_full_reg_offset(s, rn);
+unsigned m = vec_full_reg_offset(s, rm);
/* Use host vector move insns if we have appropriate sizes
* and no unfortunate overlap.
@@ -2299,6 +2299,11 @@ static bool trans_EXT(DisasContext *s, arg_EXT *a)
return true;
}
+static bool trans_EXT(DisasContext *s, arg_EXT *a)
+{
+return do_EXT(s, a->rd, a->rn, a->rm, a->imm);
+}
+
/*
*** SVE Permute - Unpredicated Group
*/
@@ -7882,3 +7887,27 @@ static bool trans_SQRDCMLAH_(DisasContext *s,
arg_CMLA_ *a)
};
return do_sve2__fn(s, a->rd, a->rn, a->rm, a->ra, fns[a->esz], a->rot);
}
+
+static bool trans_SPLICE_zpz(DisasContext *s, arg_rpr_esz *a)
+{
+if (!dc_isar_feature(aa64_sve2, s)) {
+return false;
+}
+if (sve_access_check(s)) {
+unsigned vsz = vec_full_reg_size(s);
+tcg_gen_gvec_4_ool(vec_full_reg_offset(s, a->rd),
+ vec_full_reg_offset(s, a->rn),
+ vec_full_reg_offset(s, (a->rn + 1) % 32),
+ pred_full_reg_offset(s, a->pg),
+ vsz, vsz, a->esz, gen_helper_sve_splice);
+}
+return true;
+}
+
+static bool trans_EXT_zzi(DisasContext *s, arg_rri *a)
+{
+if (!dc_isar_feature(aa64_sve2, s)) {
+return false;
+}
+return do_EXT(s, a->rd, a->rn, (a->rn + 1) % 32, a->imm);
+}
--
2.17.1
Re: [PATCH v6 10/10] qcow2: Forward ZERO_WRITE flag for full preallocation
On 23.04.20 17:01, Kevin Wolf wrote: > The BDRV_REQ_ZERO_WRITE is currently implemented in a way that first the > image is possibly preallocated and then the zero flag is added to all > clusters. This means that a copy-on-write operation may be needed when > writing to these clusters, despite having used preallocation, negating > one of the major benefits of preallocation. > > Instead, try to forward the BDRV_REQ_ZERO_WRITE to the protocol driver, > and if the protocol driver can ensure that the new area reads as zeros, > we can skip setting the zero flag in the qcow2 layer. > > Unfortunately, the same approach doesn't work for metadata > preallocation, so we'll still set the zero flag there. > > Signed-off-by: Kevin Wolf > --- > block/qcow2.c | 22 +++--- > tests/qemu-iotests/274.out | 4 ++-- > 2 files changed, 21 insertions(+), 5 deletions(-) Oh, nice. I didn’t think you (or anyone else for that matter) would actually do this. :) With the errp thing fixed: Reviewed-by: Max Reitz signature.asc Description: OpenPGP digital signature
Re: [PATCH 09/13] qapi: Assert non-input visitors see only valid narrow integers
On 4/23/20 11:00 AM, Markus Armbruster wrote: visit_type_intN() and visit_type_uintN() fail when the value is out of bounds. This is appropriate with an input visitor: the value comes from input, and input may be bad. It should never happen with the other visitors: the value comes from the caller, and callers must keep it within bounds. Assert that. Signed-off-by: Markus Armbruster --- qapi/qapi-visit-core.c | 6 ++ 1 file changed, 6 insertions(+) Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH 08/13] qapi: Assert output visitors see only valid enum values
On 4/23/20 11:00 AM, Markus Armbruster wrote:
output_type_enum() fails when *obj is not a valid value of the enum
type. Should not happen. Drop the check, along with its unit tests.
qapi_enum_lookup()'s assertion still guards.
Signed-off-by: Markus Armbruster
---
qapi/qapi-visit-core.c | 9 ---
tests/test-qobject-output-visitor.c | 39 -
tests/test-string-output-visitor.c | 19 --
3 files changed, 67 deletions(-)
Nice cleanup.
The commit message implies adding an assertion; but in reality, the
change is deleting dead code (because we already have the assertion in
place). Maybe it's worth updating the subject?
Reviewed-by: Eric Blake
-/*
- * TODO why is this an error, not an assertion? If assertion:
- * delete, and rely on qapi_enum_lookup()
- */
-if (value < 0 || value >= lookup->size) {
-error_setg(errp, QERR_INVALID_PARAMETER, name ? name : "null");
-return;
-}
The comment being deleted is what points out the assertion that already
exists.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
[Bug 1874504] [NEW] VFIO passthrough spits out thousands of messages
Public bug reported: started qemu as: sudo qemu-system-sparc64 -device vfio-pci,host=0b:05.0,x-no-mmap=on,bus=pciB messages received thousands of times: qemu-system-sparc64: -device vfio-pci,host=0b:05.0,x-no-mmap=on,bus=pciB: iommu has granularity incompatible with target AS qemu-system-sparc64: -device vfio-pci,host=0b:05.0,x-no-mmap=on,bus=pciB: iommu map to non memory area 4079c000 qemu version (think telling a lie as sure its 5.0) QEMU emulator version 4.2.92 Copyright (c) 2003-2020 Fabrice Bellard and the QEMU Project developers pci device being passed through: 0b:05.0 Display controller [0380]: 3DLabs Permedia II 2D+3D [3d3d:0009] (rev 01) Subsystem: Tech-Source Permedia II 2D+3D [1227:0006] Flags: medium devsel, IRQ 11 Memory at 8300 (32-bit, non-prefetchable) [disabled] [size=128K] Memory at 8280 (32-bit, non-prefetchable) [disabled] [size=8M] Memory at 8200 (32-bit, non-prefetchable) [disabled] [size=8M] Expansion ROM at 8302 [disabled] [size=64K] Capabilities: Kernel driver in use: vfio-pci ** Affects: qemu Importance: Undecided Status: New ** Tags: sparc64 vfio ** Attachment added: "output.txt" https://bugs.launchpad.net/bugs/1874504/+attachment/5358566/+files/output.txt -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1874504 Title: VFIO passthrough spits out thousands of messages Status in QEMU: New Bug description: started qemu as: sudo qemu-system-sparc64 -device vfio-pci,host=0b:05.0,x-no-mmap=on,bus=pciB messages received thousands of times: qemu-system-sparc64: -device vfio-pci,host=0b:05.0,x-no-mmap=on,bus=pciB: iommu has granularity incompatible with target AS qemu-system-sparc64: -device vfio-pci,host=0b:05.0,x-no-mmap=on,bus=pciB: iommu map to non memory area 4079c000 qemu version (think telling a lie as sure its 5.0) QEMU emulator version 4.2.92 Copyright (c) 2003-2020 Fabrice Bellard and the QEMU Project developers pci device being passed through: 0b:05.0 Display controller [0380]: 3DLabs Permedia II 2D+3D [3d3d:0009] (rev 01) Subsystem: Tech-Source Permedia II 2D+3D [1227:0006] Flags: medium devsel, IRQ 11 Memory at 8300 (32-bit, non-prefetchable) [disabled] [size=128K] Memory at 8280 (32-bit, non-prefetchable) [disabled] [size=8M] Memory at 8200 (32-bit, non-prefetchable) [disabled] [size=8M] Expansion ROM at 8302 [disabled] [size=64K] Capabilities: Kernel driver in use: vfio-pci To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1874504/+subscriptions
Re: [PATCH 07/13] qapi: Fix Visitor contract for start_alternate()
On 4/23/20 11:00 AM, Markus Armbruster wrote: The contract demands v->start_alternate() for input and dealloc visitors, but visit_start_alternate() actually requires it for input and clone visitors. Fix the contract, and delete superfluous qapi_dealloc_start_alternate(). Signed-off-by: Markus Armbruster --- include/qapi/visitor-impl.h | 5 ++--- qapi/qapi-dealloc-visitor.c | 7 --- 2 files changed, 2 insertions(+), 10 deletions(-) Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH 06/13] qapi: Assert incomplete object occurs only in dealloc visitor
On 4/23/20 11:00 AM, Markus Armbruster wrote: Signed-off-by: Markus Armbruster --- include/qapi/visitor.h | 5 + qapi/qapi-visit-core.c | 5 + scripts/qapi/visit.py | 4 3 files changed, 14 insertions(+) Nice enforcement of the contract. Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH 05/13] qapi: Polish prose in visitor.h
On 4/23/20 11:00 AM, Markus Armbruster wrote: Signed-off-by: Markus Armbruster --- include/qapi/visitor.h | 104 + 1 file changed, 54 insertions(+), 50 deletions(-) Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH v21 QEMU 4/5] virtio-balloon: Implement support for page poison tracking feature
On Thu, Apr 23, 2020 at 9:02 AM David Hildenbrand wrote:
>
> On 23.04.20 16:46, Alexander Duyck wrote:
> > On Thu, Apr 23, 2020 at 1:11 AM David Hildenbrand wrote:
> >>
> >> On 22.04.20 20:21, Alexander Duyck wrote:
> >>> From: Alexander Duyck
> >>>
> >>> We need to make certain to advertise support for page poison tracking if
> >>> we want to actually get data on if the guest will be poisoning pages.
> >>>
> >>> Add a value for tracking the poison value being used if page poisoning is
> >>> enabled. With this we can determine if we will need to skip page reporting
> >>> when it is enabled in the future.
> >>
> >> Maybe add something about the semantics
> >>
> >> "VIRTIO_BALLOON_F_PAGE_POISON will not change the behavior of free page
> >> hinting or ordinary balloon inflation/deflation."
> >>
> >> I do wonder if we should just unconditionally enable
> >> VIRTIO_BALLOON_F_PAGE_POISON here, gluing it to the QEMU compat machine
> >> (via a property that is default-enabled, and disabled from compat
> >> machines).
> >>
> >> Because, as Michael said, knowing that the guest is using page poisoning
> >> might be interesting even if free page reporting is not around.
> >
> > I could do that. So if that is the case though would I disable page
> > reporting if it isn't enabled, or would I be enabling page poison if
> > page reporting is enabled?
>
>
> So, I would suggest this the following as a diff to this patch (the TODO part
> as a
> separate patch - we will have these compat properties briefly after the 5.0
> release)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index c1a444cb75..2e96cba4ff 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -28,6 +28,12 @@
> #include "hw/mem/nvdimm.h"
> #include "migration/vmstate.h"
>
> +// TODO: After 5.0 release
> +GlobalProperty hw_compat_5_0[] = {
> +{ "virtio-balloon-device", "page-hint", "false"},
> +};
> +const size_t hw_compat_5_0_len = G_N_ELEMENTS(hw_compat_5_0);
> +
> GlobalProperty hw_compat_4_2[] = {
> { "virtio-blk-device", "queue-size", "128"},
> { "virtio-scsi-device", "virtqueue_size", "128"},
Okay, so the bit above is for after 5_0 is released then? Is there a
way to queue up a reminder or something so we get to it when the time
comes, or I just need to watch for 5.0 to come out and submit a patch
then?
> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> index a4729f7fc9..5ff8a669cf 100644
> --- a/hw/virtio/virtio-balloon.c
> +++ b/hw/virtio/virtio-balloon.c
> @@ -924,6 +924,8 @@ static Property virtio_balloon_properties[] = {
> qemu_4_0_config_size, false),
> DEFINE_PROP_LINK("iothread", VirtIOBalloon, iothread, TYPE_IOTHREAD,
> IOThread *),
> +DEFINE_PROP_BIT("page-poison", VirtIOBalloon, host_features,
> +VIRTIO_BALLOON_F_PAGE_POISON, true),
> DEFINE_PROP_END_OF_LIST(),
> };
>
>
> What to do with page reporting depends: I would not implicitly enable
> features.
> I think we are talking about
>
> -device virtio-balloon-pci,...,page-poison=false,free-page-reporting=true
>
> a) Valid scenario. Fix Linux guests as we discussed to not use reporting if
> they rely on page poisoning.
Okay I will probably go this route and resubmit the patch I had
submitted before that only allows us to do page reporting if poisoning
is disabled on the guest kernel, or the page-poison is enabled on the
hypervisor.
> b) Invalid scenario. E.g., bail out when trying to realize that device
> ("'free-page-reporting' requires 'page-poison'").
>
> With new QEMU machines, this should not happen with
>
> -device virtio-balloon-pci,...,free-page-reporting=true
>
> as page-poison=true is the new default.
>
> What's your opinion?
I will just clean up and resubmit my earlier kernel patch, this time
without it messing with free page hinting.
> >
> >>>
> >>> Signed-off-by: Alexander Duyck
> >>> ---
> >>> hw/virtio/virtio-balloon.c |7 +++
> >>> include/hw/virtio/virtio-balloon.h |1 +
> >>> 2 files changed, 8 insertions(+)
> >>>
> >>> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> >>> index a1d6fb52c876..5effc8b4653b 100644
> >>> --- a/hw/virtio/virtio-balloon.c
> >>> +++ b/hw/virtio/virtio-balloon.c
> >>> @@ -634,6 +634,7 @@ static void virtio_balloon_get_config(VirtIODevice
> >>> *vdev, uint8_t *config_data)
> >>>
> >>> config.num_pages = cpu_to_le32(dev->num_pages);
> >>> config.actual = cpu_to_le32(dev->actual);
> >>> +config.poison_val = cpu_to_le32(dev->poison_val);
> >>>
> >>> if (dev->free_page_hint_status == FREE_PAGE_HINT_S_REQUESTED) {
> >>> config.free_page_hint_cmd_id =
> >>> @@ -697,6 +698,10 @@ static void virtio_balloon_set_config(VirtIODevice
> >>> *vdev,
> >>> qapi_event_send_balloon_change(vm_ram_size -
> >>> ((ram_addr_t) dev->actual <<
> >>> VIRTIO_BALLOON_PFN_SHIFT));
> >>> }
> >>> +dev->poison_val = 0;
Re: [PATCH 0/5] QEMU Gating CI
On Thu, 23 Apr 2020 at 18:37, Cleber Rosa wrote: > We're already using the shared x86 runners, but with a different goal. The > goal of the "Gating CI" is indeed to expand on non-x86 environments. We're > in a "chicken and egg" kind of situation, because we'd like to prove that > GitLab CI will allow QEMU to expand to very different runners and jobs, while > not really having all that hardware setup and publicly available at this time. We do have the S390 machine that IBM kindly made available to the project -- that is not a personal or Linaro machine, so there are no issues with giving you a login on that so you can set it up as a CI runner. Drop me an email if you want access to it. thanks -- PMM
Re: [PATCH] qcow2: Allow resize of images with internal snapshots
On 4/23/20 12:21 PM, Max Reitz wrote: The previous comment was incorrect as well, but actually qcow2_truncate_bitmaps_check() doesn’t return an error when there is any bitmap, but only if there are non-active bitmaps, or active bitmaps that cannot be modified. So for non-disabled bitmaps, we generally do happily proceed. The comment change is collateral (only because I noticed it in the diff); but I could indeed reword it slightly more accurately as: Check if bitmaps prevent a resize. Although bitmaps can be resized, there are situations where we don't know whether to set or clear new bits, so for now it's easiest to just prevent resize in those cases. But I don’t know whether that explanation is actually correct. I mean, that would apply for enabled active bitmaps just as well. But we do allow resizing an image with such bitmaps so it seems clear that we have some idea on what to do. (Or at least we pretend we do, for better or worse). (Which is that bdrv_dirty_bitmap_truncate() just calls hbitmap_truncate(), which fills the new space with zeroes.) The real reason we can’t resize with certain kinds of bitmaps present seems more like: (1) There are some bitmaps that can’t be written to, but we’d have to if we wanted to resize the image and they’re persistent, (2) The second case are bitmaps that are persistent but present in memory; we just haven’t implemented that case, I presume. So it seems less like a case of “We don’t know what to do”, but more a combination of “We can’t“ and “We haven‘t implemented this case, but it’s clear what to do if we did”. Indeed. So definitely a reason to split this change to a separate patch (and/or fix the code to finally implement it) Speaking of the image size. Is it intentional that the size is changed to 32 MB? Should amend work more like a transaction, in that we should at least do a loose check on whether the options can be changed before we touch the image? Yes, the commit message tried to call it out. It's a pre-existing problem - during amend, we DO make changes to the disk in one step, with no way to roll back those changes, even if a later step fails. Pre-patch, if you request an upgrade to v3 as well as a resize, but resize fails, you still end up with the image being changed to v3. That's no different from post-patch where if you request a resize and a downgrade to v2, the resize happens but not the downgrade. On the bright side, our current failure scenarios at least leave the resulting image viable, even if it is not the same as it was pre-attempt. Yes. OK. Okay, v2 will have a better commit message. Yeah. I don’t think anyone even would have realistic use for transactional amends. I suppose all users can easily split their amend calls with multiple options into multiple amends in the order that would be most likely reversible, if something went wrong along the way. (And that also works. I.e., downgrading/upgrading is probably the most easy to revert, but maybe you can only downgrade if your image has the correct size, so you potentially need to truncate it first. OTOH, I can’t imagine many people actually use qemu-img amend to downgrade qcow2 images anyway...) Indeed - any time that you worry that an interaction of multiple changes might fail half-way through, you can always serialize those changes yourself instead of hoping the tool sequences them correctly ;) I feel very much reminded of the LUKS keyslot discussion... (That is to say, my thoughts on this have little to do with this specific patch at this point.) Too true ! -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH 0/5] QEMU Gating CI
- Original Message - > From: "Daniel P. Berrangé" > To: "Cleber Rosa" > Cc: "Peter Maydell" , "Fam Zheng" > , "Thomas Huth" , > "Beraldo Leal" , "Erik Skultety" , > "Philippe Mathieu-Daudé" > , "Wainer Moschetta" , "Markus > Armbruster" , "Wainer dos > Santos Moschetta" , "QEMU Developers" > , "Willian Rampazzo" > , "Alex Bennée" , "Eduardo > Habkost" > Sent: Thursday, April 23, 2020 1:13:22 PM > Subject: Re: [PATCH 0/5] QEMU Gating CI > > On Thu, Apr 23, 2020 at 01:04:13PM -0400, Cleber Rosa wrote: > > > > > > - Original Message - > > > From: "Peter Maydell" > > > To: "Markus Armbruster" > > > Cc: "Fam Zheng" , "Thomas Huth" , > > > "Beraldo Leal" , "Erik > > > Skultety" , "Alex Bennée" , > > > "Wainer Moschetta" , > > > "QEMU Developers" , "Wainer dos Santos Moschetta" > > > , "Willian Rampazzo" > > > , "Cleber Rosa" , "Philippe > > > Mathieu-Daudé" , "Eduardo > > > Habkost" > > > Sent: Tuesday, April 21, 2020 8:53:49 AM > > > Subject: Re: [PATCH 0/5] QEMU Gating CI > > > > > > On Thu, 19 Mar 2020 at 16:33, Markus Armbruster > > > wrote: > > > > Peter Maydell writes: > > > > > I think we should start by getting the gitlab setup working > > > > > for the basic "x86 configs" first. Then we can try adding > > > > > a runner for s390 (that one's logistically easiest because > > > > > it is a project machine, not one owned by me personally or > > > > > by Linaro) once the basic framework is working, and expand > > > > > from there. > > > > > > > > Makes sense to me. > > > > > > > > Next steps to get this off the ground: > > > > > > > > * Red Hat provides runner(s) for x86 stuff we care about. > > > > > > > > * If that doesn't cover 'basic "x86 configs" in your judgement, we > > > > fill the gaps as described below under "Expand from there". > > > > > > > > * Add an s390 runner using the project machine you mentioned. > > > > > > > > * Expand from there: identify the remaining gaps, map them to people / > > > > organizations interested in them, and solicit contributions from > > > > these > > > > guys. > > > > > > > > A note on contributions: we need both hardware and people. By people I > > > > mean maintainers for the infrastructure, the tools and all the runners. > > > > Cleber & team are willing to serve for the infrastructure, the tools > > > > and > > > > the Red Hat runners. > > > > > > So, with 5.0 nearly out the door it seems like a good time to check > > > in on this thread again to ask where we are progress-wise with this. > > > My impression is that this patchset provides most of the scripting > > > and config side of the first step, so what we need is for RH to provide > > > an x86 runner machine and tell the gitlab CI it exists. I appreciate > > > that the whole coronavirus and working-from-home situation will have > > > upended everybody's plans, especially when actual hardware might > > > be involved, but how's it going ? > > > > > > > Hi Peter, > > > > You hit the nail in the head here. We were affected indeed with our > > ability > > to move some machines from one lab to another (across the country), but > > we're > > actively working on it. > > For x86, do we really need to be using custom runners ? > Hi Daniel, We're already using the shared x86 runners, but with a different goal. The goal of the "Gating CI" is indeed to expand on non-x86 environments. We're in a "chicken and egg" kind of situation, because we'd like to prove that GitLab CI will allow QEMU to expand to very different runners and jobs, while not really having all that hardware setup and publicly available at this time. My experiments were really around that point, I mean, confirming that we can grow the number of architectures/runners/jobs/configurations to provide a coverage equal or greater to what Peter already does. > With GitLab if someone forks the repo to their personal namespace, they > cannot use any custom runners setup by the origin project. So if we use > custom runners for x86, people forking won't be able to run the GitLab > CI jobs. > They will continue to be able use the jobs and runners already defined in the .gitlab-ci.yml file. This work will only affect people pushing to the/a "staging" branch. > As a sub-system maintainer I wouldn't like this, because I ideally want > to be able to run the same jobs on my staging tree, that Peter will run > at merge time for the PULL request I send. > If you're looking for symmetry between any PR and "merge time" jobs, the only solution is to allow any PR to access all the diverse set of non-shared machines we're hoping to have in the near future. This may be something we'll get to, but I doubt we can tackle it in the near future now. > Thus my strong preference would be to use the GitLab runners in every > scenario where they are viable to use. Only use custom runners in the > cases where GitLab runners are clearly inadequate for our needs. > > Based on what we've setup in GitLab for libvirt, the
Re: [PATCH 04/13] qapi: Document @errp usage more thoroughly in visitor.h
On 4/23/20 11:00 AM, Markus Armbruster wrote: Signed-off-by: Markus Armbruster --- include/qapi/visitor.h | 37 +++-- 1 file changed, 23 insertions(+), 14 deletions(-) Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH 02/13] qapi: Fix the virtual walk example in visitor.h's big comment
On 4/23/20 11:00 AM, Markus Armbruster wrote: Call visit_check_list(). Missed in commit a4a1c70dc7 "qapi: Make input visitors detect unvisited list tails". Drop an irrelevant error_propagate() while there. Aha - you found this because of your error cleanup work ;) Signed-off-by: Markus Armbruster --- include/qapi/visitor.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH 03/13] qapi: Fix typo in visit_start_list()'s contract
On 4/23/20 11:00 AM, Markus Armbruster wrote: Signed-off-by: Markus Armbruster --- include/qapi/visitor.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) Too much copy-and-paste in the original ;) Reviewed-by: Eric Blake diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h index 7f63e4c381..c5d0ce9184 100644 --- a/include/qapi/visitor.h +++ b/include/qapi/visitor.h @@ -345,9 +345,9 @@ void visit_end_struct(Visitor *v, void **obj); * input visitors set *@list to NULL. * * After visit_start_list() succeeds, the caller may visit its members - * one after the other. A real visit (where @obj is non-NULL) uses + * one after the other. A real visit (where @list is non-NULL) uses * visit_next_list() for traversing the linked list, while a virtual - * visit (where @obj is NULL) uses other means. For each list + * visit (where @list is NULL) uses other means. For each list * element, call the appropriate visit_type_FOO() with name set to * NULL and obj set to the address of the value member of the list * element. Finally, visit_end_list() needs to be called with the -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH v4 22/30] qcow2: Fix offset calculation in handle_dependencies()
On Wed 22 Apr 2020 02:38:54 PM CEST, Vladimir Sementsov-Ogievskiy wrote: > 17.03.2020 21:16, Alberto Garcia wrote: >> l2meta_cow_start() and l2meta_cow_end() are not necessarily >> cluster-aligned if the image has subclusters, so update the >> calculation of old_start and old_end to guarantee that no two requests >> try to write on the same cluster. >> >> Signed-off-by: Alberto Garcia >> Reviewed-by: Max Reitz > > Somehow, this patch say me "hey, there may be a lot of other small > places, which we forget to fix about subclusters, and you have no > idea, how to find and check them all" :) Probably the only way is > reviewing the whole qcow2 code, but it's too huge task.. [this is just > thinking out loud] :-) > Actually, you call it "Fix", and it seems to be a fix for your "[PATCH > v4 17/30] qcow2: Add subcluster support to > calculate_l2_meta()". Shouldn't it be squashed in? Maybe it't not a bad idea... I'll have a look. Berto
Re: [PATCH 01/13] qapi: Belatedly update visitor.h's big comment for QAPI modules
On 4/23/20 11:00 AM, Markus Armbruster wrote: Signed-off-by: Markus Armbruster --- include/qapi/visitor.h | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH v2 09/36] tcg: Consolidate 3 bits into enum TCGTempKind
On Thu, Apr 23, 2020 at 08:40:10AM -0700, Richard Henderson wrote:
> On 4/23/20 2:00 AM, Philippe Mathieu-Daudé wrote:
> >>> @@ -1885,12 +1896,17 @@ static char *tcg_get_arg_str_ptr(TCGContext *s,
> >>> char *buf, int buf_size,
> >>> {
> >>> int idx = temp_idx(ts);
> >>>
> >>> -if (ts->temp_global) {
> >>> +switch (ts->kind) {
> >>> +case TEMP_FIXED:
> >>> +case TEMP_GLOBAL:
> >>> pstrcpy(buf, buf_size, ts->name);
> >>> -} else if (ts->temp_local) {
> >>> +break;
> >>> +case TEMP_LOCAL:
> >>> snprintf(buf, buf_size, "loc%d", idx - s->nb_globals);
> >>> -} else {
> >>> +break;
> >>> +case TEMP_NORMAL:
> >>> snprintf(buf, buf_size, "tmp%d", idx - s->nb_globals);
> >>> +break;
> >>> }
> >>
> >> Hmm, why this switch doesn't have:
> >>
> >> default:
> >> g_assert_not_reached();
> >>
> >> like the other ones?
> >
> > ... then all switch should have a default case, as noticed Aleksandar.
>
> There's a bit of a conflict between wanting to use -Werror -Wswitch, and
> making
> sure every switch has a default.
>
> With the former, you get a compiler error of the form
>
> error: enumeration value ‘FOO’ not handled in switch
>
> which lets you easily find places that need adjustment enumerators are added.
FYI, -Wswitch-enum can deal with this. This gives a warning about
missing enum cases, even if there is a default statement:
[quote]
'-Wswitch-enum'
Warn whenever a 'switch' statement has an index of enumerated type
and lacks a 'case' for one or more of the named codes of that
enumeration. 'case' labels outside the enumeration range also
provoke warnings when this option is used. The only difference
between '-Wswitch' and this option is that this option gives a
warning about an omitted enumeration code even if there is a
'default' label.
[/quote]
If we want to have a default: in every switch, then we could also
use -Wswitch-default too !
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH v4 21/30] qcow2: Add subcluster support to check_refcounts_l2()
On Wed 22 Apr 2020 02:06:56 PM CEST, Vladimir Sementsov-Ogievskiy wrote: > 17.03.2020 21:16, Alberto Garcia wrote: >> Setting the QCOW_OFLAG_ZERO bit of the L2 entry is forbidden if an >> image has subclusters. Instead, the individual 'all zeroes' bits must >> be used. >> >> Signed-off-by: Alberto Garcia >> Reviewed-by: Max Reitz > > Patch itself seems correct.. Still, would be good also to check, is > QCOW_OFLAG_ZERO set in subclustres case and add corresponding > corruptions++, and may be even fix (by using > QCOW_L2_BITMAP_ALL_ZEROES instead) I'll add it to my TODO list for a later patch. Berto
Re: [PATCH] qcow2: Allow resize of images with internal snapshots
On 23.04.20 16:35, Eric Blake wrote:
> On 4/23/20 8:55 AM, Max Reitz wrote:
>> On 22.04.20 22:53, Eric Blake wrote:
>>> We originally refused to allow resize of images with internal
>>> snapshots because the v2 image format did not require the tracking of
>>> snapshot size, making it impossible to safely revert to a snapshot
>>> with a different size than the current view of the image. But the
>>> snapshot size tracking was rectified in v3, and our recent fixes to
>>> qemu-img amend (see 0a85af35) guarantee that we always have a valid
>>> snapshot size. Thus, we no longer need to artificially limit image
>>> resizes, but it does become one more thing that would prevent a
>>> downgrade back to v2. And now that we support different-sized
>>> snapshots, it's also easy to fix reverting to a snapshot to apply the
>>> new size.
>>>
>>> Upgrade iotest 61 to cover this (we previously had NO coverage of
>>> refusal to resize while snapshots exist). Note that the amend process
>>> can fail but still have effects: in particular, since we break things
>>> into upgrade, resize, downgrade, if a failure does not happen until a
>>> later phase (such as the downgrade attempt), earlier steps are still
>>> visible (a truncation and downgrade attempt will fail, but only after
>>> truncating data). But even before this patch, an attempt to upgrade
>>> and resize would fail but only after changing the image to v3. In
>>> some sense, partial image changes on failure are inevitible, since we
>>> can't avoid a mid-change EIO (if you are trying to amend more than one
>>> thing at once, but something fails, I hope you have a backup image).
>>>
>
>>> @@ -775,10 +776,22 @@ int qcow2_snapshot_goto(BlockDriverState *bs,
>>> const char *snapshot_id)
>>> }
>>>
>>> if (sn->disk_size != bs->total_sectors * BDRV_SECTOR_SIZE) {
>>> - error_report("qcow2: Loading snapshots with different disk "
>>> - "size is not implemented");
>>> - ret = -ENOTSUP;
>>> - goto fail;
>>> + BlockBackend *blk = blk_new(bdrv_get_aio_context(bs),
>>> + BLK_PERM_RESIZE, BLK_PERM_ALL);
>>> + ret = blk_insert_bs(blk, bs, _err);
>>
>> I wonder whether maybe we should reintroduce blk_new_with_bs().
>
> This code segment is copied from what 'qemu-img amend' does, so adding a
> helper function would indeed make life a bit easier for more than one
> spot in the code base. Separate patch, obviously.
>
>
>>> +++ b/block/qcow2.c
>>> @@ -3988,14 +3988,21 @@ static int coroutine_fn
>>> qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
>>>
>>> qemu_co_mutex_lock(>lock);
>>>
>>> - /* cannot proceed if image has snapshots */
>>> - if (s->nb_snapshots) {
>>> - error_setg(errp, "Can't resize an image which has snapshots");
>>> + /*
>>> + * Even though we store snapshot size for all images, it was not
>>> + * required until v3, so it is not safe to proceed for v2.
>>> + */
>>> + if (s->nb_snapshots && s->qcow_version < 3) {
>>> + error_setg(errp, "Can't resize a v2 image which has
>>> snapshots");
>>> ret = -ENOTSUP;
>>> goto fail;
>>> }
>>>
>>> - /* cannot proceed if image has bitmaps */
>>> + /*
>>> + * For now, it's easier to not proceed if image has bitmaps, even
>>> + * though we could resize bitmaps, because it is not obvious
>>> + * whether new bits should be set or clear.
>>
>> The previous comment was incorrect as well, but actually
>> qcow2_truncate_bitmaps_check() doesn’t return an error when there is any
>> bitmap, but only if there are non-active bitmaps, or active bitmaps that
>> cannot be modified. So for non-disabled bitmaps, we generally do
>> happily proceed.
>
> The comment change is collateral (only because I noticed it in the
> diff); but I could indeed reword it slightly more accurately as:
>
> Check if bitmaps prevent a resize. Although bitmaps can be resized,
> there are situations where we don't know whether to set or clear new
> bits, so for now it's easiest to just prevent resize in those cases.
But I don’t know whether that explanation is actually correct. I mean,
that would apply for enabled active bitmaps just as well. But we do
allow resizing an image with such bitmaps so it seems clear that we have
some idea on what to do. (Or at least we pretend we do, for better or
worse).
(Which is that bdrv_dirty_bitmap_truncate() just calls
hbitmap_truncate(), which fills the new space with zeroes.)
The real reason we can’t resize with certain kinds of bitmaps present
seems more like:
(1) There are some bitmaps that can’t be written to, but we’d have to if
we wanted to resize the image and they’re persistent,
(2) The second case are bitmaps that are persistent but present in
memory; we just haven’t implemented that case, I presume.
So it seems less like a case of “We don’t know what to do”, but more a
combination of “We can’t“ and “We haven‘t implemented
Re: ARM SVE issues with non "standard" vector lengths
On Thu, Apr 23, 2020 at 5:19 PM Richard Henderson wrote: > > On 4/23/20 6:59 AM, Laurent Desnogues wrote: > > 2. sve_zip_p > > > > This generates extraneous data in the higher part of the result. > > > > I hit this when I got a wrong result on an instruction that ends up > > using sve_cntp which counts all bits set in each 64-bit chunk. There > > might be some other instructions beyond ZIP that generate extra data > > that would break sve_cntp. So perhaps it'd be easier to fix sve_cmtp > > (and hope that it's the only function that uses bits beyond vector > > length...). > > There are quite a few places that assume (and some that assert, such as the > load/store paths) that there are no predicate bits set beyond VL. > > I will fix zip. Looking at the code I think sve_punpk_p is affected too. Thanks, Laurent > > > r~
Re: [RFC 0/3] 64bit block-layer part I
Am 30.03.2020 um 16:18 hat Vladimir Sementsov-Ogievskiy geschrieben: > Hi all! > > There is an idea to make NBD protocol extension to support 64bit > write-zero/discard/block-status commands (commands, which doesn't > transfer user data). It's needed to increase performance of zeroing > large ranges (up to the whole image). Zeroing of the whole image is used > as first step of mirror job, qemu-img convert, it should be also used at > start of backup actually.. > > We need to support it in block-layer, so we want 64bit write_zeros. > Currently driver handler now have int bytes parameter. > > write_zeros path goes through normal pwritev, so we need 64bit write, > and then we need 64bit read for symmetry, and better, let's make all io > path work with 64bit bytes parameter. > > Actually most of block-layer already have 64bit parameters: offset is > sometimes int64_t and sometimes uint64_t. bytes parameter is one of > int64_t, uint64_t, int, unsigned int... > > I think we need one type for all of this, and this one type is int64_t. > Signed int64_t is a bit better than uint64_t: you can use same variable > to get some result (including error < 0) and than reuse it as an > argument without any type conversion. > > So, I propose, as a first step, convert all uint64_t parameters to > int64_t. > > Still, I don't have good idea of how to split this into more than 3 > patches, so, this is an RFC. I think the split in three patches isn't too bad because it's not a whole lot of code. But of course, it is little code that has lots of implications which does make it hard to review. If we think that we might bisect a bug in the series later, maybe it would be better to split it into more patches. write/write_zeroes has to be a single thing, I'm afraid. But I guess read could be a separate patch, as could be copy_range. Not sure about discard. > What's next? > > Converting write_zero and discard is not as simple: we can't just > s/int/uint64_t/, as some functions may use some int variables for > calculations and this will be broken by something larger than int. > > So, I think the simplest way is to add .bdrv_co_pwritev_zeros64 and > .bdrv_co_pdiscard64 and update drivers one-by-one. If at some point all > drivers updated - drop unused 32bit functions, and then drop "64" suffix > from API. If not - we'll live with both APIs. We already have too many unfinished conversions in QEMU, let's not add one more. Fortunately, we already have a tool that could help us here: Things like bs->bl.max_pwrite_zeroes. We could make BDRV_REQUEST_MAX_BYTES the default value and only drivers that override it can get bigger requests. > Another thing to do is updating default limiting of request (currently > they are limited to INT_MAX). As above, I wouldn't update the default, but rather enable drivers to overload the default with a larger value. This will involve changing some places where we use MIN() between INT_MAX and the driver's value. > Then we may move some drivers to 64bit discard/write_zero: I think about > qcow2, file-posix and nbd (as a proof-of-concept for already proposed > NBD extension). Makes sense to me. Kevin
Re: [PATCH 0/5] QEMU Gating CI
On Thu, Apr 23, 2020 at 01:04:13PM -0400, Cleber Rosa wrote: > > > - Original Message - > > From: "Peter Maydell" > > To: "Markus Armbruster" > > Cc: "Fam Zheng" , "Thomas Huth" , > > "Beraldo Leal" , "Erik > > Skultety" , "Alex Bennée" , > > "Wainer Moschetta" , > > "QEMU Developers" , "Wainer dos Santos Moschetta" > > , "Willian Rampazzo" > > , "Cleber Rosa" , "Philippe > > Mathieu-Daudé" , "Eduardo > > Habkost" > > Sent: Tuesday, April 21, 2020 8:53:49 AM > > Subject: Re: [PATCH 0/5] QEMU Gating CI > > > > On Thu, 19 Mar 2020 at 16:33, Markus Armbruster wrote: > > > Peter Maydell writes: > > > > I think we should start by getting the gitlab setup working > > > > for the basic "x86 configs" first. Then we can try adding > > > > a runner for s390 (that one's logistically easiest because > > > > it is a project machine, not one owned by me personally or > > > > by Linaro) once the basic framework is working, and expand > > > > from there. > > > > > > Makes sense to me. > > > > > > Next steps to get this off the ground: > > > > > > * Red Hat provides runner(s) for x86 stuff we care about. > > > > > > * If that doesn't cover 'basic "x86 configs" in your judgement, we > > > fill the gaps as described below under "Expand from there". > > > > > > * Add an s390 runner using the project machine you mentioned. > > > > > > * Expand from there: identify the remaining gaps, map them to people / > > > organizations interested in them, and solicit contributions from these > > > guys. > > > > > > A note on contributions: we need both hardware and people. By people I > > > mean maintainers for the infrastructure, the tools and all the runners. > > > Cleber & team are willing to serve for the infrastructure, the tools and > > > the Red Hat runners. > > > > So, with 5.0 nearly out the door it seems like a good time to check > > in on this thread again to ask where we are progress-wise with this. > > My impression is that this patchset provides most of the scripting > > and config side of the first step, so what we need is for RH to provide > > an x86 runner machine and tell the gitlab CI it exists. I appreciate > > that the whole coronavirus and working-from-home situation will have > > upended everybody's plans, especially when actual hardware might > > be involved, but how's it going ? > > > > Hi Peter, > > You hit the nail in the head here. We were affected indeed with our ability > to move some machines from one lab to another (across the country), but we're > actively working on it. For x86, do we really need to be using custom runners ? With GitLab if someone forks the repo to their personal namespace, they cannot use any custom runners setup by the origin project. So if we use custom runners for x86, people forking won't be able to run the GitLab CI jobs. As a sub-system maintainer I wouldn't like this, because I ideally want to be able to run the same jobs on my staging tree, that Peter will run at merge time for the PULL request I send. Thus my strong preference would be to use the GitLab runners in every scenario where they are viable to use. Only use custom runners in the cases where GitLab runners are clearly inadequate for our needs. Based on what we've setup in GitLab for libvirt, the shared runners they have work fine for x86. Just need the environments you are testing to be provided as Docker containers (you can actually build and cache the container images during your CI job too). IOW, any Linux distro build and test jobs should be able to use shared runners on x86, and likewise mingw builds. Custom runners should only be needed if the jobs need todo *BSD / macOS builds, and/or have access to specific hardware devices for some reason. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
[PATCH v1 10/14] gdbstub/linux-user: support debugging over a unix socket
While debugging over TCP is fairly straightforward now we have test
cases that want to orchestrate via make and currently a parallel build
fails as two processes can't use the same listening port. While system
emulation offers a wide cornucopia of connection methods thanks to the
chardev abstraction we are a little more limited for linux user.
Thankfully the programming API for a TCP socket and a local UNIX
socket is pretty much the same once it's set up.
Signed-off-by: Alex Bennée
---
include/exec/gdbstub.h | 14 --
bsd-user/main.c| 8 +--
gdbstub.c | 107 ++---
linux-user/main.c | 12 ++---
4 files changed, 108 insertions(+), 33 deletions(-)
diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index 4a2b8e3089..94d8f83e92 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -177,11 +177,15 @@ static inline uint8_t * gdb_get_reg_ptr(GByteArray *buf,
int len)
#endif
-#ifdef CONFIG_USER_ONLY
-int gdbserver_start(int);
-#else
-int gdbserver_start(const char *port);
-#endif
+/**
+ * gdbserver_start: start the gdb server
+ * @port_or_device: connection spec for gdb
+ *
+ * For CONFIG_USER this is either a tcp port or a path to a fifo. For
+ * system emulation you can use a full chardev spec for your gdbserver
+ * port.
+ */
+int gdbserver_start(const char *port_or_device);
void gdbserver_cleanup(void);
diff --git a/bsd-user/main.c b/bsd-user/main.c
index aef5531628..0bfe46cff9 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -738,7 +738,7 @@ int main(int argc, char **argv)
CPUState *cpu;
int optind;
const char *r;
-int gdbstub_port = 0;
+const char *gdbstub = NULL;
char **target_environ, **wrk;
envlist_t *envlist = NULL;
char *trace_file = NULL;
@@ -814,7 +814,7 @@ int main(int argc, char **argv)
exit(1);
}
} else if (!strcmp(r, "g")) {
-gdbstub_port = atoi(argv[optind++]);
+gdbstub = g_strdup(argv[optind++]);
} else if (!strcmp(r, "r")) {
qemu_uname_release = argv[optind++];
} else if (!strcmp(r, "cpu")) {
@@ -1124,8 +1124,8 @@ int main(int argc, char **argv)
#error unsupported target CPU
#endif
-if (gdbstub_port) {
-gdbserver_start (gdbstub_port);
+if (gdbstub) {
+gdbserver_start(gdbstub);
gdb_handlesig(cpu, 0);
}
cpu_loop(env);
diff --git a/gdbstub.c b/gdbstub.c
index 8c53cc0e1c..51e2ab9110 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -355,6 +355,7 @@ typedef struct GDBState {
int signal;
#ifdef CONFIG_USER_ONLY
int fd;
+char *socket_path;
int running_state;
#else
CharBackend chr;
@@ -2962,6 +2963,9 @@ void gdb_exit(CPUArchState *env, int code)
return;
}
#ifdef CONFIG_USER_ONLY
+ if (gdbserver_state.socket_path) {
+ unlink(gdbserver_state.socket_path);
+ }
if (gdbserver_state.fd < 0) {
return;
}
@@ -3034,7 +3038,6 @@ gdb_handlesig(CPUState *cpu, int sig)
n = read(gdbserver_state.fd, buf, 256);
if (n > 0) {
int i;
-
for (i = 0; i < n; i++) {
gdb_read_byte(buf[i]);
}
@@ -3066,7 +3069,66 @@ void gdb_signalled(CPUArchState *env, int sig)
put_packet(buf);
}
-static bool gdb_accept(int gdb_fd)
+static void gdb_accept_init(int fd)
+{
+init_gdbserver_state();
+create_default_process(_state);
+gdbserver_state.processes[0].attached = true;
+gdbserver_state.c_cpu = gdb_first_attached_cpu();
+gdbserver_state.g_cpu = gdbserver_state.c_cpu;
+gdbserver_state.fd = fd;
+gdb_has_xml = false;
+}
+
+static bool gdb_accept_socket(int gdb_fd)
+{
+int fd;
+
+for(;;) {
+fd = accept(gdb_fd, NULL, NULL);
+if (fd < 0 && errno != EINTR) {
+perror("accept socket");
+return false;
+} else if (fd >= 0) {
+qemu_set_cloexec(fd);
+break;
+}
+}
+
+gdb_accept_init(fd);
+return true;
+}
+
+static int gdbserver_open_socket(const char *path)
+{
+struct sockaddr_un sockaddr;
+int fd, ret;
+
+fd = socket(AF_UNIX, SOCK_STREAM, 0);
+if (fd < 0) {
+perror("create socket");
+return -1;
+}
+
+sockaddr.sun_family = AF_UNIX;
+pstrcpy(sockaddr.sun_path, sizeof(sockaddr.sun_path)-1, path);
+ret = bind(fd, (struct sockaddr *), sizeof(sockaddr));
+if (ret < 0) {
+perror("bind socket");
+close(fd);
+return -1;
+}
+ret = listen(fd, 1);
+if (ret < 0) {
+perror("listen socket");
+close(fd);
+return -1;
+}
+
+return fd;
+}
+
+static bool gdb_accept_tcp(int gdb_fd)
{
struct sockaddr_in sockaddr;
socklen_t len;
@@ -3091,17 +3153,11 @@ static bool gdb_accept(int gdb_fd)
return false;
}
-init_gdbserver_state();
-create_default_process(_state);
-
[PATCH v1 13/14] .travis.yml: show free disk space at end of run
Signed-off-by: Alex Bennée --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 2fd63eceaa..a4c3c6c805 100644 --- a/.travis.yml +++ b/.travis.yml @@ -113,6 +113,7 @@ script: $(exit $BUILD_RC); fi after_script: + - df -h - if command -v ccache ; then ccache --show-stats ; fi -- 2.20.1
[PATCH v1 12/14] tests/tcg: add a multiarch linux-user gdb test
When the gdbstub code was converted to the new API we missed a few
snafus in the various guests. Add a simple gdb test script which can
be used on all our linux-user guests to check for obvious failures.
Signed-off-by: Alex Bennée
---
v2
- use EXTRA_RUNS to queue the tests so as not to break plugins
---
tests/tcg/aarch64/Makefile.target | 5 +-
tests/tcg/cris/Makefile.target | 1 +
tests/tcg/multiarch/Makefile.target | 14 +
tests/tcg/multiarch/gdbstub/sha1.py | 81 +
4 files changed, 98 insertions(+), 3 deletions(-)
create mode 100644 tests/tcg/multiarch/gdbstub/sha1.py
diff --git a/tests/tcg/aarch64/Makefile.target
b/tests/tcg/aarch64/Makefile.target
index d99b2a9ece..312f36cde5 100644
--- a/tests/tcg/aarch64/Makefile.target
+++ b/tests/tcg/aarch64/Makefile.target
@@ -54,9 +54,6 @@ sve-ioctls: CFLAGS+=-march=armv8.1-a+sve
ifneq ($(HAVE_GDB_BIN),)
GDB_SCRIPT=$(SRC_PATH)/tests/guest-debug/run-test.py
-AARCH64_TESTS += gdbstub-sysregs gdbstub-sve-ioctls
-
-.PHONY: gdbstub-sysregs gdbstub-sve-ioctls
run-gdbstub-sysregs: sysregs
$(call run-test, $@, $(GDB_SCRIPT) \
--gdb $(HAVE_GDB_BIN) \
@@ -70,6 +67,8 @@ run-gdbstub-sve-ioctls: sve-ioctls
--qemu $(QEMU) --qargs "$(QEMU_OPTS)" \
--bin $< --test $(AARCH64_SRC)/gdbstub/test-sve-ioctl.py, \
"basic gdbstub SVE ZLEN support")
+
+EXTRA_RUNS += run-gdbstub-sysregs run-gdbstub-sve-ioctls
endif
endif
diff --git a/tests/tcg/cris/Makefile.target b/tests/tcg/cris/Makefile.target
index 24c7f2e761..e72d3cbdb2 100644
--- a/tests/tcg/cris/Makefile.target
+++ b/tests/tcg/cris/Makefile.target
@@ -23,6 +23,7 @@ CRIS_RUNS = $(patsubst %, run-%, $(CRIS_USABLE_TESTS))
# override the list of tests, as we can't build the multiarch tests
TESTS = $(CRIS_USABLE_TESTS)
+EXTRA_RUNS =
VPATH = $(CRIS_SRC)
AS = $(CC) -x assembler-with-cpp
diff --git a/tests/tcg/multiarch/Makefile.target
b/tests/tcg/multiarch/Makefile.target
index 035b09c853..51fb75ecfd 100644
--- a/tests/tcg/multiarch/Makefile.target
+++ b/tests/tcg/multiarch/Makefile.target
@@ -42,5 +42,19 @@ run-test-mmap-%: test-mmap
$(call run-test, test-mmap-$*, $(QEMU) -p $* $<,\
"$< ($* byte pages) on $(TARGET_NAME)")
+ifneq ($(HAVE_GDB_BIN),)
+GDB_SCRIPT=$(SRC_PATH)/tests/guest-debug/run-test.py
+
+run-gdbstub-sha1: sha1
+ $(call run-test, $@, $(GDB_SCRIPT) \
+ --gdb $(HAVE_GDB_BIN) \
+ --qemu $(QEMU) --qargs "$(QEMU_OPTS)" \
+ --bin $< --test $(MULTIARCH_SRC)/gdbstub/sha1.py, \
+ "basic gdbstub support")
+
+EXTRA_RUNS += run-gdbstub-sha1
+endif
+
+
# Update TESTS
TESTS += $(MULTIARCH_TESTS)
diff --git a/tests/tcg/multiarch/gdbstub/sha1.py
b/tests/tcg/multiarch/gdbstub/sha1.py
new file mode 100644
index 00..734553b98b
--- /dev/null
+++ b/tests/tcg/multiarch/gdbstub/sha1.py
@@ -0,0 +1,81 @@
+from __future__ import print_function
+#
+# A very simple smoke test for debugging the SHA1 userspace test on
+# each target.
+#
+# This is launched via tests/guest-debug/run-test.py
+#
+
+import gdb
+import sys
+
+initial_vlen = 0
+failcount = 0
+
+def report(cond, msg):
+"Report success/fail of test"
+if cond:
+print("PASS: %s" % (msg))
+else:
+print("FAIL: %s" % (msg))
+global failcount
+failcount += 1
+
+def check_break(sym_name):
+"Setup breakpoint, continue and check we stopped."
+sym, ok = gdb.lookup_symbol(sym_name)
+bp = gdb.Breakpoint(sym_name)
+
+gdb.execute("c")
+
+# hopefully we came back
+end_pc = gdb.parse_and_eval('$pc')
+report(bp.hit_count == 1,
+ "break @ %s (%s %d hits)" % (end_pc, sym.value(), bp.hit_count))
+
+bp.delete()
+
+def run_test():
+"Run through the tests one by one"
+
+check_break("SHA1Init")
+
+# check step and inspect values
+gdb.execute("next")
+val_ctx = gdb.parse_and_eval("context->state[0]")
+exp_ctx = 0x67452301
+report(int(val_ctx) == exp_ctx, "context->state[0] == %x" % exp_ctx);
+
+gdb.execute("next")
+val_ctx = gdb.parse_and_eval("context->state[1]")
+exp_ctx = 0xEFCDAB89
+report(int(val_ctx) == exp_ctx, "context->state[1] == %x" % exp_ctx);
+
+# finally check we don't barf inspecting registers
+gdb.execute("info registers")
+
+#
+# This runs as the script it sourced (via -x, via run-test.py)
+#
+try:
+inferior = gdb.selected_inferior()
+arch = inferior.architecture()
+print("ATTACHED: %s" % arch.name())
+except (gdb.error, AttributeError):
+print("SKIPPING (not connected)", file=sys.stderr)
+exit(0)
+
+try:
+# These are not very useful in scripts
+gdb.execute("set pagination off")
+gdb.execute("set confirm off")
+
+# Run the actual tests
+run_test()
+except (gdb.error):
+print ("GDB Exception: %s" % (sys.exc_info()[0]))
+failcount += 1
+pass
+
+print("All tests complete: %d failures" %
