Re: [PATCH 0/2] target/s390x: s390_probe_access fixes
On 23.08.22 23:38, Richard Henderson wrote: > First, as pointed out by David; second by inspection. > > I really wish there were a better way to structure this, > but alas, I don't see any alternatives that aren't just > different but similar amounts of ugly. > The only feasible way would be having a arch-specific callback from inside the probe code that would, similarly to tlb_fill code for !USER store these values in the cpu environment -- then we could similarly just look them up after the probe access. -- Thanks, David / dhildenb
Re: [PATCH v8 02/12] s390x/cpu_topology: CPU topology objects and structures
On 23/08/2022 19.41, Pierre Morel wrote:
On 8/23/22 15:30, Thomas Huth wrote:
On 20/06/2022 16.03, Pierre Morel wrote:
We use new objects to have a dynamic administration of the CPU topology.
The highest level object in this implementation is the s390 book and
in this first implementation of CPU topology for S390 we have a single
book.
The book is built as a SYSBUS bridge during the CPU initialization.
Other objects, sockets and core will be built after the parsing
of the QEMU -smp argument.
Every object under this single book will be build dynamically
immediately after a CPU has be realized if it is needed.
The CPU will fill the sockets once after the other, according to the
number of core per socket defined during the smp parsing.
Each CPU inside a socket will be represented by a bit in a 64bit
unsigned long. Set on plug and clear on unplug of a CPU.
For the S390 CPU topology, thread and cores are merged into
topology cores and the number of topology cores is the multiplication
of cores by the numbers of threads.
Signed-off-by: Pierre Morel
---
hw/s390x/cpu-topology.c | 391
hw/s390x/meson.build | 1 +
hw/s390x/s390-virtio-ccw.c | 6 +
include/hw/s390x/cpu-topology.h | 74 ++
target/s390x/cpu.h | 47
5 files changed, 519 insertions(+)
create mode 100644 hw/s390x/cpu-topology.c
create mode 100644 include/hw/s390x/cpu-topology.h
...
+bool s390_topology_new_cpu(MachineState *ms, int core_id, Error **errp)
+{
+ S390TopologyBook *book;
+ S390TopologySocket *socket;
+ S390TopologyCores *cores;
+ int nb_cores_per_socket;
+ int origin, bit;
+
+ book = s390_get_topology();
+
+ nb_cores_per_socket = ms->smp.cores * ms->smp.threads;
+
+ socket = s390_get_socket(ms, book, core_id / nb_cores_per_socket,
errp);
+ if (!socket) {
+ return false;
+ }
+
+ /*
+ * At the core level, each CPU is represented by a bit in a 64bit
+ * unsigned long. Set on plug and clear on unplug of a CPU.
+ * The firmware assume that all CPU in the core description have the
same
+ * type, polarization and are all dedicated or shared.
+ * In the case a socket contains CPU with different type, polarization
+ * or dedication then they will be defined in different CPU containers.
+ * Currently we assume all CPU are identical and the only reason to
have
+ * several S390TopologyCores inside a socket is to have more than 64
CPUs
+ * in that case the origin field, representing the offset of the
first CPU
+ * in the CPU container allows to represent up to the maximal number of
+ * CPU inside several CPU containers inside the socket container.
+ */
+ origin = 64 * (core_id / 64);
Maybe faster:
origin = core_id & ~63;
By the way, where is this limitation to 64 coming from? Just because we're
using a "unsigned long" for now? Or is this a limitation from the
architecture?
+ cores = s390_get_cores(ms, socket, origin, errp);
+ if (!cores) {
+ return false;
+ }
+
+ bit = 63 - (core_id - origin);
+ set_bit(bit, &cores->mask);
+ cores->origin = origin;
+
+ return true;
+}
...
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index cc3097bfee..a586875b24 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -43,6 +43,7 @@
#include "sysemu/sysemu.h"
#include "hw/s390x/pv.h"
#include "migration/blocker.h"
+#include "hw/s390x/cpu-topology.h"
static Error *pv_mig_blocker;
@@ -89,6 +90,7 @@ static void s390_init_cpus(MachineState *machine)
/* initialize possible_cpus */
mc->possible_cpu_arch_ids(machine);
+ s390_topology_setup(machine);
Is this safe with regards to migration? Did you tried a ping-pong
migration from an older QEMU to a QEMU with your modifications and back to
the older one? If it does not work, we might need to wire this setup to
the machine types...
I checked with the follow-up series :
OLD-> NEW -> OLD -> NEW
It is working fine, of course we need to fence the CPU topology facility
with ctop=off on the new QEMU to avoid authorizing the new instructions, PTF
and STSI(15).
When using an older machine type, the facility should be disabled by
default, so the user does not have to know that ctop=off has to be set ...
so I think you should only do the s390_topology_setup() by default if using
the 7.2 machine type (or newer).
Thomas
Re: [PATCH 2/2] target/s390x: Align __excp_addr in s390_probe_access
On 23.08.22 23:38, Richard Henderson wrote:
> Per the comment in s390_cpu_record_sigsegv, the saved address
> is always page aligned.
>
> Signed-off-by: Richard Henderson
> ---
> target/s390x/tcg/mem_helper.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/s390x/tcg/mem_helper.c b/target/s390x/tcg/mem_helper.c
> index 4c0f8baa39..19ea7d2f8d 100644
> --- a/target/s390x/tcg/mem_helper.c
> +++ b/target/s390x/tcg/mem_helper.c
> @@ -147,7 +147,7 @@ static int s390_probe_access(CPUArchState *env,
> target_ulong addr, int size,
> #if defined(CONFIG_USER_ONLY)
> flags = page_get_flags(addr);
> if (!(flags & (access_type == MMU_DATA_LOAD ? PAGE_READ :
> PAGE_WRITE_ORG))) {
> -env->__excp_addr = addr;
> +env->__excp_addr = addr & TARGET_PAGE_MASK;
> flags = (flags & PAGE_VALID) ? PGM_PROTECTION : PGM_ADDRESSING;
> if (nonfault) {
> return flags;
Reviewed-by: David Hildenbrand
--
Thanks,
David / dhildenb
Re: [PATCH 1/2] Revert "target/s390x: Use probe_access_flags in s390_probe_access"
On 23.08.22 23:38, Richard Henderson wrote:
> This reverts commit db9aab5783a2fb62250e12f0c4cfed5e1778c189.
>
> This patch breaks the contract of s390_probe_access, in that
> it no longer returns an exception code, nor set __excp_addr.
>
> Reported-by: David Hildenbrand
> Signed-off-by: Richard Henderson
> ---
> target/s390x/tcg/mem_helper.c | 18 +-
> 1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/target/s390x/tcg/mem_helper.c b/target/s390x/tcg/mem_helper.c
> index fc52aa128b..4c0f8baa39 100644
> --- a/target/s390x/tcg/mem_helper.c
> +++ b/target/s390x/tcg/mem_helper.c
> @@ -142,12 +142,20 @@ static int s390_probe_access(CPUArchState *env,
> target_ulong addr, int size,
> MMUAccessType access_type, int mmu_idx,
> bool nonfault, void **phost, uintptr_t ra)
> {
> -#if defined(CONFIG_USER_ONLY)
> -return probe_access_flags(env, addr, access_type, mmu_idx,
> - nonfault, phost, ra);
> -#else
> int flags;
>
> +#if defined(CONFIG_USER_ONLY)
> +flags = page_get_flags(addr);
> +if (!(flags & (access_type == MMU_DATA_LOAD ? PAGE_READ :
> PAGE_WRITE_ORG))) {
> +env->__excp_addr = addr;
> +flags = (flags & PAGE_VALID) ? PGM_PROTECTION : PGM_ADDRESSING;
> +if (nonfault) {
> +return flags;
> +}
> +tcg_s390_program_interrupt(env, flags, ra);
> +}
> +*phost = g2h(env_cpu(env), addr);
> +#else
> /*
> * For !CONFIG_USER_ONLY, we cannot rely on TLB_INVALID_MASK or
> haddr==NULL
> * to detect if there was an exception during tlb_fill().
> @@ -166,8 +174,8 @@ static int s390_probe_access(CPUArchState *env,
> target_ulong addr, int size,
> (access_type == MMU_DATA_STORE
>? BP_MEM_WRITE : BP_MEM_READ), ra);
> }
> -return 0;
> #endif
> +return 0;
> }
>
> static int access_prepare_nf(S390Access *access, CPUS390XState *env,
Reviewed-by: David Hildenbrand
--
Thanks,
David / dhildenb
Re: [PATCH 2/5] vdpa: Add vhost_vdpa_net_load_mq
On Wed, Aug 24, 2022 at 6:23 AM Jason Wang wrote:
>
>
> 在 2022/8/20 01:13, Eugenio Pérez 写道:
> > Same way as with the MAC, restore the expected number of queues at
> > device's start.
> >
> > Signed-off-by: Eugenio Pérez
> > ---
> > net/vhost-vdpa.c | 33 +
> > 1 file changed, 33 insertions(+)
> >
> > diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> > index 1e0dbfcced..96fd3bc835 100644
> > --- a/net/vhost-vdpa.c
> > +++ b/net/vhost-vdpa.c
> > @@ -391,6 +391,35 @@ static int vhost_vdpa_net_load_mac(VhostVDPAState *s,
> > return 0;
> > }
> >
> > +static int vhost_vdpa_net_load_mq(VhostVDPAState *s,
> > + const VirtIONet *n)
> > +{
> > +uint64_t features = n->parent_obj.guest_features;
> > +ssize_t dev_written;
> > +void *cursor = s->cvq_cmd_out_buffer;
> > +if (!(features & BIT_ULL(VIRTIO_NET_F_MQ))) {
> > +return 0;
> > +}
> > +
> > +*(struct virtio_net_ctrl_hdr *)cursor = (struct virtio_net_ctrl_hdr) {
> > +.class = VIRTIO_NET_CTRL_MQ,
> > +.cmd = VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET,
> > +};
> > +cursor += sizeof(struct virtio_net_ctrl_hdr);
> > +*(struct virtio_net_ctrl_mq *)cursor = (struct virtio_net_ctrl_mq) {
> > +.virtqueue_pairs = cpu_to_le16(n->curr_queue_pairs),
> > +};
>
>
> Such casting is not elegant, let's just prepare buffer and then do the
> copy inside vhost_vdpa_net_cvq_add()?
>
I'm not sure what you propose here. I can pre-fill a buffer in the
stack and then do an extra copy in vhost_vdpa_net_cvq_add. The
compiler should be able to optimize it, but I'm not sure if it
simplifies the code.
We can have a dedicated buffer for mac, another for mq, and one for
each different command, and map all of them at the device's start. But
this seems too much overhead to me.
Some alternatives that come to my mind:
* Declare a struct with both virtio_net_ctrl_hdr and each of the
control commands (using unions?), and cast s->cvq_cmd_out_buffer
accordingly.
* Declare a struct with all of the supported commands one after
another, and let qemu fill and send these accordingly.
>
> > +cursor += sizeof(struct virtio_net_ctrl_mq);
> > +
> > +dev_written = vhost_vdpa_net_cvq_add(s, cursor - s->cvq_cmd_out_buffer,
> > + sizeof(virtio_net_ctrl_ack));
> > +if (unlikely(dev_written < 0)) {
> > +return dev_written;
> > +}
> > +
> > +return *((virtio_net_ctrl_ack *)s->cvq_cmd_in_buffer) != VIRTIO_NET_OK;
>
>
> So I think we should have a dedicated buffer just for ack, then there's
> no need for such casting.
>
You mean to declare cvq_cmd_in_buffer as virtio_net_ctrl_ack type
directly and map it to the device?
Thanks!
Re: [PATCH v2 0/4] hw/arm/virt: Improve address assignment for high memory regions
Hi Gavin,
On 8/24/22 05:29, Gavin Shan wrote:
> Hi Marc,
>
> On 8/15/22 4:29 PM, Gavin Shan wrote:
>> There are three high memory regions, which are VIRT_HIGH_REDIST2,
>> VIRT_HIGH_PCIE_ECAM and VIRT_HIGH_PCIE_MMIO. Their base addresses
>> are floating on highest RAM address. However, they can be disabled
>> in several cases.
>> (1) One specific high memory region is disabled by developer by
>> toggling vms->highmem_{redists, ecam, mmio}.
>> (2) VIRT_HIGH_PCIE_ECAM region is disabled on machine, which is
>> 'virt-2.12' or ealier than it.
>> (3) VIRT_HIGH_PCIE_ECAM region is disabled when firmware is loaded
>> on 32-bits system.
>> (4) One specific high memory region is disabled when it breaks the
>> PA space limit.
>> The current implementation of virt_set_memmap() isn't comprehensive
>> because the space for one specific high memory region is always
>> reserved from the PA space for case (1), (2) and (3). In the code,
>> 'base' and 'vms->highest_gpa' are always increased for those three
>> cases. It's unnecessary since the assigned space of the disabled
>> high memory region won't be used afterwards.
>>
>> The series intends to improve the address assignment for these
>> high memory regions:
>>
>> PATCH[1] and PATCH[2] are cleanup and preparatory works.
>> PATCH[3] improves address assignment for these high memory regions
>> PATCH[4] moves the address assignment logic into standalone helper
>>
>> History
>> ===
>> v1: https://lists.nongnu.org/archive/html/qemu-arm/2022-08/msg00013.html
>>
>> Changelog
>> =
>> v2:
>> * Split the patches for easier review (Gavin)
>> * Improved changelog (Marc)
>> * Use 'bool fits' in virt_set_high_memmap() (Eric)
>>
You did not really convince me about migration compat wrt the high MMIO
region. Aren't the PCI BARs saved/restored meaning the device driver is
expecting to find data at the same GPA. But what if your high MMIO
region was relocated in the dest QEMU with a possibly smaller VM IPA?
Don't you have MMIO regions now allocated outside of the dest MMIO
region? How does the PCI host bridge route accesses to those regions?
What do I miss?
Thanks
Eric
>
> Could you help to review when you have free cycles? It's just a kindly
> ping :)
>
> Thanks,
> Gavin
>
>>
>> Gavin Shan (4):
>> hw/arm/virt: Rename variable size to region_size in virt_set_memmap()
>> hw/arm/virt: Introduce variable region_base in virt_set_memmap()
>> hw/arm/virt: Improve address assignment for high memory regions
>> virt/hw/virt: Add virt_set_high_memmap() helper
>>
>> hw/arm/virt.c | 84 ++-
>> 1 file changed, 50 insertions(+), 34 deletions(-)
>>
>
[PATCH] tests/avocado: Fix trivial typo
The intention was likely to use "intend" instead of "indent" here. Signed-off-by: Thomas Huth --- tests/avocado/avocado_qemu/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/avocado/avocado_qemu/__init__.py b/tests/avocado/avocado_qemu/__init__.py index ed4853c805..26ebf01440 100644 --- a/tests/avocado/avocado_qemu/__init__.py +++ b/tests/avocado/avocado_qemu/__init__.py @@ -508,7 +508,7 @@ def default_kernel_params(self): class LinuxTest(LinuxSSHMixIn, QemuSystemTest): """Facilitates having a cloud-image Linux based available. -For tests that indent to interact with guests, this is a better choice +For tests that intend to interact with guests, this is a better choice to start with than the more vanilla `QemuSystemTest` class. """ -- 2.31.1
Re: [PATCH v9 1/3] hw/intc: Move mtimer/mtimecmp to aclint
On Tue, Aug 23, 2022 at 2:10 PM Alistair Francis
wrote:
> On Thu, Aug 11, 2022 at 4:57 AM Atish Patra wrote:
> >
> > Historically, The mtime/mtimecmp has been part of the CPU because
> > they are per hart entities. However, they actually belong to aclint
> > which is a MMIO device.
> >
> > Move them to the ACLINT device. This also emulates the real hardware
> > more closely.
> >
> > Reviewed-by: Anup Patel
> > Reviewed-by: Alistair Francis
> > Reviewed-by: Andrew Jones
> > Signed-off-by: Atish Patra
>
> This patch breaks my multi-socket boot.
>
> When using OpenSBI 1.1 and Linux 5.19-rc7
>
> qemu-system-riscv64 \
> -machine virt \
> -serial mon:stdio -serial null -nographic \
> -append "root=/dev/vda rw highres=off console=ttyS0 ip=dhcp
> earlycon=sbi" \
> -device virtio-net-device,netdev=net0,mac=52:54:00:12:34:02 \
> -netdev user,id=net0 \
> -object rng-random,filename=/dev/urandom,id=rng0 \
> -device virtio-rng-device,rng=rng0 \
> -smp 4 \
> -d guest_errors \
> -m 2G \
> -object
> memory-backend-ram,size=1G,policy=bind,host-nodes=0,id=ram-node0 \
> -numa node,memdev=ram-node0 \
> -object
> memory-backend-ram,size=1G,policy=bind,host-nodes=0,id=ram-node1 \
> -numa node,memdev=ram-node1 \
> -numa cpu,node-id=0,core-id=0 \
> -numa cpu,node-id=0,core-id=1 \
> -numa cpu,node-id=1,core-id=2 \
> -numa cpu,node-id=1,core-id=3 \
> -kernel ./images/qemuriscv64/Image
> -bios default
>
> It looks like OpenSBI hangs when booting after applying this patch
>
>
Argh. It was due to relative hartid vs absolute hartid per socket. This
fixes the issue for me.
Sorry for the breakage!
diff --git a/hw/intc/riscv_aclint.c b/hw/intc/riscv_aclint.c
index a125c73d535c..eee04643cb19 100644
--- a/hw/intc/riscv_aclint.c
+++ b/hw/intc/riscv_aclint.c
@@ -66,18 +66,21 @@ static void
riscv_aclint_mtimer_write_timecmp(RISCVAclintMTimerState *mtimer,
uint64_t rtc_r = cpu_riscv_read_rtc(mtimer);
+/* Compute the relative hartid w.r.t the socket */
+hartid = hartid - mtimer->hartid_base;
+
mtimer->timecmp[hartid] = value;
if (mtimer->timecmp[hartid] <= rtc_r) {
/*
* If we're setting an MTIMECMP value in the "past",
* immediately raise the timer interrupt
*/
-qemu_irq_raise(mtimer->timer_irqs[hartid - mtimer->hartid_base]);
+qemu_irq_raise(mtimer->timer_irqs[hartid]);
return;
}
/* otherwise, set up the future timer interrupt */
-qemu_irq_lower(mtimer->timer_irqs[hartid - mtimer->hartid_base]);
+qemu_irq_lower(mtimer->timer_irqs[hartid]);
diff = mtimer->timecmp[hartid] - rtc_r;
/* back to ns (note args switched in muldiv64) */
uint64_t ns_diff = muldiv64(diff, NANOSECONDS_PER_SECOND,
timebase_freq);
Alistair
>
> > ---
> > hw/intc/riscv_aclint.c | 41 --
> > hw/timer/ibex_timer.c | 18 ++-
> > include/hw/intc/riscv_aclint.h | 2 ++
> > include/hw/timer/ibex_timer.h | 2 ++
> > target/riscv/cpu.h | 2 --
> > target/riscv/machine.c | 5 ++---
> > 6 files changed, 42 insertions(+), 28 deletions(-)
> >
> > diff --git a/hw/intc/riscv_aclint.c b/hw/intc/riscv_aclint.c
> > index e7942c4e5a32..a125c73d535c 100644
> > --- a/hw/intc/riscv_aclint.c
> > +++ b/hw/intc/riscv_aclint.c
> > @@ -32,6 +32,7 @@
> > #include "hw/intc/riscv_aclint.h"
> > #include "qemu/timer.h"
> > #include "hw/irq.h"
> > +#include "migration/vmstate.h"
> >
> > typedef struct riscv_aclint_mtimer_callback {
> > RISCVAclintMTimerState *s;
> > @@ -65,8 +66,8 @@ static void
> riscv_aclint_mtimer_write_timecmp(RISCVAclintMTimerState *mtimer,
> >
> > uint64_t rtc_r = cpu_riscv_read_rtc(mtimer);
> >
> > -cpu->env.timecmp = value;
> > -if (cpu->env.timecmp <= rtc_r) {
> > +mtimer->timecmp[hartid] = value;
> > +if (mtimer->timecmp[hartid] <= rtc_r) {
> > /*
> > * If we're setting an MTIMECMP value in the "past",
> > * immediately raise the timer interrupt
> > @@ -77,7 +78,7 @@ static void
> riscv_aclint_mtimer_write_timecmp(RISCVAclintMTimerState *mtimer,
> >
> > /* otherwise, set up the future timer interrupt */
> > qemu_irq_lower(mtimer->timer_irqs[hartid - mtimer->hartid_base]);
> > -diff = cpu->env.timecmp - rtc_r;
> > +diff = mtimer->timecmp[hartid] - rtc_r;
> > /* back to ns (note args switched in muldiv64) */
> > uint64_t ns_diff = muldiv64(diff, NANOSECONDS_PER_SECOND,
> timebase_freq);
> >
> > @@ -102,7 +103,7 @@ static void
> riscv_aclint_mtimer_write_timecmp(RISCVAclintMTimerState *mtimer,
> > next = MIN(next, INT64_MAX);
> > }
> >
> > -timer_mod(cpu->env.timer, next);
> > +timer_mod(mtimer->timers[hartid], next);
> > }
> >
> > /*
> > @@ -133,11 +134,11 @@ static uint64_t riscv_aclint_mtimer_read(void
> *opaque, hwaddr addr,
> >"aclint-mtimer: inva
Re: [PATCH v5 1/2] Update AVX512 support for xbzrle_encode_buffer
ling xu wrote:
> This commit updates code of avx512 support for xbzrle_encode_buffer function
> to
> accelerate xbzrle encoding speed. We add runtime check of avx512 and add
> benchmark for this feature. Compared with C version of
> xbzrle_encode_buffer function, avx512 version can achieve 50%-70%
> performance improvement on benchmarking. In addition, if dirty data is
> randomly located in 4K page, the avx512 version can achieve almost 140%
> performance gain.
>
> Signed-off-by: ling xu
> Co-authored-by: Zhou Zhao
> Co-authored-by: Jun Jin
> ---
> meson.build| 16 ++
> meson_options.txt | 2 +
> migration/ram.c| 35 ++--
> migration/xbzrle.c | 130 +
> migration/xbzrle.h | 4 ++
> 5 files changed, 184 insertions(+), 3 deletions(-)
>
> diff --git a/meson.build b/meson.build
> index 30a380752c..c9d90a5bff 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -2264,6 +2264,22 @@ config_host_data.set('CONFIG_AVX512F_OPT',
> get_option('avx512f') \
> int main(int argc, char *argv[]) { return bar(argv[0]); }
>'''), error_message: 'AVX512F not available').allowed())
>
> +config_host_data.set('CONFIG_AVX512BW_OPT', get_option('avx512bw') \
> + .require(have_cpuid_h, error_message: 'cpuid.h not available, cannot
> enable AVX512BW') \
> + .require(cc.links('''
> +#pragma GCC push_options
> +#pragma GCC target("avx512bw")
> +#include
> +#include
> +static int bar(void *a) {
> + __m512i x = *(__m512i *)a;
> + __m512i res= _mm512_abs_epi8(x);
Cast is as ugly as hell, what about:
__m512i *x = a;
__m512i res = _mm512_abs_epi8(*x);
??
> +static void __attribute__((constructor)) init_cpu_flag(void)
> +{
> +unsigned max = __get_cpuid_max(0, NULL);
> +int a, b, c, d;
> +if (max >= 1) {
> +__cpuid(1, a, b, c, d);
> + /* We must check that AVX is not just available, but usable. */
> +if ((c & bit_OSXSAVE) && (c & bit_AVX) && max >= 7) {
> +int bv;
> +__asm("xgetbv" : "=a"(bv), "=d"(d) : "c"(0));
> +__cpuid_count(7, 0, a, b, c, d);
> + /* 0xe6:
> +* XCR0[7:5] = 111b (OPMASK state, upper 256-bit of ZMM0-ZMM15
> +*and ZMM16-ZMM31 state are enabled by OS)
> +* XCR0[2:1] = 11b (XMM state and YMM state are enabled by OS)
> +*/
> +if ((bv & 0xe6) == 0xe6 && (b & bit_AVX512BW)) {
> +xbzrle_encode_buffer_func = xbzrle_encode_buffer_avx512;
> +}
> +}
> +}
> +return ;
This return line is not needed.
> +}
> +#endif
> +
> XBZRLECacheStats xbzrle_counters;
>
> /* struct contains XBZRLE cache and a static page
> @@ -802,9 +831,9 @@ static int save_xbzrle_page(RAMState *rs, uint8_t
> **current_data,
> memcpy(XBZRLE.current_buf, *current_data, TARGET_PAGE_SIZE);
>
> /* XBZRLE encoding (if there is no overflow) */
> -encoded_len = xbzrle_encode_buffer(prev_cached_page, XBZRLE.current_buf,
> - TARGET_PAGE_SIZE, XBZRLE.encoded_buf,
> - TARGET_PAGE_SIZE);
> +encoded_len = xbzrle_encode_buffer_func(prev_cached_page,
> XBZRLE.current_buf,
> +TARGET_PAGE_SIZE,
> XBZRLE.encoded_buf,
> +TARGET_PAGE_SIZE);
>
> /*
> * Update the cache contents, so that it corresponds to the data
> diff --git a/migration/xbzrle.c b/migration/xbzrle.c
> index 1ba482ded9..6da7f79625 100644
> --- a/migration/xbzrle.c
> +++ b/migration/xbzrle.c
> @@ -174,3 +174,133 @@ int xbzrle_decode_buffer(uint8_t *src, int slen,
> uint8_t *dst, int dlen)
>
> return d;
> }
> +
> +#if defined(CONFIG_AVX512BW_OPT)
> +#pragma GCC push_options
> +#pragma GCC target("avx512bw")
> +#include
> +int xbzrle_encode_buffer_avx512(uint8_t *old_buf, uint8_t *new_buf, int slen,
> + uint8_t *dst, int dlen)
> +{
> +uint32_t zrun_len = 0, nzrun_len = 0;
> +int d = 0, i = 0, num = 0;
> +uint8_t *nzrun_start = NULL;
> +/* add 1 to include residual part in main loop */
> +uint32_t count512s = (slen >> 6) + 1;
> +/* countResidual is tail of data, i.e., countResidual = slen % 64 */
> +uint32_t countResidual = slen & 0b11;
> +bool never_same = true;
> +uint64_t maskResidual = 1;
> +maskResidual <<= countResidual;
> +maskResidual -=1;
> +uint64_t comp = 0;
> +int bytesToCheck = 0;
> +
> +while (count512s) {
> +if (d + 2 > dlen) {
> +return -1;
> +}
> +
> +if(count512s != 1){
> +__m512i old_data = _mm512_mask_loadu_epi8(old_data,
> + 0x,
> old_buf + i);
> +__m512i new_data = _mm512_mask_loadu_epi8(new_data,
> +
Re: [PATCH v8 02/12] s390x/cpu_topology: CPU topology objects and structures
On 8/24/22 09:30, Thomas Huth wrote:
On 23/08/2022 19.41, Pierre Morel wrote:
On 8/23/22 15:30, Thomas Huth wrote:
On 20/06/2022 16.03, Pierre Morel wrote:
We use new objects to have a dynamic administration of the CPU
topology.
The highest level object in this implementation is the s390 book and
in this first implementation of CPU topology for S390 we have a single
book.
The book is built as a SYSBUS bridge during the CPU initialization.
Other objects, sockets and core will be built after the parsing
of the QEMU -smp argument.
Every object under this single book will be build dynamically
immediately after a CPU has be realized if it is needed.
The CPU will fill the sockets once after the other, according to the
number of core per socket defined during the smp parsing.
Each CPU inside a socket will be represented by a bit in a 64bit
unsigned long. Set on plug and clear on unplug of a CPU.
For the S390 CPU topology, thread and cores are merged into
topology cores and the number of topology cores is the multiplication
of cores by the numbers of threads.
Signed-off-by: Pierre Morel
---
hw/s390x/cpu-topology.c | 391
hw/s390x/meson.build | 1 +
hw/s390x/s390-virtio-ccw.c | 6 +
include/hw/s390x/cpu-topology.h | 74 ++
target/s390x/cpu.h | 47
5 files changed, 519 insertions(+)
create mode 100644 hw/s390x/cpu-topology.c
create mode 100644 include/hw/s390x/cpu-topology.h
...
+bool s390_topology_new_cpu(MachineState *ms, int core_id, Error
**errp)
+{
+ S390TopologyBook *book;
+ S390TopologySocket *socket;
+ S390TopologyCores *cores;
+ int nb_cores_per_socket;
+ int origin, bit;
+
+ book = s390_get_topology();
+
+ nb_cores_per_socket = ms->smp.cores * ms->smp.threads;
+
+ socket = s390_get_socket(ms, book, core_id /
nb_cores_per_socket, errp);
+ if (!socket) {
+ return false;
+ }
+
+ /*
+ * At the core level, each CPU is represented by a bit in a 64bit
+ * unsigned long. Set on plug and clear on unplug of a CPU.
+ * The firmware assume that all CPU in the core description
have the same
+ * type, polarization and are all dedicated or shared.
+ * In the case a socket contains CPU with different type,
polarization
+ * or dedication then they will be defined in different CPU
containers.
+ * Currently we assume all CPU are identical and the only
reason to have
+ * several S390TopologyCores inside a socket is to have more
than 64 CPUs
+ * in that case the origin field, representing the offset of
the first CPU
+ * in the CPU container allows to represent up to the maximal
number of
+ * CPU inside several CPU containers inside the socket container.
+ */
+ origin = 64 * (core_id / 64);
Maybe faster:
origin = core_id & ~63;
By the way, where is this limitation to 64 coming from? Just because
we're using a "unsigned long" for now? Or is this a limitation from
the architecture?
+ cores = s390_get_cores(ms, socket, origin, errp);
+ if (!cores) {
+ return false;
+ }
+
+ bit = 63 - (core_id - origin);
+ set_bit(bit, &cores->mask);
+ cores->origin = origin;
+
+ return true;
+}
...
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index cc3097bfee..a586875b24 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -43,6 +43,7 @@
#include "sysemu/sysemu.h"
#include "hw/s390x/pv.h"
#include "migration/blocker.h"
+#include "hw/s390x/cpu-topology.h"
static Error *pv_mig_blocker;
@@ -89,6 +90,7 @@ static void s390_init_cpus(MachineState *machine)
/* initialize possible_cpus */
mc->possible_cpu_arch_ids(machine);
+ s390_topology_setup(machine);
Is this safe with regards to migration? Did you tried a ping-pong
migration from an older QEMU to a QEMU with your modifications and
back to the older one? If it does not work, we might need to wire
this setup to the machine types...
I checked with the follow-up series :
OLD-> NEW -> OLD -> NEW
It is working fine, of course we need to fence the CPU topology
facility with ctop=off on the new QEMU to avoid authorizing the new
instructions, PTF and STSI(15).
When using an older machine type, the facility should be disabled by
default, so the user does not have to know that ctop=off has to be set
... so I think you should only do the s390_topology_setup() by default
if using the 7.2 machine type (or newer).
Thomas
Oh OK, thanks.
I add this for the next series of course.
Regards,
Pierre
--
Pierre Morel
IBM Lab Boeblingen
Re: [PATCH v5 1/9] parallels: Out of image offset in BAT leads to image inflation
On 8/23/22 13:11, Denis V. Lunev wrote:
On 23.08.2022 11:58, Vladimir Sementsov-Ogievskiy wrote:
On 8/23/22 12:20, Denis V. Lunev wrote:
On 23.08.2022 09:23, Alexander Ivanov wrote:
On 23.08.2022 08:58, Vladimir Sementsov-Ogievskiy wrote:
On 8/22/22 12:05, Alexander Ivanov wrote:
data_end field in BDRVParallelsState is set to the biggest offset present
in BAT. If this offset is outside of the image, any further write will create
the cluster at this offset and/or the image will be truncated to this
offset on close. This is definitely not correct.
Raise an error in parallels_open() if data_end points outside the image and
it is not a check (let the check to repaire the image).
Signed-off-by: Alexander Ivanov
---
block/parallels.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/block/parallels.c b/block/parallels.c
index a229c06f25..c245ca35cd 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -732,6 +732,7 @@ static int parallels_open(BlockDriverState *bs, QDict
*options, int flags,
BDRVParallelsState *s = bs->opaque;
ParallelsHeader ph;
int ret, size, i;
+ int64_t file_size;
QemuOpts *opts = NULL;
Error *local_err = NULL;
char *buf;
@@ -811,6 +812,19 @@ static int parallels_open(BlockDriverState *bs, QDict
*options, int flags,
}
}
+ file_size = bdrv_getlength(bs->file->bs);
+ if (file_size < 0) {
+ ret = file_size;
+ goto fail;
+ }
+
+ file_size >>= BDRV_SECTOR_BITS;
+ if (s->data_end > file_size && !(flags & BDRV_O_CHECK)) {
+ error_setg(errp, "parallels: Offset in BAT is out of image");
+ ret = -EINVAL;
+ goto fail;
+ }
If image is unaligned to sector size, and image size is less than s->data_end,
but the difference itself is less than sector, the error message would be
misleading.
Should we consider "file_size = DIV_ROUND_UP(file_size, BDRV_SECTOR_SIZE)" instead of
"file_size >>= BDRV_SECTOR_BITS"?
It's hardly possible to get such image on valid scenarios with Qemu (keeping in
mind bdrv_truncate() call in parallels_close()). But it still may be possible
to have such images produced by another software or by some failure path.
I think you are right, it would be better to align image size up to sector size.
I would say that we need to align not on sector size but on cluster size.
That would worth additional check.
And not simply align, as data_offset is not necessarily aligned to cluster size.
Finally, what should we check?
I suggest
diff --git a/block/parallels.c b/block/parallels.c
index 6d4ed77f16..b882ea1200 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -725,6 +725,7 @@ static int parallels_open(BlockDriverState *bs, QDict
*options, int flags,
BDRVParallelsState *s = bs->opaque;
ParallelsHeader ph;
int ret, size, i;
+ int64_t file_size;
QemuOpts *opts = NULL;
Error *local_err = NULL;
char *buf;
@@ -735,6 +736,11 @@ static int parallels_open(BlockDriverState *bs, QDict
*options, int flags,
return -EINVAL;
}
+ file_size = bdrv_getlength(bs->file->bs);
+ if (file_size < 0) {
+ return file_size;
+ }
+
ret = bdrv_pread(bs->file, 0, &ph, sizeof(ph));
if (ret < 0) {
goto fail;
@@ -798,6 +804,13 @@ static int parallels_open(BlockDriverState *bs, QDict
*options, int flags,
for (i = 0; i < s->bat_size; i++) {
int64_t off = bat2sect(s, i);
+ if (off >= file_size) {
Like this, especially >= check which we have had missed.
Though this would break the repair. We need additional
if (flags & BDRV_O_CHECK) {
continue;
}
No incorrect data_end assignment, which would be
very welcome.
Den
'continue' here will change the logic around data_end. We'll drop "wrong"
clusters from calculation of data_end, and should check, how it affects further logic.
What about:
for (i = 0; i < s->bat_size; i++) {
int64_t off = bat2sect(s, i);
if (off >= file_size && !(flags & BDRV_O_CHECK)) {
error_setg(errp, "parallels: Offset %" PRIi64 " in BAT[%d] entry "
"is larger than file size (%" PRIi64 ")",
off, i, file_size);
ret = -EINVAL;
goto fail;
}
if (off >= s->data_end) {
s->data_end = off + s->tracks;
}
}
- this we simply add new error-out on no-O_CHECK path.
+ error_setg(errp, "parallels: Offset %" PRIi64 " in BAT[%d] entry "
+ "is larger than file size (%" PRIi64 ")",
+ off, i, file_size);
+ ret = -EINVAL;
+ goto fail;
+ }
if (off >= s->data_end) {
s->data_end = off + s->tracks;
}
- better error message, and we check exactly what's written in the spec
(docs/interop/parallels.c):
Cluster offsets specified by BAT entries must meet the following requirements:
[...]
- the value
[PATCH v3 1/3] util/main-loop: Fix maximum number of wait objects for win32
From: Bin Meng
The maximum number of wait objects for win32 should be
MAXIMUM_WAIT_OBJECTS, not MAXIMUM_WAIT_OBJECTS + 1.
Signed-off-by: Bin Meng
---
Changes in v3:
- move the check of adding the same HANDLE twice to a separete patch
Changes in v2:
- fix the logic in qemu_add_wait_object() to avoid adding
the same HANDLE twice
util/main-loop.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/util/main-loop.c b/util/main-loop.c
index f00a25451b..cb018dc33c 100644
--- a/util/main-loop.c
+++ b/util/main-loop.c
@@ -363,10 +363,10 @@ void qemu_del_polling_cb(PollingFunc *func, void *opaque)
/* Wait objects support */
typedef struct WaitObjects {
int num;
-int revents[MAXIMUM_WAIT_OBJECTS + 1];
-HANDLE events[MAXIMUM_WAIT_OBJECTS + 1];
-WaitObjectFunc *func[MAXIMUM_WAIT_OBJECTS + 1];
-void *opaque[MAXIMUM_WAIT_OBJECTS + 1];
+int revents[MAXIMUM_WAIT_OBJECTS];
+HANDLE events[MAXIMUM_WAIT_OBJECTS];
+WaitObjectFunc *func[MAXIMUM_WAIT_OBJECTS];
+void *opaque[MAXIMUM_WAIT_OBJECTS];
} WaitObjects;
static WaitObjects wait_objects = {0};
@@ -395,6 +395,9 @@ void qemu_del_wait_object(HANDLE handle, WaitObjectFunc
*func, void *opaque)
if (w->events[i] == handle) {
found = 1;
}
+if (i == MAXIMUM_WAIT_OBJECTS - 1) {
+break;
+}
if (found) {
w->events[i] = w->events[i + 1];
w->func[i] = w->func[i + 1];
--
2.34.1
[PATCH v3 2/3] util/main-loop: Avoid adding the same HANDLE twice
From: Bin Meng
Fix the logic in qemu_add_wait_object() to avoid adding the same
HANDLE twice, as the behavior is undefined when passing an array
that contains same HANDLEs to WaitForMultipleObjects() API.
Signed-off-by: Bin Meng
---
Changes in v3:
- new patch: avoid adding the same HANDLE twice
include/qemu/main-loop.h | 2 ++
util/main-loop.c | 10 ++
2 files changed, 12 insertions(+)
diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
index c50d1b7e3a..db8d380550 100644
--- a/include/qemu/main-loop.h
+++ b/include/qemu/main-loop.h
@@ -157,6 +157,8 @@ typedef void WaitObjectFunc(void *opaque);
* in the main loop's calls to WaitForMultipleObjects. When the handle
* is in a signaled state, QEMU will call @func.
*
+ * If the same HANDLE is added twice, this function returns -1.
+ *
* @handle: The Windows handle to be observed.
* @func: A function to be called when @handle is in a signaled state.
* @opaque: A pointer-size value that is passed to @func.
diff --git a/util/main-loop.c b/util/main-loop.c
index cb018dc33c..dae33a8daf 100644
--- a/util/main-loop.c
+++ b/util/main-loop.c
@@ -373,10 +373,20 @@ static WaitObjects wait_objects = {0};
int qemu_add_wait_object(HANDLE handle, WaitObjectFunc *func, void *opaque)
{
+int i;
WaitObjects *w = &wait_objects;
+
if (w->num >= MAXIMUM_WAIT_OBJECTS) {
return -1;
}
+
+for (i = 0; i < w->num; i++) {
+/* check if the same handle is added twice */
+if (w->events[i] == handle) {
+return -1;
+}
+}
+
w->events[w->num] = handle;
w->func[w->num] = func;
w->opaque[w->num] = opaque;
--
2.34.1
Re: [PATCH 2/5] vdpa: Add vhost_vdpa_net_load_mq
On Wed, Aug 24, 2022 at 3:47 PM Eugenio Perez Martin
wrote:
>
> On Wed, Aug 24, 2022 at 6:23 AM Jason Wang wrote:
> >
> >
> > 在 2022/8/20 01:13, Eugenio Pérez 写道:
> > > Same way as with the MAC, restore the expected number of queues at
> > > device's start.
> > >
> > > Signed-off-by: Eugenio Pérez
> > > ---
> > > net/vhost-vdpa.c | 33 +
> > > 1 file changed, 33 insertions(+)
> > >
> > > diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> > > index 1e0dbfcced..96fd3bc835 100644
> > > --- a/net/vhost-vdpa.c
> > > +++ b/net/vhost-vdpa.c
> > > @@ -391,6 +391,35 @@ static int vhost_vdpa_net_load_mac(VhostVDPAState *s,
> > > return 0;
> > > }
> > >
> > > +static int vhost_vdpa_net_load_mq(VhostVDPAState *s,
> > > + const VirtIONet *n)
> > > +{
> > > +uint64_t features = n->parent_obj.guest_features;
> > > +ssize_t dev_written;
> > > +void *cursor = s->cvq_cmd_out_buffer;
> > > +if (!(features & BIT_ULL(VIRTIO_NET_F_MQ))) {
> > > +return 0;
> > > +}
> > > +
> > > +*(struct virtio_net_ctrl_hdr *)cursor = (struct virtio_net_ctrl_hdr)
> > > {
> > > +.class = VIRTIO_NET_CTRL_MQ,
> > > +.cmd = VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET,
> > > +};
> > > +cursor += sizeof(struct virtio_net_ctrl_hdr);
> > > +*(struct virtio_net_ctrl_mq *)cursor = (struct virtio_net_ctrl_mq) {
> > > +.virtqueue_pairs = cpu_to_le16(n->curr_queue_pairs),
> > > +};
> >
> >
> > Such casting is not elegant, let's just prepare buffer and then do the
> > copy inside vhost_vdpa_net_cvq_add()?
> >
>
> I'm not sure what you propose here. I can pre-fill a buffer in the
> stack and then do an extra copy in vhost_vdpa_net_cvq_add. The
> compiler should be able to optimize it, but I'm not sure if it
> simplifies the code.
>
> We can have a dedicated buffer for mac, another for mq, and one for
> each different command, and map all of them at the device's start. But
> this seems too much overhead to me.
Considering we may need to support and restore a lot of other fields,
this looks a little complicated.
I meant the caller can simply do:
struct virtio_net_ctrl_mq mq = { ...};
Then we do
vhost_vdpa_net_cvq_add(&mq, sizeof(mq), ...);
Then we can do memcpy inside vhost_vdpa_net_cvq_add() and hide the
cmd_out_buffer etc from the caller.
>
> Some alternatives that come to my mind:
>
> * Declare a struct with both virtio_net_ctrl_hdr and each of the
> control commands (using unions?), and cast s->cvq_cmd_out_buffer
> accordingly.
> * Declare a struct with all of the supported commands one after
> another, and let qemu fill and send these accordingly.
>
> >
> > > +cursor += sizeof(struct virtio_net_ctrl_mq);
> > > +
> > > +dev_written = vhost_vdpa_net_cvq_add(s, cursor -
> > > s->cvq_cmd_out_buffer,
> > > +
> > > sizeof(virtio_net_ctrl_ack));
> > > +if (unlikely(dev_written < 0)) {
> > > +return dev_written;
> > > +}
> > > +
> > > +return *((virtio_net_ctrl_ack *)s->cvq_cmd_in_buffer) !=
> > > VIRTIO_NET_OK;
> >
> >
> > So I think we should have a dedicated buffer just for ack, then there's
> > no need for such casting.
> >
>
> You mean to declare cvq_cmd_in_buffer as virtio_net_ctrl_ack type
> directly and map it to the device?
Kind of, considering the ack is the only kind of structure in the near
future, can we simply use the structure virtio_net_ctl_ack?
Thanks
>
> Thanks!
>
Re: [PATCH v2 11/24] vhost-net: vhost-kernel: introduce vhost_net_virtqueue_stop()
在 2022/8/24 11:33, Kangjie Xu 写道:
在 2022/8/24 10:40, Jason Wang 写道:
在 2022/8/16 09:06, Kangjie Xu 写道:
Introduce vhost_virtqueue_stop(), which can reset the virtqueue
in the device. Then it will unmap vrings and the desc of the
virtqueue.
This patch only considers the case for vhost-kernel, when
NetClientDriver is NET_CLIENT_DRIVER_TAP.
Signed-off-by: Kangjie Xu
Signed-off-by: Xuan Zhuo
---
hw/net/vhost_net.c | 21 +
include/net/vhost_net.h | 2 ++
2 files changed, 23 insertions(+)
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index ccac5b7a64..aa60dd901c 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -514,3 +514,24 @@ int vhost_net_set_mtu(struct vhost_net *net,
uint16_t mtu)
return vhost_ops->vhost_net_set_mtu(&net->dev, mtu);
}
+
+void vhost_net_virtqueue_stop(VirtIODevice *vdev, NetClientState *nc,
+ int vq_index)
+{
+ VHostNetState *net = get_vhost_net(nc->peer);
+ const VhostOps *vhost_ops = net->dev.vhost_ops;
+ struct vhost_vring_file file = { .fd = -1 };
+ int idx;
+
+ assert(vhost_ops);
+
+ idx = vhost_ops->vhost_get_vq_index(&net->dev, vq_index);
+
+ if (net->nc->info->type == NET_CLIENT_DRIVER_TAP) {
+ file.index = idx;
+ int r = vhost_net_set_backend(&net->dev, &file);
+ assert(r >= 0);
+ }
Let's have a vhost_ops here instead of open code it.
Thanks
I double-checked it, vhost_net_set_backend is already a wrapper of
vhost_ops->vhost_net_set_backend().
It seems that, to further simplify it, we can only add idx and fd to
the parameter list of vhost_net_set_backend().
Ok, so we can leave it as is. (Probably need a vhost_net_ops in the future).
Thanks
Thanks
+
+ vhost_dev_virtqueue_stop(&net->dev, vdev, idx);
+}
diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h
index 387e913e4e..9b3aaf3814 100644
--- a/include/net/vhost_net.h
+++ b/include/net/vhost_net.h
@@ -48,4 +48,6 @@ uint64_t
vhost_net_get_acked_features(VHostNetState *net);
int vhost_net_set_mtu(struct vhost_net *net, uint16_t mtu);
+void vhost_net_virtqueue_stop(VirtIODevice *vdev, NetClientState
*nc,
+ int vq_index);
#endif
[PATCH v3 3/3] util/aio-win32: Correct the event array size in aio_poll()
From: Bin Meng
WaitForMultipleObjects() can only wait for MAXIMUM_WAIT_OBJECTS
object handles. Correct the event array size in aio_poll() and
add a assert() to ensure it does not cause out of bound access.
Signed-off-by: Bin Meng
Reviewed-by: Stefan Weil
Reviewed-by: Marc-André Lureau
---
(no changes since v2)
Changes in v2:
- change 'count' to unsigned
util/aio-win32.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/util/aio-win32.c b/util/aio-win32.c
index 44003d645e..80cfe012ad 100644
--- a/util/aio-win32.c
+++ b/util/aio-win32.c
@@ -326,9 +326,9 @@ void aio_dispatch(AioContext *ctx)
bool aio_poll(AioContext *ctx, bool blocking)
{
AioHandler *node;
-HANDLE events[MAXIMUM_WAIT_OBJECTS + 1];
+HANDLE events[MAXIMUM_WAIT_OBJECTS];
bool progress, have_select_revents, first;
-int count;
+unsigned count;
int timeout;
/*
@@ -369,6 +369,7 @@ bool aio_poll(AioContext *ctx, bool blocking)
QLIST_FOREACH_RCU(node, &ctx->aio_handlers, node) {
if (!node->deleted && node->io_notify
&& aio_node_check(ctx, node->is_external)) {
+assert(count < MAXIMUM_WAIT_OBJECTS);
events[count++] = event_notifier_get_handle(node->e);
}
}
--
2.34.1
Re: [PATCH v2 07/24] virtio-pci: support queue enable
在 2022/8/23 16:20, Kangjie Xu 写道:
在 2022/8/23 15:44, Jason Wang 写道:
在 2022/8/16 09:06, Kangjie Xu 写道:
PCI devices support vq enable.
Nit: it might be "support device specific vq enable"
Get it.
Based on this function, the driver can re-enable the virtqueue after
the
virtqueue is reset.
Signed-off-by: Kangjie Xu
Signed-off-by: Xuan Zhuo
---
hw/virtio/virtio-pci.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index ec8e92052f..3d560e45ad 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1335,6 +1335,7 @@ static void virtio_pci_common_write(void
*opaque, hwaddr addr,
proxy->vqs[vdev->queue_sel].avail[0],
((uint64_t)proxy->vqs[vdev->queue_sel].used[1]) << 32 |
proxy->vqs[vdev->queue_sel].used[0]);
+ virtio_queue_enable(vdev, vdev->queue_sel);
proxy->vqs[vdev->queue_sel].enabled = 1;
proxy->vqs[vdev->queue_sel].reset = 0;
Any reason we do it before the assignment of 1? It probably means the
device specific method can't depend on virtio_queue_enabled()?
Thanks
Sorry, I don't get why device specific method can't depend on
virtio_queue_enabled().
I meant if the device specific method call virtio_queue_enabled() it
will return false in this case, is this intended?
Before virtio_queue_enable() is done, virtqueue should always be not
ready and disabled.
Otherwise, If we put it after the assignment of enabled to 1, the
virtqueue may be accessed illegally and may cause panic, because the
virtqueue is still being intialized and being configured.
How? Shouldn't we make transport ready before making device
virtqueue(device) ready?
Thanks
Thanks
} else {
Re: [PATCH v2 06/24] virtio-pci: support queue reset
在 2022/8/23 15:52, Kangjie Xu 写道:
在 2022/8/23 15:40, Jason Wang 写道:
在 2022/8/16 09:06, Kangjie Xu 写道:
From: Xuan Zhuo
PCI devices support vq reset.
Based on this function, the driver can adjust the size of the ring, and
quickly recycle the buffer in the ring.
The migration of the virtio devices will not happen during a reset
operation. This is becuase the global iothread lock is held. Migration
thread also needs the lock. As a result, we do not need to migrate the
reset state of VirtIOPCIQueue.
Signed-off-by: Xuan Zhuo
Signed-off-by: Kangjie Xu
---
hw/virtio/virtio-pci.c | 19 +++
include/hw/virtio/virtio-pci.h | 1 +
2 files changed, 20 insertions(+)
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 45327f0b31..ec8e92052f 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1246,6 +1246,9 @@ static uint64_t virtio_pci_common_read(void
*opaque, hwaddr addr,
case VIRTIO_PCI_COMMON_Q_USEDHI:
val = proxy->vqs[vdev->queue_sel].used[1];
break;
+ case VIRTIO_PCI_COMMON_Q_RESET:
+ val = proxy->vqs[vdev->queue_sel].reset;
+ break;
default:
val = 0;
}
@@ -1333,6 +1336,7 @@ static void virtio_pci_common_write(void
*opaque, hwaddr addr,
((uint64_t)proxy->vqs[vdev->queue_sel].used[1]) << 32 |
proxy->vqs[vdev->queue_sel].used[0]);
proxy->vqs[vdev->queue_sel].enabled = 1;
+ proxy->vqs[vdev->queue_sel].reset = 0;
} else {
virtio_error(vdev, "wrong value for queue_enable
%"PRIx64, val);
}
@@ -1355,6 +1359,20 @@ static void virtio_pci_common_write(void
*opaque, hwaddr addr,
case VIRTIO_PCI_COMMON_Q_USEDHI:
proxy->vqs[vdev->queue_sel].used[1] = val;
break;
+ case VIRTIO_PCI_COMMON_Q_RESET:
+ if (val == 1) {
+ /*
+ * With the global iothread lock taken, the migration
will not
+ * happen until the virtqueue reset is done.
+ */
This comment applies to all other common cfg operation as well, So it
looks not necessary?
Get it.
+ proxy->vqs[vdev->queue_sel].reset = 1;
+
+ virtio_queue_reset(vdev, vdev->queue_sel);
+
+ proxy->vqs[vdev->queue_sel].reset = 0;
+ proxy->vqs[vdev->queue_sel].enabled = 0;
+ }
+ break;
default:
break;
}
@@ -1950,6 +1968,7 @@ static void virtio_pci_reset(DeviceState *qdev)
for (i = 0; i < VIRTIO_QUEUE_MAX; i++) {
proxy->vqs[i].enabled = 0;
+ proxy->vqs[i].reset = 0;
proxy->vqs[i].num = 0;
proxy->vqs[i].desc[0] = proxy->vqs[i].desc[1] = 0;
proxy->vqs[i].avail[0] = proxy->vqs[i].avail[1] = 0;
diff --git a/include/hw/virtio/virtio-pci.h
b/include/hw/virtio/virtio-pci.h
index 2446dcd9ae..e9290e2b94 100644
--- a/include/hw/virtio/virtio-pci.h
+++ b/include/hw/virtio/virtio-pci.h
@@ -117,6 +117,7 @@ typedef struct VirtIOPCIRegion {
typedef struct VirtIOPCIQueue {
uint16_t num;
bool enabled;
+ bool reset;
Do we need to migrate this?
Thanks
I think we do not need to migrate this because we hold the global
iothread lock when virtqueue reset is triggered. The migration of
these device states also needs this lock.
On the other hand, the 'reset' state of virtqueue is same(is 0) before
and after the process of resetting a virtqueue.
Thus, the migration will not happen when we are resetting a virtqueue
and we do not to migrate it.
Ok, let's add a comment above reset to explain this.
Thanks
Thanks
uint32_t desc[2];
uint32_t avail[2];
uint32_t used[2];
Re: [PATCH v2 18/24] vhost-net: vhost-user: update vhost_net_virtqueue_stop()
在 2022/8/24 12:57, Kangjie Xu 写道:
在 2022/8/24 12:05, Jason Wang 写道:
在 2022/8/16 09:06, Kangjie Xu 写道:
Update vhost_net_virtqueue_stop() for vhost-user scenario.
Let's explain why it is needed now or why it doesn't cause any issue
or it's a bug fix or not.
Thanks
This patch is to suppport vq reset for vhost-user.
We need this simply because the behavior of vhost_ops->get_vq_index()
is different in vhost-user and vhost-kernel.
vhost_user_get_vq_index(dev, idx) simply returns "idx".
vhost_kernel_get_vq_index(dev, idx) returns "idx - dev->vq_index".
Thanks
Let's add them in the change-log in the next version.
But the question still, is this a bug fix (requires a Fixes tag)? If not
why do we need this now?
Thanks
Signed-off-by: Kangjie Xu
Signed-off-by: Xuan Zhuo
---
hw/net/vhost_net.c | 4
1 file changed, 4 insertions(+)
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 2ab67e875e..c0d408f3b4 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -533,6 +533,10 @@ void vhost_net_virtqueue_stop(VirtIODevice
*vdev, NetClientState *nc,
assert(r >= 0);
}
+ if (net->nc->info->type == NET_CLIENT_DRIVER_VHOST_USER) {
+ idx = idx - net->dev.vq_index;
+ }
+
vhost_dev_virtqueue_stop(&net->dev, vdev, idx);
}
Re: [PATCH v2 12/24] vhost-net: vhost-kernel: introduce vhost_net_virtqueue_restart()
在 2022/8/24 10:53, Kangjie Xu 写道:
在 2022/8/24 10:44, Jason Wang 写道:
在 2022/8/16 09:06, Kangjie Xu 写道:
Introduce vhost_net_virtqueue_restart(), which can restart the
virtqueue when the vhost net started running before. If it fails
to restart the virtqueue, the device will be stopped.
This patch only considers the case for vhost-kernel, when
NetClientDriver is NET_CLIENT_DRIVER_TAP.
Signed-off-by: Kangjie Xu
Signed-off-by: Xuan Zhuo
I would explain why current
vhost_net_start_one()/vhost_net_stop_one() can't work. Is it because
it works at queue pair level? If yes can we restructure the code and
try to reuse ?
Thanks
Because vhost_net_start_one()/vhost_net_stop_one() works at device level.
The queue pair level start/stop are vhost_virtqueue_start() and
vhost_virtqueue_stop().
What we can reuse is the vhost_virtqueue_start().
vhost_virtqueue_stop() cannot be reused because it will destroy device.
Let's add this in the changelog or a comment in the code.
Thanks
I think we do not need to restructure because we've already had an
abstraction vhost_virtqueue_start().
Thanks.
---
hw/net/vhost_net.c | 48
+
include/net/vhost_net.h | 2 ++
2 files changed, 50 insertions(+)
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index aa60dd901c..2ab67e875e 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -535,3 +535,51 @@ void vhost_net_virtqueue_stop(VirtIODevice
*vdev, NetClientState *nc,
vhost_dev_virtqueue_stop(&net->dev, vdev, idx);
}
+
+int vhost_net_virtqueue_restart(VirtIODevice *vdev, NetClientState
*nc,
+ int vq_index)
+{
+ VHostNetState *net = get_vhost_net(nc->peer);
+ const VhostOps *vhost_ops = net->dev.vhost_ops;
+ struct vhost_vring_file file = { };
+ int idx, r;
+
+ if (!net->dev.started) {
+ return 0;
+ }
+
+ assert(vhost_ops);
+
+ idx = vhost_ops->vhost_get_vq_index(&net->dev, vq_index);
+
+ r = vhost_dev_virtqueue_restart(&net->dev, vdev, idx);
+ if (r < 0) {
+ goto err_start;
+ }
+
+ if (net->nc->info->type == NET_CLIENT_DRIVER_TAP) {
+ file.index = idx;
+ file.fd = net->backend;
+ r = vhost_net_set_backend(&net->dev, &file);
+ if (r < 0) {
+ r = -errno;
+ goto err_start;
+ }
+ }
+
+ return 0;
+
+err_start:
+ error_report("Error when restarting the queue.");
+
+ if (net->nc->info->type == NET_CLIENT_DRIVER_TAP) {
+ file.fd = -1;
+ file.index = idx;
+ int r = vhost_net_set_backend(&net->dev, &file);
+ assert(r >= 0);
+ }
+
+ vhost_dev_stop(&net->dev, vdev);
+
+ return r;
+}
diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h
index 9b3aaf3814..e11a297380 100644
--- a/include/net/vhost_net.h
+++ b/include/net/vhost_net.h
@@ -50,4 +50,6 @@ int vhost_net_set_mtu(struct vhost_net *net,
uint16_t mtu);
void vhost_net_virtqueue_stop(VirtIODevice *vdev, NetClientState
*nc,
int vq_index);
+int vhost_net_virtqueue_restart(VirtIODevice *vdev, NetClientState
*nc,
+ int vq_index);
#endif
Re: [PATCH v2 12/24] vhost-net: vhost-kernel: introduce vhost_net_virtqueue_restart()
在 2022/8/24 17:01, Jason Wang 写道:
在 2022/8/24 10:53, Kangjie Xu 写道:
在 2022/8/24 10:44, Jason Wang 写道:
在 2022/8/16 09:06, Kangjie Xu 写道:
Introduce vhost_net_virtqueue_restart(), which can restart the
virtqueue when the vhost net started running before. If it fails
to restart the virtqueue, the device will be stopped.
This patch only considers the case for vhost-kernel, when
NetClientDriver is NET_CLIENT_DRIVER_TAP.
Signed-off-by: Kangjie Xu
Signed-off-by: Xuan Zhuo
I would explain why current
vhost_net_start_one()/vhost_net_stop_one() can't work. Is it because
it works at queue pair level? If yes can we restructure the code and
try to reuse ?
Thanks
Because vhost_net_start_one()/vhost_net_stop_one() works at device
level.
The queue pair level start/stop are vhost_virtqueue_start() and
vhost_virtqueue_stop().
What we can reuse is the vhost_virtqueue_start().
vhost_virtqueue_stop() cannot be reused because it will destroy device.
Let's add this in the changelog or a comment in the code.
Thanks
Will fix.
Thanks.
I think we do not need to restructure because we've already had an
abstraction vhost_virtqueue_start().
Thanks.
---
hw/net/vhost_net.c | 48
+
include/net/vhost_net.h | 2 ++
2 files changed, 50 insertions(+)
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index aa60dd901c..2ab67e875e 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -535,3 +535,51 @@ void vhost_net_virtqueue_stop(VirtIODevice
*vdev, NetClientState *nc,
vhost_dev_virtqueue_stop(&net->dev, vdev, idx);
}
+
+int vhost_net_virtqueue_restart(VirtIODevice *vdev, NetClientState
*nc,
+ int vq_index)
+{
+ VHostNetState *net = get_vhost_net(nc->peer);
+ const VhostOps *vhost_ops = net->dev.vhost_ops;
+ struct vhost_vring_file file = { };
+ int idx, r;
+
+ if (!net->dev.started) {
+ return 0;
+ }
+
+ assert(vhost_ops);
+
+ idx = vhost_ops->vhost_get_vq_index(&net->dev, vq_index);
+
+ r = vhost_dev_virtqueue_restart(&net->dev, vdev, idx);
+ if (r < 0) {
+ goto err_start;
+ }
+
+ if (net->nc->info->type == NET_CLIENT_DRIVER_TAP) {
+ file.index = idx;
+ file.fd = net->backend;
+ r = vhost_net_set_backend(&net->dev, &file);
+ if (r < 0) {
+ r = -errno;
+ goto err_start;
+ }
+ }
+
+ return 0;
+
+err_start:
+ error_report("Error when restarting the queue.");
+
+ if (net->nc->info->type == NET_CLIENT_DRIVER_TAP) {
+ file.fd = -1;
+ file.index = idx;
+ int r = vhost_net_set_backend(&net->dev, &file);
+ assert(r >= 0);
+ }
+
+ vhost_dev_stop(&net->dev, vdev);
+
+ return r;
+}
diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h
index 9b3aaf3814..e11a297380 100644
--- a/include/net/vhost_net.h
+++ b/include/net/vhost_net.h
@@ -50,4 +50,6 @@ int vhost_net_set_mtu(struct vhost_net *net,
uint16_t mtu);
void vhost_net_virtqueue_stop(VirtIODevice *vdev,
NetClientState *nc,
int vq_index);
+int vhost_net_virtqueue_restart(VirtIODevice *vdev, NetClientState
*nc,
+ int vq_index);
#endif
Re: [PATCH 2/5] vdpa: Add vhost_vdpa_net_load_mq
On Wed, Aug 24, 2022 at 5:06 PM Eugenio Perez Martin
wrote:
>
> On Wed, Aug 24, 2022 at 10:52 AM Jason Wang wrote:
> >
> > On Wed, Aug 24, 2022 at 3:47 PM Eugenio Perez Martin
> > wrote:
> > >
> > > On Wed, Aug 24, 2022 at 6:23 AM Jason Wang wrote:
> > > >
> > > >
> > > > 在 2022/8/20 01:13, Eugenio Pérez 写道:
> > > > > Same way as with the MAC, restore the expected number of queues at
> > > > > device's start.
> > > > >
> > > > > Signed-off-by: Eugenio Pérez
> > > > > ---
> > > > > net/vhost-vdpa.c | 33 +
> > > > > 1 file changed, 33 insertions(+)
> > > > >
> > > > > diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> > > > > index 1e0dbfcced..96fd3bc835 100644
> > > > > --- a/net/vhost-vdpa.c
> > > > > +++ b/net/vhost-vdpa.c
> > > > > @@ -391,6 +391,35 @@ static int
> > > > > vhost_vdpa_net_load_mac(VhostVDPAState *s,
> > > > > return 0;
> > > > > }
> > > > >
> > > > > +static int vhost_vdpa_net_load_mq(VhostVDPAState *s,
> > > > > + const VirtIONet *n)
> > > > > +{
> > > > > +uint64_t features = n->parent_obj.guest_features;
> > > > > +ssize_t dev_written;
> > > > > +void *cursor = s->cvq_cmd_out_buffer;
> > > > > +if (!(features & BIT_ULL(VIRTIO_NET_F_MQ))) {
> > > > > +return 0;
> > > > > +}
> > > > > +
> > > > > +*(struct virtio_net_ctrl_hdr *)cursor = (struct
> > > > > virtio_net_ctrl_hdr) {
> > > > > +.class = VIRTIO_NET_CTRL_MQ,
> > > > > +.cmd = VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET,
> > > > > +};
> > > > > +cursor += sizeof(struct virtio_net_ctrl_hdr);
> > > > > +*(struct virtio_net_ctrl_mq *)cursor = (struct
> > > > > virtio_net_ctrl_mq) {
> > > > > +.virtqueue_pairs = cpu_to_le16(n->curr_queue_pairs),
> > > > > +};
> > > >
> > > >
> > > > Such casting is not elegant, let's just prepare buffer and then do the
> > > > copy inside vhost_vdpa_net_cvq_add()?
> > > >
> > >
> > > I'm not sure what you propose here. I can pre-fill a buffer in the
> > > stack and then do an extra copy in vhost_vdpa_net_cvq_add. The
> > > compiler should be able to optimize it, but I'm not sure if it
> > > simplifies the code.
> > >
> > > We can have a dedicated buffer for mac, another for mq, and one for
> > > each different command, and map all of them at the device's start. But
> > > this seems too much overhead to me.
> >
> > Considering we may need to support and restore a lot of other fields,
> > this looks a little complicated.
> >
> > I meant the caller can simply do:
> >
> > struct virtio_net_ctrl_mq mq = { ...};
> >
> > Then we do
> >
> > vhost_vdpa_net_cvq_add(&mq, sizeof(mq), ...);
> >
> > Then we can do memcpy inside vhost_vdpa_net_cvq_add() and hide the
> > cmd_out_buffer etc from the caller.
> >
>
> We need to add the ctrl header too. But yes, that is feasible, something like:
>
> vhost_vdpa_net_cvq_add(&ctrl, &mq, sizeof(mq), ...);
>
> > >
> > > Some alternatives that come to my mind:
> > >
> > > * Declare a struct with both virtio_net_ctrl_hdr and each of the
> > > control commands (using unions?), and cast s->cvq_cmd_out_buffer
> > > accordingly.
> > > * Declare a struct with all of the supported commands one after
> > > another, and let qemu fill and send these accordingly.
> > >
> > > >
> > > > > +cursor += sizeof(struct virtio_net_ctrl_mq);
> > > > > +
> > > > > +dev_written = vhost_vdpa_net_cvq_add(s, cursor -
> > > > > s->cvq_cmd_out_buffer,
> > > > > +
> > > > > sizeof(virtio_net_ctrl_ack));
> > > > > +if (unlikely(dev_written < 0)) {
> > > > > +return dev_written;
> > > > > +}
> > > > > +
> > > > > +return *((virtio_net_ctrl_ack *)s->cvq_cmd_in_buffer) !=
> > > > > VIRTIO_NET_OK;
> > > >
> > > >
> > > > So I think we should have a dedicated buffer just for ack, then there's
> > > > no need for such casting.
> > > >
> > >
> > > You mean to declare cvq_cmd_in_buffer as virtio_net_ctrl_ack type
> > > directly and map it to the device?
> >
> > Kind of, considering the ack is the only kind of structure in the near
> > future, can we simply use the structure virtio_net_ctl_ack?
> >
>
> Almost, but we need to map to the device in a page size. And I think
> it's better to allocate a whole page for that, so it does not share
> memory with qemu.
I guess using a union will solve the problem?
Thanks
>
> Other than that, yes, I think it can be declared as virtio_net_ctl_ack
> directly.
>
> Thanks!
>
Re: [PATCH v2 15/24] vhost-user: add op to enable or disable a single vring
在 2022/8/24 11:09, Kangjie Xu 写道:
在 2022/8/24 10:53, Jason Wang 写道:
在 2022/8/16 09:06, Kangjie Xu 写道:
The interface to set enable status for a single vring is lacked in
VhostOps, since the vhost_set_vring_enable_op will manipulate all
virtqueues in a device.
Resetting a single vq will rely on this interface.
Signed-off-by: Kangjie Xu
Signed-off-by: Xuan Zhuo
---
hw/virtio/vhost-user.c | 26 +++---
include/hw/virtio/vhost-backend.h | 3 +++
2 files changed, 22 insertions(+), 7 deletions(-)
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 56033f7a92..8307976cda 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -1199,6 +1199,22 @@ static int vhost_user_set_vring_base(struct
vhost_dev *dev,
return vhost_set_vring(dev, VHOST_USER_SET_VRING_BASE, ring);
}
+static int vhost_user_set_single_vring_enable(struct vhost_dev *dev,
+ int index,
+ int enable)
+{
+ if (index < dev->vq_index || index >= dev->vq_index + dev->nvqs) {
+ return -EINVAL;
+ }
+
+ struct vhost_vring_state state = {
+ .index = index,
+ .num = enable,
+ };
+
+ return vhost_set_vring(dev, VHOST_USER_SET_VRING_ENABLE, &state);
+}
+
static int vhost_user_set_vring_enable(struct vhost_dev *dev, int
enable)
{
int i;
@@ -1208,13 +1224,8 @@ static int vhost_user_set_vring_enable(struct
vhost_dev *dev, int enable)
}
for (i = 0; i < dev->nvqs; ++i) {
- int ret;
- struct vhost_vring_state state = {
- .index = dev->vq_index + i,
- .num = enable,
- };
-
- ret = vhost_set_vring(dev, VHOST_USER_SET_VRING_ENABLE,
&state);
Then I'd squash this into previous patch or re-roder to let this
patch (vhost_user_set_single_vring_enable()) to be first.
Thanks
Sorry, I don't get why we should re-order them, since these two
patches are independent.
I meant it's not good to introduce some codes in patch 14 but delete
them in patch 15 (the above part for example).
Thanks
Thanks
+ int ret = vhost_user_set_single_vring_enable(dev,
dev->vq_index + i,
+ enable);
if (ret < 0) {
/*
* Restoring the previous state is likely infeasible,
as well as
@@ -2668,6 +2679,7 @@ const VhostOps user_ops = {
.vhost_reset_vring = vhost_user_reset_vring,
.vhost_reset_device = vhost_user_reset_device,
.vhost_get_vq_index = vhost_user_get_vq_index,
+ .vhost_set_single_vring_enable =
vhost_user_set_single_vring_enable,
.vhost_set_vring_enable = vhost_user_set_vring_enable,
.vhost_requires_shm_log = vhost_user_requires_shm_log,
.vhost_migration_done = vhost_user_migration_done,
diff --git a/include/hw/virtio/vhost-backend.h
b/include/hw/virtio/vhost-backend.h
index f23bf71a8d..38f6b752ff 100644
--- a/include/hw/virtio/vhost-backend.h
+++ b/include/hw/virtio/vhost-backend.h
@@ -83,6 +83,8 @@ typedef int (*vhost_reset_vring_op)(struct
vhost_dev *dev,
struct vhost_vring_state *ring);
typedef int (*vhost_reset_device_op)(struct vhost_dev *dev);
typedef int (*vhost_get_vq_index_op)(struct vhost_dev *dev, int idx);
+typedef int (*vhost_set_single_vring_enable_op)(struct vhost_dev *dev,
+ int index, int
enable);
typedef int (*vhost_set_vring_enable_op)(struct vhost_dev *dev,
int enable);
typedef bool (*vhost_requires_shm_log_op)(struct vhost_dev *dev);
@@ -158,6 +160,7 @@ typedef struct VhostOps {
vhost_reset_device_op vhost_reset_device;
vhost_reset_vring_op vhost_reset_vring;
vhost_get_vq_index_op vhost_get_vq_index;
+ vhost_set_single_vring_enable_op vhost_set_single_vring_enable;
vhost_set_vring_enable_op vhost_set_vring_enable;
vhost_requires_shm_log_op vhost_requires_shm_log;
vhost_migration_done_op vhost_migration_done;
Re: [PATCH v2 18/24] vhost-net: vhost-user: update vhost_net_virtqueue_stop()
在 2022/8/24 17:04, Jason Wang 写道:
在 2022/8/24 12:57, Kangjie Xu 写道:
在 2022/8/24 12:05, Jason Wang 写道:
在 2022/8/16 09:06, Kangjie Xu 写道:
Update vhost_net_virtqueue_stop() for vhost-user scenario.
Let's explain why it is needed now or why it doesn't cause any issue
or it's a bug fix or not.
Thanks
This patch is to suppport vq reset for vhost-user.
We need this simply because the behavior of vhost_ops->get_vq_index()
is different in vhost-user and vhost-kernel.
vhost_user_get_vq_index(dev, idx) simply returns "idx".
vhost_kernel_get_vq_index(dev, idx) returns "idx - dev->vq_index".
Thanks
Let's add them in the change-log in the next version.
Sorry, i don't get what to be changed here, could you explain it?
But the question still, is this a bug fix (requires a Fixes tag)? If
not why do we need this now?
Thanks
Actually, it is not a bugfix, it is simply intended to support vhost-user.
Because vhost_ops->get_vq_index returns different values for
vhost-kernel and vhost-user.
To align vhost-kernel and vhost-user and reuse the following code,
vhost_dev_virtqueue_stop(&net->dev, vdev, idx);
we process the 'idx' here for vhost-user specifically.
Thanks.
Signed-off-by: Kangjie Xu
Signed-off-by: Xuan Zhuo
---
hw/net/vhost_net.c | 4
1 file changed, 4 insertions(+)
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 2ab67e875e..c0d408f3b4 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -533,6 +533,10 @@ void vhost_net_virtqueue_stop(VirtIODevice
*vdev, NetClientState *nc,
assert(r >= 0);
}
+ if (net->nc->info->type == NET_CLIENT_DRIVER_VHOST_USER) {
+ idx = idx - net->dev.vq_index;
+ }
+
vhost_dev_virtqueue_stop(&net->dev, vdev, idx);
}
Re: [PATCH 2/5] vdpa: Add vhost_vdpa_net_load_mq
On Wed, Aug 24, 2022 at 11:08 AM Jason Wang wrote:
>
> On Wed, Aug 24, 2022 at 5:06 PM Eugenio Perez Martin
> wrote:
> >
> > On Wed, Aug 24, 2022 at 10:52 AM Jason Wang wrote:
> > >
> > > On Wed, Aug 24, 2022 at 3:47 PM Eugenio Perez Martin
> > > wrote:
> > > >
> > > > On Wed, Aug 24, 2022 at 6:23 AM Jason Wang wrote:
> > > > >
> > > > >
> > > > > 在 2022/8/20 01:13, Eugenio Pérez 写道:
> > > > > > Same way as with the MAC, restore the expected number of queues at
> > > > > > device's start.
> > > > > >
> > > > > > Signed-off-by: Eugenio Pérez
> > > > > > ---
> > > > > > net/vhost-vdpa.c | 33 +
> > > > > > 1 file changed, 33 insertions(+)
> > > > > >
> > > > > > diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> > > > > > index 1e0dbfcced..96fd3bc835 100644
> > > > > > --- a/net/vhost-vdpa.c
> > > > > > +++ b/net/vhost-vdpa.c
> > > > > > @@ -391,6 +391,35 @@ static int
> > > > > > vhost_vdpa_net_load_mac(VhostVDPAState *s,
> > > > > > return 0;
> > > > > > }
> > > > > >
> > > > > > +static int vhost_vdpa_net_load_mq(VhostVDPAState *s,
> > > > > > + const VirtIONet *n)
> > > > > > +{
> > > > > > +uint64_t features = n->parent_obj.guest_features;
> > > > > > +ssize_t dev_written;
> > > > > > +void *cursor = s->cvq_cmd_out_buffer;
> > > > > > +if (!(features & BIT_ULL(VIRTIO_NET_F_MQ))) {
> > > > > > +return 0;
> > > > > > +}
> > > > > > +
> > > > > > +*(struct virtio_net_ctrl_hdr *)cursor = (struct
> > > > > > virtio_net_ctrl_hdr) {
> > > > > > +.class = VIRTIO_NET_CTRL_MQ,
> > > > > > +.cmd = VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET,
> > > > > > +};
> > > > > > +cursor += sizeof(struct virtio_net_ctrl_hdr);
> > > > > > +*(struct virtio_net_ctrl_mq *)cursor = (struct
> > > > > > virtio_net_ctrl_mq) {
> > > > > > +.virtqueue_pairs = cpu_to_le16(n->curr_queue_pairs),
> > > > > > +};
> > > > >
> > > > >
> > > > > Such casting is not elegant, let's just prepare buffer and then do the
> > > > > copy inside vhost_vdpa_net_cvq_add()?
> > > > >
> > > >
> > > > I'm not sure what you propose here. I can pre-fill a buffer in the
> > > > stack and then do an extra copy in vhost_vdpa_net_cvq_add. The
> > > > compiler should be able to optimize it, but I'm not sure if it
> > > > simplifies the code.
> > > >
> > > > We can have a dedicated buffer for mac, another for mq, and one for
> > > > each different command, and map all of them at the device's start. But
> > > > this seems too much overhead to me.
> > >
> > > Considering we may need to support and restore a lot of other fields,
> > > this looks a little complicated.
> > >
> > > I meant the caller can simply do:
> > >
> > > struct virtio_net_ctrl_mq mq = { ...};
> > >
> > > Then we do
> > >
> > > vhost_vdpa_net_cvq_add(&mq, sizeof(mq), ...);
> > >
> > > Then we can do memcpy inside vhost_vdpa_net_cvq_add() and hide the
> > > cmd_out_buffer etc from the caller.
> > >
> >
> > We need to add the ctrl header too. But yes, that is feasible, something
> > like:
> >
> > vhost_vdpa_net_cvq_add(&ctrl, &mq, sizeof(mq), ...);
> >
> > > >
> > > > Some alternatives that come to my mind:
> > > >
> > > > * Declare a struct with both virtio_net_ctrl_hdr and each of the
> > > > control commands (using unions?), and cast s->cvq_cmd_out_buffer
> > > > accordingly.
> > > > * Declare a struct with all of the supported commands one after
> > > > another, and let qemu fill and send these accordingly.
> > > >
> > > > >
> > > > > > +cursor += sizeof(struct virtio_net_ctrl_mq);
> > > > > > +
> > > > > > +dev_written = vhost_vdpa_net_cvq_add(s, cursor -
> > > > > > s->cvq_cmd_out_buffer,
> > > > > > +
> > > > > > sizeof(virtio_net_ctrl_ack));
> > > > > > +if (unlikely(dev_written < 0)) {
> > > > > > +return dev_written;
> > > > > > +}
> > > > > > +
> > > > > > +return *((virtio_net_ctrl_ack *)s->cvq_cmd_in_buffer) !=
> > > > > > VIRTIO_NET_OK;
> > > > >
> > > > >
> > > > > So I think we should have a dedicated buffer just for ack, then
> > > > > there's
> > > > > no need for such casting.
> > > > >
> > > >
> > > > You mean to declare cvq_cmd_in_buffer as virtio_net_ctrl_ack type
> > > > directly and map it to the device?
> > >
> > > Kind of, considering the ack is the only kind of structure in the near
> > > future, can we simply use the structure virtio_net_ctl_ack?
> > >
> >
> > Almost, but we need to map to the device in a page size. And I think
> > it's better to allocate a whole page for that, so it does not share
> > memory with qemu.
>
> I guess using a union will solve the problem?
>
It was more a nitpick than a problem, pointing out the need to
allocate a whole page casting it or not to virtio_net_ctrl_ack. In
other words, we must init status as "status =
g_malloc0(real_host_page_size())", not "g_malloc0(sizeof(*status))".
But I think the union is a good idea. The
Re: [PATCH 2/5] vdpa: Add vhost_vdpa_net_load_mq
On Wed, Aug 24, 2022 at 10:52 AM Jason Wang wrote:
>
> On Wed, Aug 24, 2022 at 3:47 PM Eugenio Perez Martin
> wrote:
> >
> > On Wed, Aug 24, 2022 at 6:23 AM Jason Wang wrote:
> > >
> > >
> > > 在 2022/8/20 01:13, Eugenio Pérez 写道:
> > > > Same way as with the MAC, restore the expected number of queues at
> > > > device's start.
> > > >
> > > > Signed-off-by: Eugenio Pérez
> > > > ---
> > > > net/vhost-vdpa.c | 33 +
> > > > 1 file changed, 33 insertions(+)
> > > >
> > > > diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> > > > index 1e0dbfcced..96fd3bc835 100644
> > > > --- a/net/vhost-vdpa.c
> > > > +++ b/net/vhost-vdpa.c
> > > > @@ -391,6 +391,35 @@ static int vhost_vdpa_net_load_mac(VhostVDPAState
> > > > *s,
> > > > return 0;
> > > > }
> > > >
> > > > +static int vhost_vdpa_net_load_mq(VhostVDPAState *s,
> > > > + const VirtIONet *n)
> > > > +{
> > > > +uint64_t features = n->parent_obj.guest_features;
> > > > +ssize_t dev_written;
> > > > +void *cursor = s->cvq_cmd_out_buffer;
> > > > +if (!(features & BIT_ULL(VIRTIO_NET_F_MQ))) {
> > > > +return 0;
> > > > +}
> > > > +
> > > > +*(struct virtio_net_ctrl_hdr *)cursor = (struct
> > > > virtio_net_ctrl_hdr) {
> > > > +.class = VIRTIO_NET_CTRL_MQ,
> > > > +.cmd = VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET,
> > > > +};
> > > > +cursor += sizeof(struct virtio_net_ctrl_hdr);
> > > > +*(struct virtio_net_ctrl_mq *)cursor = (struct virtio_net_ctrl_mq)
> > > > {
> > > > +.virtqueue_pairs = cpu_to_le16(n->curr_queue_pairs),
> > > > +};
> > >
> > >
> > > Such casting is not elegant, let's just prepare buffer and then do the
> > > copy inside vhost_vdpa_net_cvq_add()?
> > >
> >
> > I'm not sure what you propose here. I can pre-fill a buffer in the
> > stack and then do an extra copy in vhost_vdpa_net_cvq_add. The
> > compiler should be able to optimize it, but I'm not sure if it
> > simplifies the code.
> >
> > We can have a dedicated buffer for mac, another for mq, and one for
> > each different command, and map all of them at the device's start. But
> > this seems too much overhead to me.
>
> Considering we may need to support and restore a lot of other fields,
> this looks a little complicated.
>
> I meant the caller can simply do:
>
> struct virtio_net_ctrl_mq mq = { ...};
>
> Then we do
>
> vhost_vdpa_net_cvq_add(&mq, sizeof(mq), ...);
>
> Then we can do memcpy inside vhost_vdpa_net_cvq_add() and hide the
> cmd_out_buffer etc from the caller.
>
We need to add the ctrl header too. But yes, that is feasible, something like:
vhost_vdpa_net_cvq_add(&ctrl, &mq, sizeof(mq), ...);
> >
> > Some alternatives that come to my mind:
> >
> > * Declare a struct with both virtio_net_ctrl_hdr and each of the
> > control commands (using unions?), and cast s->cvq_cmd_out_buffer
> > accordingly.
> > * Declare a struct with all of the supported commands one after
> > another, and let qemu fill and send these accordingly.
> >
> > >
> > > > +cursor += sizeof(struct virtio_net_ctrl_mq);
> > > > +
> > > > +dev_written = vhost_vdpa_net_cvq_add(s, cursor -
> > > > s->cvq_cmd_out_buffer,
> > > > +
> > > > sizeof(virtio_net_ctrl_ack));
> > > > +if (unlikely(dev_written < 0)) {
> > > > +return dev_written;
> > > > +}
> > > > +
> > > > +return *((virtio_net_ctrl_ack *)s->cvq_cmd_in_buffer) !=
> > > > VIRTIO_NET_OK;
> > >
> > >
> > > So I think we should have a dedicated buffer just for ack, then there's
> > > no need for such casting.
> > >
> >
> > You mean to declare cvq_cmd_in_buffer as virtio_net_ctrl_ack type
> > directly and map it to the device?
>
> Kind of, considering the ack is the only kind of structure in the near
> future, can we simply use the structure virtio_net_ctl_ack?
>
Almost, but we need to map to the device in a page size. And I think
it's better to allocate a whole page for that, so it does not share
memory with qemu.
Other than that, yes, I think it can be declared as virtio_net_ctl_ack directly.
Thanks!
[PULL v2 for 7.1 0/6] testing and doc updates
The following changes since commit a8cc5842b5cb863e46a2d009151c6ccbdecadaba: Merge tag 'for-upstream' of git://repo.or.cz/qemu/kevin into staging (2022-08-23 10:37:21 -0700) are available in the Git repository at: https://github.com/stsquad/qemu.git tags/pull-for-7.1-fixes-240822-3 for you to fetch changes up to 5af2b0f6eace7b368ed5cad9677e3bc995b6a7e3: qemu-options: try and clarify preferred block semantics (2022-08-24 10:14:49 +0100) Testing and doc updates: - move default timeout to QemuBaseTests - optimise migration tests to run faster - removed duplicate migration test - add some clarifying language to block options in manual Alex Bennée (2): tests/avocado: push default timeout to QemuBaseTest qemu-options: try and clarify preferred block semantics Thomas Huth (4): tests/qtest/migration-test: Only wait for serial output where migration succeeds tests/migration/aarch64: Speed up the aarch64 migration test tests/migration/i386: Speed up the i386 migration test (when using TCG) tests/qtest/migration-test: Remove duplicated test_postcopy from the test plan tests/migration/aarch64/a-b-kernel.h | 10 +- tests/migration/i386/a-b-bootblock.h | 12 ++-- tests/qtest/migration-test.c | 5 +++-- qemu-options.hx| 13 + tests/avocado/avocado_qemu/__init__.py | 5 - tests/migration/aarch64/a-b-kernel.S | 3 +-- tests/migration/i386/a-b-bootblock.S | 1 + 7 files changed, 33 insertions(+), 16 deletions(-) -- 2.30.2
[PATCH v1 4/5] vhost-user-blk: make 'config_wce' part of 'host_features'
No reason to have this be a separate field. This also makes it more akin
to what the virtio-blk device does.
Signed-off-by: Daniil Tatianin
---
hw/block/vhost-user-blk.c | 6 ++
include/hw/virtio/vhost-user-blk.h | 1 -
2 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index e89164c358..64f3457373 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -262,9 +262,6 @@ static uint64_t vhost_user_blk_get_features(VirtIODevice
*vdev,
virtio_add_feature(&features, VIRTIO_BLK_F_FLUSH);
virtio_add_feature(&features, VIRTIO_BLK_F_RO);
-if (s->config_wce) {
-virtio_add_feature(&features, VIRTIO_BLK_F_CONFIG_WCE);
-}
if (s->num_queues > 1) {
virtio_add_feature(&features, VIRTIO_BLK_F_MQ);
}
@@ -591,7 +588,8 @@ static Property vhost_user_blk_properties[] = {
DEFINE_PROP_UINT16("num-queues", VHostUserBlk, num_queues,
VHOST_USER_BLK_AUTO_NUM_QUEUES),
DEFINE_PROP_UINT32("queue-size", VHostUserBlk, queue_size, 128),
-DEFINE_PROP_BIT("config-wce", VHostUserBlk, config_wce, 0, true),
+DEFINE_PROP_BIT64("config-wce", VHostUserBlk, host_features,
+ VIRTIO_BLK_F_CONFIG_WCE, true),
DEFINE_PROP_BIT64("discard", VHostUserBlk, host_features,
VIRTIO_BLK_F_DISCARD, true),
DEFINE_PROP_BIT64("write-zeroes", VHostUserBlk, host_features,
diff --git a/include/hw/virtio/vhost-user-blk.h
b/include/hw/virtio/vhost-user-blk.h
index 20573dd586..6252095c45 100644
--- a/include/hw/virtio/vhost-user-blk.h
+++ b/include/hw/virtio/vhost-user-blk.h
@@ -34,7 +34,6 @@ struct VHostUserBlk {
struct virtio_blk_config blkcfg;
uint16_t num_queues;
uint32_t queue_size;
-uint32_t config_wce;
struct vhost_dev dev;
struct vhost_inflight *inflight;
VhostUserState vhost_user;
--
2.25.1
[PATCH v1 3/5] vhost-user-blk: make it possible to disable write-zeroes/discard
It is useful to have the ability to disable these features for
compatibility with older VMs that don't have these implemented.
Signed-off-by: Daniil Tatianin
---
hw/block/vhost-user-blk.c | 8 ++--
include/hw/virtio/vhost-user-blk.h | 2 ++
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index 9117222456..e89164c358 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -251,6 +251,8 @@ static uint64_t vhost_user_blk_get_features(VirtIODevice
*vdev,
{
VHostUserBlk *s = VHOST_USER_BLK(vdev);
+features |= s->host_features;
+
/* Turn on pre-defined features */
virtio_add_feature(&features, VIRTIO_BLK_F_SIZE_MAX);
virtio_add_feature(&features, VIRTIO_BLK_F_SEG_MAX);
@@ -259,8 +261,6 @@ static uint64_t vhost_user_blk_get_features(VirtIODevice
*vdev,
virtio_add_feature(&features, VIRTIO_BLK_F_BLK_SIZE);
virtio_add_feature(&features, VIRTIO_BLK_F_FLUSH);
virtio_add_feature(&features, VIRTIO_BLK_F_RO);
-virtio_add_feature(&features, VIRTIO_BLK_F_DISCARD);
-virtio_add_feature(&features, VIRTIO_BLK_F_WRITE_ZEROES);
if (s->config_wce) {
virtio_add_feature(&features, VIRTIO_BLK_F_CONFIG_WCE);
@@ -592,6 +592,10 @@ static Property vhost_user_blk_properties[] = {
VHOST_USER_BLK_AUTO_NUM_QUEUES),
DEFINE_PROP_UINT32("queue-size", VHostUserBlk, queue_size, 128),
DEFINE_PROP_BIT("config-wce", VHostUserBlk, config_wce, 0, true),
+DEFINE_PROP_BIT64("discard", VHostUserBlk, host_features,
+ VIRTIO_BLK_F_DISCARD, true),
+DEFINE_PROP_BIT64("write-zeroes", VHostUserBlk, host_features,
+ VIRTIO_BLK_F_WRITE_ZEROES, true),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/include/hw/virtio/vhost-user-blk.h
b/include/hw/virtio/vhost-user-blk.h
index 7c91f15040..20573dd586 100644
--- a/include/hw/virtio/vhost-user-blk.h
+++ b/include/hw/virtio/vhost-user-blk.h
@@ -51,6 +51,8 @@ struct VHostUserBlk {
bool connected;
/* vhost_user_blk_start/vhost_user_blk_stop */
bool started_vu;
+
+uint64_t host_features;
};
#endif
--
2.25.1
Re: [PATCH v2 15/24] vhost-user: add op to enable or disable a single vring
在 2022/8/24 17:02, Jason Wang 写道:
在 2022/8/24 11:09, Kangjie Xu 写道:
在 2022/8/24 10:53, Jason Wang 写道:
在 2022/8/16 09:06, Kangjie Xu 写道:
The interface to set enable status for a single vring is lacked in
VhostOps, since the vhost_set_vring_enable_op will manipulate all
virtqueues in a device.
Resetting a single vq will rely on this interface.
Signed-off-by: Kangjie Xu
Signed-off-by: Xuan Zhuo
---
hw/virtio/vhost-user.c | 26 +++---
include/hw/virtio/vhost-backend.h | 3 +++
2 files changed, 22 insertions(+), 7 deletions(-)
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 56033f7a92..8307976cda 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -1199,6 +1199,22 @@ static int vhost_user_set_vring_base(struct
vhost_dev *dev,
return vhost_set_vring(dev, VHOST_USER_SET_VRING_BASE, ring);
}
+static int vhost_user_set_single_vring_enable(struct vhost_dev
*dev,
+ int index,
+ int enable)
+{
+ if (index < dev->vq_index || index >= dev->vq_index +
dev->nvqs) {
+ return -EINVAL;
+ }
+
+ struct vhost_vring_state state = {
+ .index = index,
+ .num = enable,
+ };
+
+ return vhost_set_vring(dev, VHOST_USER_SET_VRING_ENABLE, &state);
+}
+
static int vhost_user_set_vring_enable(struct vhost_dev *dev, int
enable)
{
int i;
@@ -1208,13 +1224,8 @@ static int
vhost_user_set_vring_enable(struct vhost_dev *dev, int enable)
}
for (i = 0; i < dev->nvqs; ++i) {
- int ret;
- struct vhost_vring_state state = {
- .index = dev->vq_index + i,
- .num = enable,
- };
-
- ret = vhost_set_vring(dev, VHOST_USER_SET_VRING_ENABLE,
&state);
Then I'd squash this into previous patch or re-roder to let this
patch (vhost_user_set_single_vring_enable()) to be first.
Thanks
Sorry, I don't get why we should re-order them, since these two
patches are independent.
I meant it's not good to introduce some codes in patch 14 but delete
them in patch 15 (the above part for example).
Thanks
I get your point, but in fact, it seems that the deleded codes here in
patch 15 do not appear in patch 14
Patch 14 is about vhost_user_reset_vring(), patch 15 is about
vhost_user_set_vring_enable().
Thanks.
Thanks
+ int ret = vhost_user_set_single_vring_enable(dev,
dev->vq_index + i,
+ enable);
if (ret < 0) {
/*
* Restoring the previous state is likely infeasible,
as well as
@@ -2668,6 +2679,7 @@ const VhostOps user_ops = {
.vhost_reset_vring = vhost_user_reset_vring,
.vhost_reset_device = vhost_user_reset_device,
.vhost_get_vq_index = vhost_user_get_vq_index,
+ .vhost_set_single_vring_enable =
vhost_user_set_single_vring_enable,
.vhost_set_vring_enable = vhost_user_set_vring_enable,
.vhost_requires_shm_log = vhost_user_requires_shm_log,
.vhost_migration_done = vhost_user_migration_done,
diff --git a/include/hw/virtio/vhost-backend.h
b/include/hw/virtio/vhost-backend.h
index f23bf71a8d..38f6b752ff 100644
--- a/include/hw/virtio/vhost-backend.h
+++ b/include/hw/virtio/vhost-backend.h
@@ -83,6 +83,8 @@ typedef int (*vhost_reset_vring_op)(struct
vhost_dev *dev,
struct vhost_vring_state *ring);
typedef int (*vhost_reset_device_op)(struct vhost_dev *dev);
typedef int (*vhost_get_vq_index_op)(struct vhost_dev *dev, int
idx);
+typedef int (*vhost_set_single_vring_enable_op)(struct vhost_dev
*dev,
+ int index, int
enable);
typedef int (*vhost_set_vring_enable_op)(struct vhost_dev *dev,
int enable);
typedef bool (*vhost_requires_shm_log_op)(struct vhost_dev *dev);
@@ -158,6 +160,7 @@ typedef struct VhostOps {
vhost_reset_device_op vhost_reset_device;
vhost_reset_vring_op vhost_reset_vring;
vhost_get_vq_index_op vhost_get_vq_index;
+ vhost_set_single_vring_enable_op vhost_set_single_vring_enable;
vhost_set_vring_enable_op vhost_set_vring_enable;
vhost_requires_shm_log_op vhost_requires_shm_log;
vhost_migration_done_op vhost_migration_done;
Re: [PULL 1/6] tests/avocado: push default timeout to QemuBaseTest
Richard Henderson writes:
> On 8/23/22 08:25, Alex Bennée wrote:
>> All of the QEMU tests eventually end up derrived from this class. Move
>> the default timeout from LinuxTest to ensure we catch them all. As 15
>> minutes is fairly excessive we drop the default down to 2 minutes
>> which is a more reasonable target for tests to aim for.
>> Signed-off-by: Alex Bennée
>> Reviewed-by: Richard Henderson
>> Message-Id: <20220822165608.2980552-2-alex.ben...@linaro.org>
>> diff --git a/tests/avocado/avocado_qemu/__init__.py
>> b/tests/avocado/avocado_qemu/__init__.py
>> index ed4853c805..0efd2bd212 100644
>> --- a/tests/avocado/avocado_qemu/__init__.py
>> +++ b/tests/avocado/avocado_qemu/__init__.py
>> @@ -227,6 +227,10 @@ def exec_command_and_wait_for_pattern(test, command,
>> _console_interaction(test, success_message, failure_message, command +
>> '\r')
>> class QemuBaseTest(avocado.Test):
>> +
>> +# default timeout for all tests, can be overridden
>> +timeout = 120
>> +
>> def _get_unique_tag_val(self, tag_name):
>> """
>> Gets a tag value, if unique for a key
>> @@ -512,7 +516,6 @@ class LinuxTest(LinuxSSHMixIn, QemuSystemTest):
>> to start with than the more vanilla `QemuSystemTest` class.
>> """
>> -timeout = 900
>> distro = None
>> username = 'root'
>> password = 'password'
>
> Bah.
>
> https://gitlab.com/qemu-project/qemu/-/jobs/2923804714
Hmm weird - the avocado CFI job doesn't even appear on my CI list (even
with push-ci-now). Anyway I've reverted the timeout to 900s and sent a
v2 of the PR. I'll drop it back down to 120s next cycle and explicitly
increase the timeouts for the known slow tests.
>
> (001/192)
> tests/avocado/boot_linux.py:BootLinuxX8664.test_pc_i440fx_tcg:
> INTERRUPTED: Test interrupted by SIGTERM\nRunner error occurred:
> Timeout reached\nOriginal status: ERROR\n{'name':
> '001-tests/avocado/boot_linux.py:BootLinuxX8664.test_pc_i440fx_tcg',
> 'logdir':
>
> '/builds/qemu-project/qemu/build/tests/results/job-2022-08-23T21.03-6d06db2/t...
> (120.85 s)
> (003/192) tests/avocado/boot_linux.py:BootLinuxX8664.test_pc_q35_tcg:
> INTERRUPTED: Test interrupted by SIGTERM\nRunner error occurred:
> Timeout reached\nOriginal status: ERROR\n{'name':
> '003-tests/avocado/boot_linux.py:BootLinuxX8664.test_pc_q35_tcg',
> 'logdir':
>
> '/builds/qemu-project/qemu/build/tests/results/job-2022-08-23T21.03-6d06db2/test...
> (120.81 s)
>
> The previous successful run had
>
> (001/192) tests/avocado/boot_linux.py:BootLinuxX8664.test_pc_i440fx_tcg:
> PASS (257.00 s)
> (003/192) tests/avocado/boot_linux.py:BootLinuxX8664.test_pc_q35_tcg: PASS
> (238.67 s)
>
>
> r~
--
Alex Bennée
[PATCH v1 5/5] vhost-user-blk: dynamically resize config space based on features
Make vhost-user-blk backwards compatible when migrating from older VMs
running with modern features turned off, the same way it was done for
virtio-blk in 20764be0421c ("virtio-blk: set config size depending on the
features enabled")
It's currently impossible to migrate from an older VM with
vhost-user-blk (with disable-legacy=off) because of errors like this:
qemu-system-x86_64: get_pci_config_device: Bad config data: i=0x10 read: 41
device: 1 cmask: ff wmask: 80 w1cmask:0
qemu-system-x86_64: Failed to load PCIDevice:config
qemu-system-x86_64: Failed to load virtio-blk:virtio
qemu-system-x86_64: error while loading state for instance 0x0 of device
':00:05.0:00.0:02.0/virtio-blk'
qemu-system-x86_64: load of migration failed: Invalid argument
This is caused by the newer (destination) VM requiring a bigger BAR0
alignment because it has to cover a bigger configuration space, which
isn't actually needed since those additional config fields are not
active (write-zeroes/discard).
Signed-off-by: Daniil Tatianin
---
hw/block/vhost-user-blk.c | 15 ---
include/hw/virtio/vhost-user-blk.h | 1 +
2 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index 64f3457373..d18a7a2cd4 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -23,6 +23,7 @@
#include "hw/qdev-core.h"
#include "hw/qdev-properties.h"
#include "hw/qdev-properties-system.h"
+#include "hw/virtio/virtio-blk-common.h"
#include "hw/virtio/vhost.h"
#include "hw/virtio/vhost-user-blk.h"
#include "hw/virtio/virtio.h"
@@ -63,7 +64,7 @@ static void vhost_user_blk_update_config(VirtIODevice *vdev,
uint8_t *config)
/* Our num_queues overrides the device backend */
virtio_stw_p(vdev, &s->blkcfg.num_queues, s->num_queues);
-memcpy(config, &s->blkcfg, sizeof(struct virtio_blk_config));
+memcpy(config, &s->blkcfg, s->config_size);
}
static void vhost_user_blk_set_config(VirtIODevice *vdev, const uint8_t
*config)
@@ -96,8 +97,7 @@ static int vhost_user_blk_handle_config_change(struct
vhost_dev *dev)
Error *local_err = NULL;
ret = vhost_dev_get_config(dev, (uint8_t *)&blkcfg,
- sizeof(struct virtio_blk_config),
- &local_err);
+ s->config_size, &local_err);
if (ret < 0) {
error_report_err(local_err);
return ret;
@@ -106,7 +106,7 @@ static int vhost_user_blk_handle_config_change(struct
vhost_dev *dev)
/* valid for resize only */
if (blkcfg.capacity != s->blkcfg.capacity) {
s->blkcfg.capacity = blkcfg.capacity;
-memcpy(dev->vdev->config, &s->blkcfg, sizeof(struct
virtio_blk_config));
+memcpy(dev->vdev->config, &s->blkcfg, s->config_size);
virtio_notify_config(dev->vdev);
}
@@ -444,7 +444,7 @@ static int vhost_user_blk_realize_connect(VHostUserBlk *s,
Error **errp)
assert(s->connected);
ret = vhost_dev_get_config(&s->dev, (uint8_t *)&s->blkcfg,
- sizeof(struct virtio_blk_config), errp);
+ s->config_size, errp);
if (ret < 0) {
qemu_chr_fe_disconnect(&s->chardev);
vhost_dev_cleanup(&s->dev);
@@ -489,8 +489,9 @@ static void vhost_user_blk_device_realize(DeviceState *dev,
Error **errp)
return;
}
-virtio_init(vdev, VIRTIO_ID_BLOCK,
-sizeof(struct virtio_blk_config));
+s->config_size = virtio_blk_common_get_config_size(s->host_features);
+
+virtio_init(vdev, VIRTIO_ID_BLOCK, s->config_size);
s->virtqs = g_new(VirtQueue *, s->num_queues);
for (i = 0; i < s->num_queues; i++) {
diff --git a/include/hw/virtio/vhost-user-blk.h
b/include/hw/virtio/vhost-user-blk.h
index 6252095c45..b7810360b9 100644
--- a/include/hw/virtio/vhost-user-blk.h
+++ b/include/hw/virtio/vhost-user-blk.h
@@ -52,6 +52,7 @@ struct VHostUserBlk {
bool started_vu;
uint64_t host_features;
+size_t config_size;
};
#endif
--
2.25.1
[PATCH 03/51] block: Unify the get_tmp_filename() implementation
From: Bin Meng
At present get_tmp_filename() has platform specific implementations
to get the directory to use for temporary files. Switch over to use
g_get_tmp_dir() which works on all supported platforms.
Signed-off-by: Bin Meng
---
block.c | 16 ++--
1 file changed, 2 insertions(+), 14 deletions(-)
diff --git a/block.c b/block.c
index bc85f46eed..d06df47f72 100644
--- a/block.c
+++ b/block.c
@@ -864,21 +864,10 @@ int bdrv_probe_geometry(BlockDriverState *bs, HDGeometry
*geo)
*/
int get_tmp_filename(char *filename, int size)
{
-#ifdef _WIN32
-char temp_dir[MAX_PATH];
-/* GetTempFileName requires that its output buffer (4th param)
- have length MAX_PATH or greater. */
-assert(size >= MAX_PATH);
-return (GetTempPath(MAX_PATH, temp_dir)
-&& GetTempFileName(temp_dir, "qem", 0, filename)
-? 0 : -GetLastError());
-#else
int fd;
const char *tmpdir;
-tmpdir = getenv("TMPDIR");
-if (!tmpdir) {
-tmpdir = "/var/tmp";
-}
+tmpdir = g_get_tmp_dir();
+
if (snprintf(filename, size, "%s/vl.XX", tmpdir) >= size) {
return -EOVERFLOW;
}
@@ -891,7 +880,6 @@ int get_tmp_filename(char *filename, int size)
return -errno;
}
return 0;
-#endif
}
/*
--
2.34.1
[PATCH v1 0/5] vhost-user-blk: dynamically resize config space based on features
This patch set attempts to align vhost-user-blk with virtio-blk in terms of backward compatibility and flexibility. In particular it adds the following things: - Ability to disable modern features like discard/write-zeroes. - Dynamic configuration space resizing based on enabled features, by reusing the code, which was already present in virtio-blk. - Makes the VHostUserBlk structure a bit less clunky by using the 'host_features' field to represent enabled features, as opposed to using a separate field per feature. This was already done for virtio-blk a long time ago. Daniil Tatianin (5): virtio-blk: decouple config size determination code from VirtIOBlock virtio-blk: move config space sizing code to virtio-blk-common vhost-user-blk: make it possible to disable write-zeroes/discard vhost-user-blk: make 'config_wce' part of 'host_features' vhost-user-blk: dynamically resize config space based on features MAINTAINERS | 4 +++ hw/block/meson.build | 4 +-- hw/block/vhost-user-blk.c | 29 +- hw/block/virtio-blk-common.c | 42 +++ hw/block/virtio-blk.c | 25 ++-- include/hw/virtio/vhost-user-blk.h| 4 ++- include/hw/virtio/virtio-blk-common.h | 21 ++ 7 files changed, 90 insertions(+), 39 deletions(-) create mode 100644 hw/block/virtio-blk-common.c create mode 100644 include/hw/virtio/virtio-blk-common.h -- 2.25.1
[PATCH v1 2/5] virtio-blk: move config space sizing code to virtio-blk-common
This way we can reuse it for other virtio-blk devices, e.g
vhost-user-blk, which currently does not control its config space size
dynamically.
Signed-off-by: Daniil Tatianin
---
MAINTAINERS | 4 +++
hw/block/meson.build | 4 +--
hw/block/virtio-blk-common.c | 42 +++
hw/block/virtio-blk.c | 24 +--
include/hw/virtio/virtio-blk-common.h | 21 ++
5 files changed, 70 insertions(+), 25 deletions(-)
create mode 100644 hw/block/virtio-blk-common.c
create mode 100644 include/hw/virtio/virtio-blk-common.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 5ce4227ff6..a7d3914735 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2030,8 +2030,10 @@ virtio-blk
M: Stefan Hajnoczi
L: qemu-bl...@nongnu.org
S: Supported
+F: hw/block/virtio-blk-common.c
F: hw/block/virtio-blk.c
F: hw/block/dataplane/*
+F: include/hw/virtio/virtio-blk-common.h
F: tests/qtest/virtio-blk-test.c
T: git https://github.com/stefanha/qemu.git block
@@ -2271,11 +2273,13 @@ S: Maintained
F: contrib/vhost-user-blk/
F: contrib/vhost-user-scsi/
F: hw/block/vhost-user-blk.c
+F: hw/block/virtio-blk-common.c
F: hw/scsi/vhost-user-scsi.c
F: hw/virtio/vhost-user-blk-pci.c
F: hw/virtio/vhost-user-scsi-pci.c
F: include/hw/virtio/vhost-user-blk.h
F: include/hw/virtio/vhost-user-scsi.h
+F: include/hw/virtio/virtio-blk-common.h
vhost-user-gpu
M: Marc-André Lureau
diff --git a/hw/block/meson.build b/hw/block/meson.build
index 2389326112..1908abd45c 100644
--- a/hw/block/meson.build
+++ b/hw/block/meson.build
@@ -16,7 +16,7 @@ softmmu_ss.add(when: 'CONFIG_SWIM', if_true: files('swim.c'))
softmmu_ss.add(when: 'CONFIG_XEN', if_true: files('xen-block.c'))
softmmu_ss.add(when: 'CONFIG_TC58128', if_true: files('tc58128.c'))
-specific_ss.add(when: 'CONFIG_VIRTIO_BLK', if_true: files('virtio-blk.c'))
-specific_ss.add(when: 'CONFIG_VHOST_USER_BLK', if_true:
files('vhost-user-blk.c'))
+specific_ss.add(when: 'CONFIG_VIRTIO_BLK', if_true: files('virtio-blk.c',
'virtio-blk-common.c'))
+specific_ss.add(when: 'CONFIG_VHOST_USER_BLK', if_true:
files('vhost-user-blk.c', 'virtio-blk-common.c'))
subdir('dataplane')
diff --git a/hw/block/virtio-blk-common.c b/hw/block/virtio-blk-common.c
new file mode 100644
index 00..ac54568eb6
--- /dev/null
+++ b/hw/block/virtio-blk-common.c
@@ -0,0 +1,42 @@
+/*
+ * Virtio Block Device common helpers
+ *
+ * Copyright IBM, Corp. 2007
+ *
+ * Authors:
+ * Anthony Liguori
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+
+#include "standard-headers/linux/virtio_blk.h"
+#include "hw/virtio/virtio.h"
+#include "hw/virtio/virtio-blk-common.h"
+
+/* Config size before the discard support (hide associated config fields) */
+#define VIRTIO_BLK_CFG_SIZE offsetof(struct virtio_blk_config, \
+ max_discard_sectors)
+
+/*
+ * Starting from the discard feature, we can use this array to properly
+ * set the config size depending on the features enabled.
+ */
+static VirtIOFeature feature_sizes[] = {
+{.flags = 1ULL << VIRTIO_BLK_F_DISCARD,
+ .end = endof(struct virtio_blk_config, discard_sector_alignment)},
+{.flags = 1ULL << VIRTIO_BLK_F_WRITE_ZEROES,
+ .end = endof(struct virtio_blk_config, write_zeroes_may_unmap)},
+{}
+};
+
+size_t virtio_blk_common_get_config_size(uint64_t host_features)
+{
+size_t config_size = MAX(VIRTIO_BLK_CFG_SIZE,
+virtio_feature_get_config_size(feature_sizes, host_features));
+
+assert(config_size <= sizeof(struct virtio_blk_config));
+return config_size;
+}
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index a4162dbbf2..4ca6d0f211 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -32,31 +32,9 @@
#include "hw/virtio/virtio-bus.h"
#include "migration/qemu-file-types.h"
#include "hw/virtio/virtio-access.h"
+#include "hw/virtio/virtio-blk-common.h"
#include "qemu/coroutine.h"
-/* Config size before the discard support (hide associated config fields) */
-#define VIRTIO_BLK_CFG_SIZE offsetof(struct virtio_blk_config, \
- max_discard_sectors)
-/*
- * Starting from the discard feature, we can use this array to properly
- * set the config size depending on the features enabled.
- */
-static const VirtIOFeature feature_sizes[] = {
-{.flags = 1ULL << VIRTIO_BLK_F_DISCARD,
- .end = endof(struct virtio_blk_config, discard_sector_alignment)},
-{.flags = 1ULL << VIRTIO_BLK_F_WRITE_ZEROES,
- .end = endof(struct virtio_blk_config, write_zeroes_may_unmap)},
-{}
-};
-
-static size_t virtio_blk_common_get_config_size(uint64_t host_features)
-{
-size_t config_size = MAX(VIRTIO_BLK_CFG_SIZE,
-virtio_feature_get_config_size(feature_sizes, host_features));
-
-assert(config_size <= sizeof(struct virtio_blk_confi
[PATCH 01/51] tests/qtest: Use g_setenv()
From: Bin Meng
Windows does not provide a setenv() API, but glib does.
Replace setenv() call with the glib version.
Signed-off-by: Bin Meng
---
tests/qtest/fuzz/generic_fuzz.c | 8
tests/qtest/libqtest.c | 2 +-
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/tests/qtest/fuzz/generic_fuzz.c b/tests/qtest/fuzz/generic_fuzz.c
index 447ffe8178..afc1d20355 100644
--- a/tests/qtest/fuzz/generic_fuzz.c
+++ b/tests/qtest/fuzz/generic_fuzz.c
@@ -994,16 +994,16 @@ static GString
*generic_fuzz_predefined_config_cmdline(FuzzTarget *t)
g_assert(t->opaque);
config = t->opaque;
-setenv("QEMU_AVOID_DOUBLE_FETCH", "1", 1);
+g_setenv("QEMU_AVOID_DOUBLE_FETCH", "1", 1);
if (config->argfunc) {
args = config->argfunc();
-setenv("QEMU_FUZZ_ARGS", args, 1);
+g_setenv("QEMU_FUZZ_ARGS", args, 1);
g_free(args);
} else {
g_assert_nonnull(config->args);
-setenv("QEMU_FUZZ_ARGS", config->args, 1);
+g_setenv("QEMU_FUZZ_ARGS", config->args, 1);
}
-setenv("QEMU_FUZZ_OBJECTS", config->objects, 1);
+g_setenv("QEMU_FUZZ_OBJECTS", config->objects, 1);
return generic_fuzz_cmdline(t);
}
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 8c159eacf5..ad6860d774 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -1424,7 +1424,7 @@ QTestState *qtest_inproc_init(QTestState **s, bool log,
const char* arch,
* way, qtest_get_arch works for inproc qtest.
*/
gchar *bin_path = g_strconcat("/qemu-system-", arch, NULL);
-setenv("QTEST_QEMU_BINARY", bin_path, 0);
+g_setenv("QTEST_QEMU_BINARY", bin_path, 0);
g_free(bin_path);
return qts;
--
2.34.1
[PATCH 10/51] hw/usb: dev-mtp: Use g_mkdir_with_parents()
From: Bin Meng
Use the same g_mkdir_with_parents() call to create a directory on
all platforms.
Signed-off-by: Bin Meng
---
hw/usb/dev-mtp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 5831395cef..97c894f231 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -1622,7 +1622,7 @@ static void usb_mtp_write_data(MTPState *s, uint32_t
handle)
if (s->dataset.filename) {
path = g_strdup_printf("%s/%s", parent->path, s->dataset.filename);
if (s->dataset.format == FMT_ASSOCIATION) {
-ret = mkdir(path, mask);
+ret = g_mkdir_with_parents(path, mask);
if (!ret) {
usb_mtp_queue_result(s, RES_OK, d->trans, 3,
QEMU_STORAGE_ID,
--
2.34.1
[PATCH v1 1/5] virtio-blk: decouple config size determination code from VirtIOBlock
Make it more stand-alone so that we can reuse it for other virtio-blk
devices that are not VirtIOBlock in the future commits.
Signed-off-by: Daniil Tatianin
---
hw/block/virtio-blk.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index e9ba752f6b..a4162dbbf2 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -49,12 +49,13 @@ static const VirtIOFeature feature_sizes[] = {
{}
};
-static void virtio_blk_set_config_size(VirtIOBlock *s, uint64_t host_features)
+static size_t virtio_blk_common_get_config_size(uint64_t host_features)
{
-s->config_size = MAX(VIRTIO_BLK_CFG_SIZE,
+size_t config_size = MAX(VIRTIO_BLK_CFG_SIZE,
virtio_feature_get_config_size(feature_sizes, host_features));
-assert(s->config_size <= sizeof(struct virtio_blk_config));
+assert(config_size <= sizeof(struct virtio_blk_config));
+return config_size;
}
static void virtio_blk_init_request(VirtIOBlock *s, VirtQueue *vq,
@@ -1204,7 +1205,7 @@ static void virtio_blk_device_realize(DeviceState *dev,
Error **errp)
return;
}
-virtio_blk_set_config_size(s, s->host_features);
+s->config_size = virtio_blk_common_get_config_size(s->host_features);
virtio_init(vdev, VIRTIO_ID_BLOCK, s->config_size);
--
2.25.1
[PATCH 04/51] semihosting/arm-compat-semi: Avoid using hardcoded /tmp
From: Bin Meng
Use g_get_tmp_dir() to get the directory to use for temporary files.
Signed-off-by: Bin Meng
---
semihosting/arm-compat-semi.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/semihosting/arm-compat-semi.c b/semihosting/arm-compat-semi.c
index e741674238..d5e66cc298 100644
--- a/semihosting/arm-compat-semi.c
+++ b/semihosting/arm-compat-semi.c
@@ -503,7 +503,8 @@ void do_common_semihosting(CPUState *cs)
GET_ARG(0);
GET_ARG(1);
GET_ARG(2);
-len = asprintf(&s, "/tmp/qemu-%x%02x", getpid(), (int)arg1 & 0xff);
+len = asprintf(&s, "%s/qemu-%x%02x", g_get_tmp_dir(),
+ getpid(), (int)arg1 & 0xff);
if (len < 0) {
common_semi_set_ret(cs, -1);
break;
--
2.34.1
[PATCH 11/51] qga/commands-posix-ssh: Use g_mkdir_with_parents()
From: Bin Meng
g_mkdir() is a deprecated API and newer codes should use
g_mkdir_with_parents().
Signed-off-by: Bin Meng
---
qga/commands-posix-ssh.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/qga/commands-posix-ssh.c b/qga/commands-posix-ssh.c
index f3a580b8cc..2460112a38 100644
--- a/qga/commands-posix-ssh.c
+++ b/qga/commands-posix-ssh.c
@@ -59,7 +59,7 @@ static bool
mkdir_for_user(const char *path, const struct passwd *p,
mode_t mode, Error **errp)
{
-if (g_mkdir(path, mode) == -1) {
+if (g_mkdir_with_parents(path, mode) == -1) {
error_setg(errp, "failed to create directory '%s': %s",
path, g_strerror(errno));
return false;
--
2.34.1
[PATCH 00/51] tests/qtest: Enable running qtest on Windows
In prepartion to adding virtio-9p support on Windows, this series
enables running qtest on Windows, so that we can run the virtio-9p
tests on Windows to make sure it does not break accidently.
Patch 1-22 updates various components (mostly test cases) so that
they can build on Windows with the same functionality.
Patch 23 supports the qtest accelerator for Windows.
Patch 31 updates the libqtest core for Windows.
Patch 32-47 are the fixes of qtest errors exposed when running on Windows.
Patch 49 fixes the instability of running qtests on Windows.
Patch 50 updates the CI to run qtests on Windows.
Patch 51 documents best practices of writing portable test cases as
we learned during the enablement of running qtest on Windows.
Based-on: <20220802075200.907360-1-bmeng...@gmail.com>
Bin Meng (41):
tests/qtest: Use g_setenv()
tests/qtest: Use g_mkdtemp()
block: Unify the get_tmp_filename() implementation
semihosting/arm-compat-semi: Avoid using hardcoded /tmp
tcg: Avoid using hardcoded /tmp
util/qemu-sockets: Use g_get_tmp_dir() to get the directory for
temporary files
tests: Avoid using hardcoded /tmp in test cases
block/vvfat: Unify the mkdir() call
fsdev/virtfs-proxy-helper: Use g_mkdir_with_parents()
hw/usb: dev-mtp: Use g_mkdir_with_parents()
qga/commands-posix-ssh: Use g_mkdir_with_parents()
tests: Use g_mkdir_with_parents()
tests/qtest: migration-test: Handle link() for win32
backends/tpm: Exclude headers and macros that don't exist on win32
tests/qtest: Adapt {m48t59,rtc}-test cases for win32
tests/qtest: Build e1000e-test for posix only
tests/qtest: Build virtio-net-test for posix only
tests/qtest: Build cases that use memory-backend-file for posix only
tests/qtest: Build test-filter-{mirror,redirector} cases for posix
only
tests/qtest: i440fx-test: Skip running request_{bios,pflash} for win32
tests/qtest: migration-test: Skip running test_migrate_fd_proto on
win32
tests/qtest: qmp-test: Skip running test_qmp_oob for win32
tests/qtest: libqtest: Exclude the *_fds APIs for win32
tests/qtest: libqtest: Install signal handler via signal()
tests: Skip iotests and qtest when '--without-default-devices'
tests/qtest: Support libqtest to build and run on Windows
tests/qtest: Fix ERROR_SHARING_VIOLATION for win32
tests/qtest: {ahci,ide}-test: Use relative path for temporary files
tests/qtest: bios-tables-test: Adapt the case for win32
tests/qtest: device-plug-test: Reverse the usage of double/single
quotes
tests/qtest: machine-none-test: Use double quotes to pass the cpu
option
tests/qtest: migration-test: Disable IO redirection for win32
tests/qtest: npcm7xx_emc-test: Skip running test_{tx,rx} on win32
tests/qtest: microbit-test: Fix socket access for win32
tests/qtest: prom-env-test: Use double quotes to pass the prom-env
option
tests/qtest: libqtest: Replace the call to close a socket with
closesocket()
tests/qtest: libqtest: Correct the timeout unit of blocking receive
calls for win32
io/channel-watch: Drop a superfluous '#ifdef WIN32'
io/channel-watch: Fix socket watch on Windows
.gitlab-ci.d/windows.yml: Increase the timeout to the runner limit
docs/devel: testing: Document writing portable test cases
Xuzhou Cheng (10):
accel/qtest: Support qtest accelerator for Windows
tests/qtest: libqos: Drop inclusion of
tests/qtest: libqos: Rename malloc.h to libqos-malloc.h
tests/qtest: libqtest: Move global_qtest definition back to libqtest.c
tests/qtest: Use send/recv for socket communication
tests/qtest: {ahci,ide}-test: Open file in binary mode
tests/qtest: virtio-net-failover: Disable migration tests for win32
chardev/char-file: Add FILE_SHARE_WRITE when openning the file for
win32
tests/qtest: migration-test: Kill "to" after migration is canceled
hw/ppc: spapr: Use qemu_vfree() to free spapr->htab
docs/devel/testing.rst| 30
backends/tpm/tpm_ioctl.h | 4 +
include/hw/core/cpu.h | 1 +
tests/qtest/fuzz/generic_fuzz_configs.h | 8 +-
tests/qtest/libqos/generic-pcihost.h | 2 +-
.../libqos/{malloc.h => libqos-malloc.h} | 0
tests/qtest/libqos/libqos.h | 2 +-
tests/qtest/libqos/malloc-pc.h| 2 +-
tests/qtest/libqos/malloc-spapr.h | 2 +-
tests/qtest/libqos/pci-pc.h | 2 +-
tests/qtest/libqos/pci-spapr.h| 2 +-
tests/qtest/libqos/qgraph.h | 2 +-
tests/qtest/libqos/qos_external.h | 2 +-
tests/qtest/libqos/rtas.h | 2 +-
tests/qtest/libqos/virtio.h | 2 +-
tests/qtest/libqtest-single.h | 2 +-
tests/qtest/libqtest.h| 8 +
tests/qtest/migration-helpers.h | 2 +
accel/dummy-cpus.c| 14 +-
block.c
[PATCH 18/51] tests/qtest: Build cases that use memory-backend-file for posix only
From: Bin Meng
As backends/meson.build tells us, hostmem-file.c is only supported on
POSIX platforms, hence any test case that utilizes the memory backend
file should be guarded by CONFIG_POSIX too.
Signed-off-by: Bin Meng
---
tests/qtest/bios-tables-test.c | 10 ++
tests/qtest/cxl-test.c | 4
tests/qtest/meson.build| 3 ++-
3 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index 7c5f736b51..36783966b0 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -1461,6 +1461,7 @@ static void test_acpi_piix4_tcg_acpi_hmat(void)
test_acpi_tcg_acpi_hmat(MACHINE_PC);
}
+#ifdef CONFIG_POSIX
static void test_acpi_erst(const char *machine)
{
gchar *tmp_path = g_dir_make_tmp("qemu-test-erst.XX", NULL);
@@ -1511,6 +1512,7 @@ static void test_acpi_microvm_acpi_erst(void)
g_free(tmp_path);
free_test_data(&data);
}
+#endif /* CONFIG_POSIX */
static void test_acpi_virt_tcg(void)
{
@@ -1551,6 +1553,7 @@ static void test_acpi_q35_viot(void)
free_test_data(&data);
}
+#ifdef CONFIG_POSIX
static void test_acpi_q35_cxl(void)
{
gchar *tmp_path = g_dir_make_tmp("qemu-test-cxl.XX", NULL);
@@ -1593,6 +1596,7 @@ static void test_acpi_q35_cxl(void)
g_free(tmp_path);
free_test_data(&data);
}
+#endif /* CONFIG_POSIX */
static void test_acpi_virt_viot(void)
{
@@ -1805,8 +1809,10 @@ int main(int argc, char *argv[])
qtest_add_func("acpi/q35/dimmpxm", test_acpi_q35_tcg_dimm_pxm);
qtest_add_func("acpi/piix4/acpihmat", test_acpi_piix4_tcg_acpi_hmat);
qtest_add_func("acpi/q35/acpihmat", test_acpi_q35_tcg_acpi_hmat);
+#ifdef CONFIG_POSIX
qtest_add_func("acpi/piix4/acpierst", test_acpi_piix4_acpi_erst);
qtest_add_func("acpi/q35/acpierst", test_acpi_q35_acpi_erst);
+#endif
qtest_add_func("acpi/q35/applesmc", test_acpi_q35_applesmc);
qtest_add_func("acpi/q35/pvpanic-isa", test_acpi_q35_pvpanic_isa);
qtest_add_func("acpi/microvm", test_acpi_microvm_tcg);
@@ -1818,7 +1824,9 @@ int main(int argc, char *argv[])
qtest_add_func("acpi/q35/ivrs", test_acpi_q35_tcg_ivrs);
if (strcmp(arch, "x86_64") == 0) {
qtest_add_func("acpi/microvm/pcie",
test_acpi_microvm_pcie_tcg);
+#ifdef CONFIG_POSIX
qtest_add_func("acpi/microvm/acpierst",
test_acpi_microvm_acpi_erst);
+#endif
}
}
if (has_kvm) {
@@ -1826,7 +1834,9 @@ int main(int argc, char *argv[])
qtest_add_func("acpi/q35/kvm/dmar", test_acpi_q35_kvm_dmar);
}
qtest_add_func("acpi/q35/viot", test_acpi_q35_viot);
+#ifdef CONFIG_POSIX
qtest_add_func("acpi/q35/cxl", test_acpi_q35_cxl);
+#endif
qtest_add_func("acpi/q35/slic", test_acpi_q35_slic);
} else if (strcmp(arch, "aarch64") == 0) {
if (has_tcg) {
diff --git a/tests/qtest/cxl-test.c b/tests/qtest/cxl-test.c
index b3733cdb5f..4b4e7e5088 100644
--- a/tests/qtest/cxl-test.c
+++ b/tests/qtest/cxl-test.c
@@ -89,6 +89,7 @@ static void cxl_2root_port(void)
qtest_end();
}
+#ifdef CONFIG_POSIX
static void cxl_t3d(void)
{
g_autoptr(GString) cmdline = g_string_new(NULL);
@@ -136,6 +137,7 @@ static void cxl_2pxb_4rp_4t3d(void)
qtest_start(cmdline->str);
qtest_end();
}
+#endif /* CONFIG_POSIX */
int main(int argc, char **argv)
{
@@ -147,8 +149,10 @@ int main(int argc, char **argv)
qtest_add_func("/pci/cxl/pxb_x2_with_window", cxl_2pxb_with_window);
qtest_add_func("/pci/cxl/rp", cxl_root_port);
qtest_add_func("/pci/cxl/rp_x2", cxl_2root_port);
+#ifdef CONFIG_POSIX
qtest_add_func("/pci/cxl/type3_device", cxl_t3d);
qtest_add_func("/pci/cxl/rp_x2_type3_x2", cxl_1pxb_2rp_2t3d);
qtest_add_func("/pci/cxl/pxb_x2_root_port_x4_type3_x4", cxl_2pxb_4rp_4t3d);
+#endif
return g_test_run();
}
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 72bb9e21f3..9e484e60ba 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -71,7 +71,8 @@ qtests_i386 = \
(config_all_devices.has_key('CONFIG_SB16') ? ['fuzz-sb16-test'] : []) +
\
(config_all_devices.has_key('CONFIG_SDHCI_PCI') ? ['fuzz-sdcard-test'] : [])
+\
(config_all_devices.has_key('CONFIG_ESP_PCI') ? ['am53c974-test'] : []) +
\
- (config_all_devices.has_key('CONFIG_ACPI_ERST') ? ['erst-test'] : []) +
\
+ (config_host.has_key('CONFIG_POSIX') and
\
+ config_all_devices.has_key('CONFIG_ACPI_ERST') ? ['erst-test'] : []) +
\
(config_all_devices.has_key('CONFIG_VIRTIO_NET') and
\
config_all_devices.has_key('CONFIG_Q35') and
\
config_all_devices.has_key('CONFIG_VIRTIO_PCI') and
[PATCH 06/51] util/qemu-sockets: Use g_get_tmp_dir() to get the directory for temporary files
From: Bin Meng
Replace the existing logic to get the directory for temporary files
with g_get_tmp_dir(), which works for win32 too.
Signed-off-by: Bin Meng
---
util/qemu-sockets.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 83f4bd6fd2..0c41ca9e42 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -919,9 +919,8 @@ static int unix_listen_saddr(UnixSocketAddress *saddr,
if (saddr->path[0] || abstract) {
path = saddr->path;
} else {
-const char *tmpdir = getenv("TMPDIR");
-tmpdir = tmpdir ? tmpdir : "/tmp";
-path = pathbuf = g_strdup_printf("%s/qemu-socket-XX", tmpdir);
+path = pathbuf = g_strdup_printf("%s/qemu-socket-XX",
+ g_get_tmp_dir());
}
pathlen = strlen(path);
--
2.34.1
[PATCH 15/51] tests/qtest: Adapt {m48t59,rtc}-test cases for win32
From: Bin Meng There is no tm_gmtoff member in 'struct tm' on Windows. Update rtc-test.c and m48t59-test.c accordingly. Signed-off-by: Bin Meng --- tests/qtest/m48t59-test.c | 2 +- tests/qtest/rtc-test.c| 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/qtest/m48t59-test.c b/tests/qtest/m48t59-test.c index b94a1230f7..843d2ced8e 100644 --- a/tests/qtest/m48t59-test.c +++ b/tests/qtest/m48t59-test.c @@ -137,7 +137,7 @@ static void cmos_get_date_time(QTestState *s, struct tm *date) date->tm_mday = mday; date->tm_mon = mon - 1; date->tm_year = base_year + year - 1900; -#ifndef __sun__ +#if !defined(__sun__) && !defined(_WIN32) date->tm_gmtoff = 0; #endif diff --git a/tests/qtest/rtc-test.c b/tests/qtest/rtc-test.c index 8126ab1bdb..02ed4e1238 100644 --- a/tests/qtest/rtc-test.c +++ b/tests/qtest/rtc-test.c @@ -111,7 +111,7 @@ static void cmos_get_date_time(struct tm *date) date->tm_mday = mday; date->tm_mon = mon - 1; date->tm_year = base_year + year - 1900; -#ifndef __sun__ +#if !defined(__sun__) && !defined(_WIN32) date->tm_gmtoff = 0; #endif -- 2.34.1
[PATCH 02/51] tests/qtest: Use g_mkdtemp()
From: Bin Meng
Windows does not provide a mkdtemp() API, but glib does.
Replace mkdtemp() call with the glib version.
Signed-off-by: Bin Meng
---
tests/qtest/fuzz/generic_fuzz_configs.h | 2 +-
tests/qtest/cdrom-test.c| 2 +-
tests/qtest/cxl-test.c | 6 +++---
tests/qtest/ivshmem-test.c | 4 ++--
tests/qtest/libqos/virtio-9p.c | 4 ++--
tests/qtest/libqtest.c | 2 +-
tests/qtest/migration-test.c| 4 ++--
tests/qtest/qmp-test.c | 4 ++--
tests/qtest/vhost-user-test.c | 4 ++--
tests/unit/test-qga.c | 2 +-
10 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/tests/qtest/fuzz/generic_fuzz_configs.h
b/tests/qtest/fuzz/generic_fuzz_configs.h
index 004c701915..0775e6702b 100644
--- a/tests/qtest/fuzz/generic_fuzz_configs.h
+++ b/tests/qtest/fuzz/generic_fuzz_configs.h
@@ -21,7 +21,7 @@ typedef struct generic_fuzz_config {
static inline gchar *generic_fuzzer_virtio_9p_args(void){
char tmpdir[] = "/tmp/qemu-fuzz.XX";
-g_assert_nonnull(mkdtemp(tmpdir));
+g_assert_nonnull(g_mkdtemp(tmpdir));
return g_strdup_printf("-machine q35 -nodefaults "
"-device virtio-9p,fsdev=hshare,mount_tag=hshare "
diff --git a/tests/qtest/cdrom-test.c b/tests/qtest/cdrom-test.c
index a7766a9e65..26a2400181 100644
--- a/tests/qtest/cdrom-test.c
+++ b/tests/qtest/cdrom-test.c
@@ -52,7 +52,7 @@ static int prepare_image(const char *arch, char *isoimage)
perror("Error creating temporary iso image file");
return -1;
}
-if (!mkdtemp(srcdir)) {
+if (!g_mkdtemp(srcdir)) {
perror("Error creating temporary directory");
goto cleanup;
}
diff --git a/tests/qtest/cxl-test.c b/tests/qtest/cxl-test.c
index 2133e973f4..4e6d285061 100644
--- a/tests/qtest/cxl-test.c
+++ b/tests/qtest/cxl-test.c
@@ -95,7 +95,7 @@ static void cxl_t3d(void)
char template[] = "/tmp/cxl-test-XX";
const char *tmpfs;
-tmpfs = mkdtemp(template);
+tmpfs = g_mkdtemp(template);
g_string_printf(cmdline, QEMU_PXB_CMD QEMU_RP QEMU_T3D, tmpfs, tmpfs);
@@ -109,7 +109,7 @@ static void cxl_1pxb_2rp_2t3d(void)
char template[] = "/tmp/cxl-test-XX";
const char *tmpfs;
-tmpfs = mkdtemp(template);
+tmpfs = g_mkdtemp(template);
g_string_printf(cmdline, QEMU_PXB_CMD QEMU_2RP QEMU_2T3D,
tmpfs, tmpfs, tmpfs, tmpfs);
@@ -124,7 +124,7 @@ static void cxl_2pxb_4rp_4t3d(void)
char template[] = "/tmp/cxl-test-XX";
const char *tmpfs;
-tmpfs = mkdtemp(template);
+tmpfs = g_mkdtemp(template);
g_string_printf(cmdline, QEMU_2PXB_CMD QEMU_4RP QEMU_4T3D,
tmpfs, tmpfs, tmpfs, tmpfs, tmpfs, tmpfs,
diff --git a/tests/qtest/ivshmem-test.c b/tests/qtest/ivshmem-test.c
index e23a97fa8e..9611d05eb5 100644
--- a/tests/qtest/ivshmem-test.c
+++ b/tests/qtest/ivshmem-test.c
@@ -481,8 +481,8 @@ int main(int argc, char **argv)
tmpshmem = mmap(0, TMPSHMSIZE, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0);
g_assert(tmpshmem != MAP_FAILED);
/* server */
-if (mkdtemp(dir) == NULL) {
-g_error("mkdtemp: %s", g_strerror(errno));
+if (g_mkdtemp(dir) == NULL) {
+g_error("g_mkdtemp: %s", g_strerror(errno));
}
tmpdir = dir;
tmpserver = g_strconcat(tmpdir, "/server", NULL);
diff --git a/tests/qtest/libqos/virtio-9p.c b/tests/qtest/libqos/virtio-9p.c
index 70aea8bf62..ae9b0a20e2 100644
--- a/tests/qtest/libqos/virtio-9p.c
+++ b/tests/qtest/libqos/virtio-9p.c
@@ -48,9 +48,9 @@ void virtio_9p_create_local_test_dir(void)
*/
char *template = concat_path(pwd, "qtest-9p-local-XX");
-local_test_path = mkdtemp(template);
+local_test_path = g_mkdtemp(template);
if (!local_test_path) {
-g_test_message("mkdtemp('%s') failed: %s", template, strerror(errno));
+g_test_message("g_mkdtemp('%s') failed: %s", template,
strerror(errno));
}
g_assert(local_test_path != NULL);
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index ad6860d774..7c9fc07de4 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -393,7 +393,7 @@ QTestState *qtest_init_with_serial(const char *extra_args,
int *sock_fd)
char *sock_path, sock_dir[] = "/tmp/qtest-serial-XX";
QTestState *qts;
-g_assert_true(mkdtemp(sock_dir) != NULL);
+g_assert_true(g_mkdtemp(sock_dir) != NULL);
sock_path = g_strdup_printf("%s/sock", sock_dir);
sock_fd_init = init_socket(sock_path);
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 520a5f917c..52988b86eb 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -2450,9 +2450,9 @@ int main(int argc, char **argv)
return g_test_run();
}
-tmpfs = mkdtemp(template);
+tmpfs = g_mkdtemp(template);
if (!tmpfs) {
-g_test_message("mkdtemp on path
[PATCH 16/51] tests/qtest: Build e1000e-test for posix only
From: Bin Meng
The whole e1000e-test test case relies on socketpair() which does
not exist on win32.
Signed-off-by: Bin Meng
---
tests/qtest/meson.build | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index be4b30dea2..9d0f82bf1c 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -239,7 +239,6 @@ qos_test_ss.add(
'adm1272-test.c',
'ds1338-test.c',
'e1000-test.c',
- 'e1000e-test.c',
'eepro100-test.c',
'es1370-test.c',
'ipoctal232-test.c',
@@ -267,6 +266,9 @@ qos_test_ss.add(
'virtio-iommu-test.c',
'vmxnet3-test.c',
)
+if config_host.has_key('CONFIG_POSIX')
+ qos_test_ss.add(files('e1000e-test.c'))
+endif
if have_virtfs
qos_test_ss.add(files('virtio-9p-test.c'))
endif
--
2.34.1
[PATCH 32/51] tests/qtest: Fix ERROR_SHARING_VIOLATION for win32
From: Bin Meng
On Windows, the MinGW provided mkstemp() API opens the file with
exclusive access, denying other processes to read/write the file.
Such behavior prevents the QEMU executable from opening the file,
(e.g.: CreateFile returns ERROR_SHARING_VIOLATION).
This can be fixed by closing the file and reopening it.
Signed-off-by: Bin Meng
---
tests/qtest/ahci-test.c| 14 ++
tests/qtest/boot-serial-test.c | 13 +
2 files changed, 27 insertions(+)
diff --git a/tests/qtest/ahci-test.c b/tests/qtest/ahci-test.c
index f26cd6f86f..0e88cd0eef 100644
--- a/tests/qtest/ahci-test.c
+++ b/tests/qtest/ahci-test.c
@@ -1443,6 +1443,20 @@ static int prepare_iso(size_t size, unsigned char **buf,
char **name)
int fd = mkstemp(cdrom_path);
g_assert(fd != -1);
+#ifdef _WIN32
+/*
+ * On Windows, the MinGW provided mkstemp() API opens the file with
+ * exclusive access, denying other processes to read/write the file.
+ * Such behavior prevents the QEMU executable from opening the file,
+ * (e.g.: CreateFile returns ERROR_SHARING_VIOLATION).
+ *
+ * Close the file and reopen it.
+ */
+close(fd);
+fd = open(cdrom_path, O_WRONLY);
+g_assert(fd != -1);
+#endif
+
g_assert(buf);
g_assert(name);
patt = g_malloc(size);
diff --git a/tests/qtest/boot-serial-test.c b/tests/qtest/boot-serial-test.c
index 404adcfa20..fb6c81bf35 100644
--- a/tests/qtest/boot-serial-test.c
+++ b/tests/qtest/boot-serial-test.c
@@ -235,6 +235,19 @@ static void test_machine(const void *data)
ser_fd = mkstemp(serialtmp);
g_assert(ser_fd != -1);
+#ifdef _WIN32
+/*
+ * On Windows, the MinGW provided mkstemp() API opens the file with
+ * exclusive access, denying other processes to read/write the file.
+ * Such behavior prevents the QEMU executable from opening the file,
+ * (e.g.: CreateFile returns ERROR_SHARING_VIOLATION).
+ *
+ * Close the file and reopen it.
+ */
+close(ser_fd);
+ser_fd = open(serialtmp, O_RDONLY);
+g_assert(ser_fd != -1);
+#endif
if (test->kernel) {
code = test->kernel;
--
2.34.1
[PATCH 09/51] fsdev/virtfs-proxy-helper: Use g_mkdir_with_parents()
From: Bin Meng
Use the same g_mkdir_with_parents() call to create a directory on
all platforms.
Signed-off-by: Bin Meng
---
fsdev/virtfs-proxy-helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fsdev/virtfs-proxy-helper.c b/fsdev/virtfs-proxy-helper.c
index 2dde27922f..d0cf76d6d1 100644
--- a/fsdev/virtfs-proxy-helper.c
+++ b/fsdev/virtfs-proxy-helper.c
@@ -639,7 +639,7 @@ static int do_create_others(int type, struct iovec *iovec)
if (retval < 0) {
goto err_out;
}
-retval = mkdir(path.data, mode);
+retval = g_mkdir_with_parents(path.data, mode);
break;
case T_SYMLINK:
retval = proxy_unmarshal(iovec, offset, "ss", &oldpath, &path);
--
2.34.1
[PATCH 05/51] tcg: Avoid using hardcoded /tmp
From: Bin Meng
Use g_get_tmp_dir() to get the directory to use for temporary files.
Signed-off-by: Bin Meng
---
tcg/tcg.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tcg/tcg.c b/tcg/tcg.c
index 0f9cfe96f2..932156a352 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -4729,13 +4729,15 @@ static void tcg_register_jit_int(const void *buf_ptr,
size_t buf_size,
/* Enable this block to be able to debug the ELF image file creation.
One can use readelf, objdump, or other inspection utilities. */
{
-FILE *f = fopen("/tmp/qemu.jit", "w+b");
+char *jit = g_strdup_printf("%s/qemu.jit", g_get_tmp_dir());
+FILE *f = fopen(jit, "w+b");
if (f) {
if (fwrite(img, img_size, 1, f) != img_size) {
/* Avoid stupid unused return value warning for fwrite. */
}
fclose(f);
}
+g_free(jit);
}
#endif
--
2.34.1
[PATCH 19/51] tests/qtest: Build test-filter-{mirror, redirector} cases for posix only
From: Bin Meng
The test-filter-{mirror,redirector} cases use socketpair() API that
is only available on POSIX and should only be built for POSIX.
Signed-off-by: Bin Meng
---
tests/qtest/meson.build | 28 ++--
1 file changed, 18 insertions(+), 10 deletions(-)
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 9e484e60ba..c97da5a062 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -42,6 +42,7 @@ qtests_cxl = \
qtests_i386 = \
(slirp.found() ? ['pxe-test', 'test-netfilter'] : []) + \
(config_host.has_key('CONFIG_POSIX') ? ['test-filter-mirror'] : []) +
\
+ (config_host.has_key('CONFIG_POSIX') ? ['test-filter-redirector'] : []) +
\
(have_tools ? ['ahci-test'] : []) +
\
(config_all_devices.has_key('CONFIG_ISA_TESTDEV') ? ['endianness-test'] :
[]) + \
(config_all_devices.has_key('CONFIG_SGA') ? ['boot-serial-test'] : []) +
\
@@ -95,8 +96,7 @@ qtests_i386 = \
'vmgenid-test',
'migration-test',
'test-x86-cpuid-compat',
- 'numa-test',
- 'test-filter-redirector'
+ 'numa-test'
]
if dbus_display
@@ -120,29 +120,34 @@ endif
qtests_x86_64 = qtests_i386
qtests_alpha = ['boot-serial-test'] + \
- ['test-filter-mirror', 'test-filter-redirector'] + \
+ (config_host.has_key('CONFIG_POSIX') ? ['test-filter-mirror'] : []) + \
+ (config_host.has_key('CONFIG_POSIX') ? ['test-filter-redirector'] : []) + \
(slirp.found() ? ['test-netfilter'] : []) + \
(config_all_devices.has_key('CONFIG_VGA') ? ['display-vga-test'] : [])
qtests_avr = [ 'boot-serial-test' ]
qtests_hppa = ['boot-serial-test'] + \
- ['test-filter-mirror', 'test-filter-redirector'] + \
+ (config_host.has_key('CONFIG_POSIX') ? ['test-filter-mirror'] : []) + \
+ (config_host.has_key('CONFIG_POSIX') ? ['test-filter-redirector'] : []) + \
(slirp.found() ? ['test-netfilter'] : []) + \
(config_all_devices.has_key('CONFIG_VGA') ? ['display-vga-test'] : [])
qtests_m68k = ['boot-serial-test'] + \
- ['test-filter-mirror', 'test-filter-redirector'] + \
+ (config_host.has_key('CONFIG_POSIX') ? ['test-filter-mirror'] : []) + \
+ (config_host.has_key('CONFIG_POSIX') ? ['test-filter-redirector'] : []) + \
(slirp.found() ? ['test-netfilter'] : [])
qtests_microblaze = ['boot-serial-test'] + \
- ['test-filter-mirror', 'test-filter-redirector'] + \
+ (config_host.has_key('CONFIG_POSIX') ? ['test-filter-mirror'] : []) + \
+ (config_host.has_key('CONFIG_POSIX') ? ['test-filter-redirector'] : []) + \
(slirp.found() ? ['test-netfilter'] : [])
qtests_microblazeel = qtests_microblaze
qtests_mips = \
- ['test-filter-mirror', 'test-filter-redirector'] + \
+ (config_host.has_key('CONFIG_POSIX') ? ['test-filter-mirror'] : []) + \
+ (config_host.has_key('CONFIG_POSIX') ? ['test-filter-redirector'] : []) + \
(slirp.found() ? ['test-netfilter'] : []) + \
(config_all_devices.has_key('CONFIG_ISA_TESTDEV') ? ['endianness-test'] :
[]) +\
(config_all_devices.has_key('CONFIG_VGA') ? ['display-vga-test'] : [])
@@ -152,7 +157,8 @@ qtests_mips64 = qtests_mips
qtests_mips64el = qtests_mips
qtests_ppc = \
- ['test-filter-mirror', 'test-filter-redirector'] + \
+ (config_host.has_key('CONFIG_POSIX') ? ['test-filter-mirror'] : []) + \
+ (config_host.has_key('CONFIG_POSIX') ? ['test-filter-redirector'] : []) + \
(slirp.found() ? ['test-netfilter'] : []) + \
(config_all_devices.has_key('CONFIG_ISA_TESTDEV') ? ['endianness-test'] :
[]) +\
(config_all_devices.has_key('CONFIG_M48T59') ? ['m48t59-test'] : []) +
\
@@ -174,13 +180,15 @@ qtests_sh4 =
(config_all_devices.has_key('CONFIG_ISA_TESTDEV') ? ['endianness-te
qtests_sh4eb = (config_all_devices.has_key('CONFIG_ISA_TESTDEV') ?
['endianness-test'] : [])
qtests_sparc = ['prom-env-test', 'm48t59-test', 'boot-serial-test'] + \
- ['test-filter-mirror', 'test-filter-redirector'] + \
+ (config_host.has_key('CONFIG_POSIX') ? ['test-filter-mirror'] : []) + \
+ (config_host.has_key('CONFIG_POSIX') ? ['test-filter-redirector'] : []) + \
(slirp.found() ? ['test-netfilter'] : [])
qtests_sparc64 = \
(config_all_devices.has_key('CONFIG_ISA_TESTDEV') ? ['endianness-test'] :
[]) +\
(slirp.found() ? ['test-netfilter'] : []) + \
- ['test-filter-mirror', 'test-filter-redirector'] + \
+ (config_host.has_key('CONFIG_POSIX') ? ['test-filter-mirror'] : []) + \
+ (config_host.has_key('CONFIG_POSIX') ? ['test-filter-redirector'] : []) + \
['prom-env-test', 'boot-serial-test']
qtests_npcm7xx = \
--
2.34.1
[PATCH 12/51] tests: Use g_mkdir_with_parents()
From: Bin Meng
Use the same g_mkdir_with_parents() call to create a directory on
all platforms.
Signed-off-by: Bin Meng
---
tests/migration/stress.c | 2 +-
tests/qtest/migration-test.c | 6 +++---
tests/unit/test-crypto-tlscredsx509.c | 4 ++--
tests/unit/test-crypto-tlssession.c | 6 +++---
tests/unit/test-io-channel-tls.c | 6 +++---
5 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/tests/migration/stress.c b/tests/migration/stress.c
index b7240a15c8..88acf8dc25 100644
--- a/tests/migration/stress.c
+++ b/tests/migration/stress.c
@@ -232,7 +232,7 @@ static void stress(unsigned long long ramsizeGB, int ncpus)
static int mount_misc(const char *fstype, const char *dir)
{
-if (mkdir(dir, 0755) < 0 && errno != EEXIST) {
+if (g_mkdir_with_parents(dir, 0755) < 0 && errno != EEXIST) {
fprintf(stderr, "%s (%05d): ERROR: cannot create %s: %s\n",
argv0, gettid(), dir, strerror(errno));
return -1;
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 5fa4ffeff0..38356d4aba 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -761,14 +761,14 @@ test_migrate_tls_psk_start_common(QTestState *from,
data->workdir = g_strdup_printf("%s/tlscredspsk0", tmpfs);
data->pskfile = g_strdup_printf("%s/%s", data->workdir,
QCRYPTO_TLS_CREDS_PSKFILE);
-mkdir(data->workdir, 0700);
+g_mkdir_with_parents(data->workdir, 0700);
test_tls_psk_init(data->pskfile);
if (mismatch) {
data->workdiralt = g_strdup_printf("%s/tlscredspskalt0", tmpfs);
data->pskfilealt = g_strdup_printf("%s/%s", data->workdiralt,
QCRYPTO_TLS_CREDS_PSKFILE);
-mkdir(data->workdiralt, 0700);
+g_mkdir_with_parents(data->workdiralt, 0700);
test_tls_psk_init_alt(data->pskfilealt);
}
@@ -873,7 +873,7 @@ test_migrate_tls_x509_start_common(QTestState *from,
data->clientcert = g_strdup_printf("%s/client-cert.pem",
data->workdir);
}
-mkdir(data->workdir, 0700);
+g_mkdir_with_parents(data->workdir, 0700);
test_tls_init(data->keyfile);
g_assert(link(data->keyfile, data->serverkey) == 0);
diff --git a/tests/unit/test-crypto-tlscredsx509.c
b/tests/unit/test-crypto-tlscredsx509.c
index aab4149b56..3c25d75ca1 100644
--- a/tests/unit/test-crypto-tlscredsx509.c
+++ b/tests/unit/test-crypto-tlscredsx509.c
@@ -75,7 +75,7 @@ static void test_tls_creds(const void *opaque)
QCryptoTLSCreds *creds;
#define CERT_DIR "tests/test-crypto-tlscredsx509-certs/"
-mkdir(CERT_DIR, 0700);
+g_mkdir_with_parents(CERT_DIR, 0700);
unlink(CERT_DIR QCRYPTO_TLS_CREDS_X509_CA_CERT);
if (data->isServer) {
@@ -141,7 +141,7 @@ int main(int argc, char **argv)
g_test_init(&argc, &argv, NULL);
g_setenv("GNUTLS_FORCE_FIPS_MODE", "2", 1);
-mkdir(WORKDIR, 0700);
+g_mkdir_with_parents(WORKDIR, 0700);
test_tls_init(KEYFILE);
diff --git a/tests/unit/test-crypto-tlssession.c
b/tests/unit/test-crypto-tlssession.c
index f222959d36..615a1344b4 100644
--- a/tests/unit/test-crypto-tlssession.c
+++ b/tests/unit/test-crypto-tlssession.c
@@ -249,8 +249,8 @@ static void test_crypto_tls_session_x509(const void *opaque)
#define CLIENT_CERT_DIR "tests/test-crypto-tlssession-client/"
#define SERVER_CERT_DIR "tests/test-crypto-tlssession-server/"
-mkdir(CLIENT_CERT_DIR, 0700);
-mkdir(SERVER_CERT_DIR, 0700);
+g_mkdir_with_parents(CLIENT_CERT_DIR, 0700);
+g_mkdir_with_parents(SERVER_CERT_DIR, 0700);
unlink(SERVER_CERT_DIR QCRYPTO_TLS_CREDS_X509_CA_CERT);
unlink(SERVER_CERT_DIR QCRYPTO_TLS_CREDS_X509_SERVER_CERT);
@@ -398,7 +398,7 @@ int main(int argc, char **argv)
g_test_init(&argc, &argv, NULL);
g_setenv("GNUTLS_FORCE_FIPS_MODE", "2", 1);
-mkdir(WORKDIR, 0700);
+g_mkdir_with_parents(WORKDIR, 0700);
test_tls_init(KEYFILE);
test_tls_psk_init(PSKFILE);
diff --git a/tests/unit/test-io-channel-tls.c b/tests/unit/test-io-channel-tls.c
index f6fb988c01..cc39247556 100644
--- a/tests/unit/test-io-channel-tls.c
+++ b/tests/unit/test-io-channel-tls.c
@@ -125,8 +125,8 @@ static void test_io_channel_tls(const void *opaque)
#define CLIENT_CERT_DIR "tests/test-io-channel-tls-client/"
#define SERVER_CERT_DIR "tests/test-io-channel-tls-server/"
-mkdir(CLIENT_CERT_DIR, 0700);
-mkdir(SERVER_CERT_DIR, 0700);
+g_mkdir_with_parents(CLIENT_CERT_DIR, 0700);
+g_mkdir_with_parents(SERVER_CERT_DIR, 0700);
unlink(SERVER_CERT_DIR QCRYPTO_TLS_CREDS_X509_CA_CERT);
unlink(SERVER_CERT_DIR QCRYPTO_TLS_CREDS_X509_SERVER_CERT);
@@ -273,7 +273,7 @@ int main(int argc, char **argv)
g_test_init(&argc, &argv, NULL);
g_setenv("GNUTLS_FORCE_FIPS_MODE", "2", 1);
-mkdir(WORKDIR, 0700);
+g_mkdir_with_parents(WORKDIR, 0700);
test_tls_init(KEYFILE);
--
2.34.1
[PATCH 36/51] tests/qtest: machine-none-test: Use double quotes to pass the cpu option
From: Bin Meng
Single quotes in the arguments (e.g.: -cpu 'qemu64,apic-id=0') are
not removed in the Windows environment before it is passed to the
QEMU executable. Such argument causes a failure in the QEMU CPU
option parser codes.
Change to use double quotes which works fine on all platforms.
Signed-off-by: Bin Meng
---
tests/qtest/machine-none-test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/qtest/machine-none-test.c b/tests/qtest/machine-none-test.c
index f92fab479f..31cc0bfb01 100644
--- a/tests/qtest/machine-none-test.c
+++ b/tests/qtest/machine-none-test.c
@@ -81,7 +81,7 @@ static void test_machine_cpu_cli(void)
" add it to cpus_map\n", arch);
return; /* TODO: die here to force all targets have a test */
}
-qts = qtest_initf("-machine none -cpu '%s'", cpu_model);
+qts = qtest_initf("-machine none -cpu \"%s\"", cpu_model);
response = qtest_qmp(qts, "{ 'execute': 'quit' }");
g_assert(qdict_haskey(response, "return"));
--
2.34.1
[PATCH 07/51] tests: Avoid using hardcoded /tmp in test cases
From: Bin Meng
Use g_get_tmp_dir() to get the directory to use for temporary files.
Signed-off-by: Bin Meng
---
tests/qtest/fuzz/generic_fuzz_configs.h | 6 --
tests/qtest/ahci-test.c | 15 +++
tests/qtest/aspeed_smc-test.c | 4 +++-
tests/qtest/boot-serial-test.c | 8 ++--
tests/qtest/cxl-test.c | 9 ++---
tests/qtest/fdc-test.c | 4 +++-
tests/qtest/fuzz/virtio_blk_fuzz.c | 2 +-
tests/qtest/hd-geo-test.c | 8
tests/qtest/ide-test.c | 8 ++--
tests/qtest/libqtest.c | 10 +++---
tests/qtest/migration-test.c| 4 +++-
tests/qtest/pflash-cfi02-test.c | 7 +--
tests/qtest/qmp-test.c | 4 +++-
tests/qtest/vhost-user-blk-test.c | 3 ++-
tests/qtest/vhost-user-test.c | 3 ++-
tests/qtest/virtio-blk-test.c | 2 +-
tests/qtest/virtio-scsi-test.c | 3 ++-
tests/unit/test-image-locking.c | 6 --
tests/unit/test-qga.c | 2 +-
tests/vhost-user-bridge.c | 3 ++-
20 files changed, 76 insertions(+), 35 deletions(-)
diff --git a/tests/qtest/fuzz/generic_fuzz_configs.h
b/tests/qtest/fuzz/generic_fuzz_configs.h
index 0775e6702b..d0f9961187 100644
--- a/tests/qtest/fuzz/generic_fuzz_configs.h
+++ b/tests/qtest/fuzz/generic_fuzz_configs.h
@@ -20,13 +20,15 @@ typedef struct generic_fuzz_config {
} generic_fuzz_config;
static inline gchar *generic_fuzzer_virtio_9p_args(void){
-char tmpdir[] = "/tmp/qemu-fuzz.XX";
+char *tmpdir = g_strdup_printf("%s/qemu-fuzz.XX", g_get_tmp_dir());
g_assert_nonnull(g_mkdtemp(tmpdir));
-return g_strdup_printf("-machine q35 -nodefaults "
+gchar *args = g_strdup_printf("-machine q35 -nodefaults "
"-device virtio-9p,fsdev=hshare,mount_tag=hshare "
"-fsdev local,id=hshare,path=%s,security_model=mapped-xattr,"
"writeout=immediate,fmode=0600,dmode=0700", tmpdir);
+g_free(tmpdir);
+return args;
}
const generic_fuzz_config predefined_configs[] = {
diff --git a/tests/qtest/ahci-test.c b/tests/qtest/ahci-test.c
index f1e510b0ac..f26cd6f86f 100644
--- a/tests/qtest/ahci-test.c
+++ b/tests/qtest/ahci-test.c
@@ -44,9 +44,9 @@
#define TEST_IMAGE_SIZE_MB_SMALL 64
/*** Globals ***/
-static char tmp_path[] = "/tmp/qtest.XX";
-static char debug_path[] = "/tmp/qtest-blkdebug.XX";
-static char mig_socket[] = "/tmp/qtest-migration.XX";
+static char *tmp_path;
+static char *debug_path;
+static char *mig_socket;
static bool ahci_pedantic;
static const char *imgfmt;
static unsigned test_image_size_mb;
@@ -1437,7 +1437,7 @@ static void test_ncq_simple(void)
static int prepare_iso(size_t size, unsigned char **buf, char **name)
{
-char cdrom_path[] = "/tmp/qtest.iso.XX";
+char *cdrom_path = g_strdup_printf("%s/qtest.iso.XX", g_get_tmp_dir());
unsigned char *patt;
ssize_t ret;
int fd = mkstemp(cdrom_path);
@@ -1454,6 +1454,7 @@ static int prepare_iso(size_t size, unsigned char **buf,
char **name)
*name = g_strdup(cdrom_path);
*buf = patt;
+g_free(cdrom_path);
return fd;
}
@@ -1872,6 +1873,7 @@ int main(int argc, char **argv)
}
/* Create a temporary image */
+tmp_path = g_strdup_printf("%s/qtest.XX", g_get_tmp_dir());
fd = mkstemp(tmp_path);
g_assert(fd >= 0);
if (have_qemu_img()) {
@@ -1889,11 +1891,13 @@ int main(int argc, char **argv)
close(fd);
/* Create temporary blkdebug instructions */
+debug_path = g_strdup_printf("%s/qtest-blkdebug.XX", g_get_tmp_dir());
fd = mkstemp(debug_path);
g_assert(fd >= 0);
close(fd);
/* Reserve a hollow file to use as a socket for migration tests */
+mig_socket = g_strdup_printf("%s/qtest-migration.XX", g_get_tmp_dir());
fd = mkstemp(mig_socket);
g_assert(fd >= 0);
close(fd);
@@ -1947,8 +1951,11 @@ int main(int argc, char **argv)
/* Cleanup */
unlink(tmp_path);
+g_free(tmp_path);
unlink(debug_path);
+g_free(debug_path);
unlink(mig_socket);
+g_free(mig_socket);
return ret;
}
diff --git a/tests/qtest/aspeed_smc-test.c b/tests/qtest/aspeed_smc-test.c
index 05ce941566..cab769459c 100644
--- a/tests/qtest/aspeed_smc-test.c
+++ b/tests/qtest/aspeed_smc-test.c
@@ -608,7 +608,7 @@ static void test_write_block_protect_bottom_bit(void)
flash_reset();
}
-static char tmp_path[] = "/tmp/qtest.m25p80.XX";
+static char *tmp_path;
int main(int argc, char **argv)
{
@@ -617,6 +617,7 @@ int main(int argc, char **argv)
g_test_init(&argc, &argv, NULL);
+tmp_path = g_strdup_printf("%s/qtest.m25p80.XX", g_get_tmp_dir());
fd = mkstemp(tmp_path);
g_assert(fd >= 0);
ret = ftruncate(fd, FLASH_SIZE);
@@ -646,5 +647,6 @@ int main(int argc, char **argv)
qtest_quit(global_qtest);
u
[PATCH 20/51] tests/qtest: i440fx-test: Skip running request_{bios, pflash} for win32
From: Bin Meng
The request_{bios,pflash} test cases call mmap() which does not
exist on win32. Exclude them.
Signed-off-by: Bin Meng
---
tests/qtest/i440fx-test.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/tests/qtest/i440fx-test.c b/tests/qtest/i440fx-test.c
index 6d7d4d8d8f..3890f1237c 100644
--- a/tests/qtest/i440fx-test.c
+++ b/tests/qtest/i440fx-test.c
@@ -278,6 +278,8 @@ static void test_i440fx_pam(gconstpointer opaque)
qtest_end();
}
+#ifndef _WIN32
+
#define BLOB_SIZE ((size_t)65536)
#define ISA_BIOS_MAXSZ ((size_t)(128 * 1024))
@@ -396,6 +398,8 @@ static void request_pflash(FirmwareTestFixture *fixture,
fixture->is_bios = false;
}
+#endif /* _WIN32 */
+
int main(int argc, char **argv)
{
TestData data;
@@ -406,8 +410,10 @@ int main(int argc, char **argv)
qtest_add_data_func("i440fx/defaults", &data, test_i440fx_defaults);
qtest_add_data_func("i440fx/pam", &data, test_i440fx_pam);
+#ifndef _WIN32
add_firmware_test("i440fx/firmware/bios", request_bios);
add_firmware_test("i440fx/firmware/pflash", request_pflash);
+#endif
return g_test_run();
}
--
2.34.1
[PATCH 13/51] tests/qtest: migration-test: Handle link() for win32
From: Bin Meng
Windows does not provide a link() API like POSIX. Instead it provides
a similar API CreateHardLink() that does the same thing, but with
different argument order and return value.
Signed-off-by: Bin Meng
---
tests/qtest/migration-test.c | 8
1 file changed, 8 insertions(+)
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 38356d4aba..af9250750b 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -876,9 +876,17 @@ test_migrate_tls_x509_start_common(QTestState *from,
g_mkdir_with_parents(data->workdir, 0700);
test_tls_init(data->keyfile);
+#ifndef _WIN32
g_assert(link(data->keyfile, data->serverkey) == 0);
+#else
+g_assert(CreateHardLink(data->serverkey, data->keyfile, NULL) != 0);
+#endif
if (args->clientcert) {
+#ifndef _WIN32
g_assert(link(data->keyfile, data->clientkey) == 0);
+#else
+g_assert(CreateHardLink(data->clientkey, data->keyfile, NULL) != 0);
+#endif
}
TLS_ROOT_REQ_SIMPLE(cacertreq, data->cacert);
--
2.34.1
[PATCH 37/51] tests/qtest: migration-test: Disable IO redirection for win32
From: Bin Meng
On Windows the QEMU executable is created via CreateProcess() and IO
redirection does not work, so we need to set MigrateStart::hide_stderr
to false to disable adding IO redirection to the command line.
Signed-off-by: Bin Meng
---
tests/qtest/migration-test.c | 39 +++-
1 file changed, 25 insertions(+), 14 deletions(-)
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 2ae7498d5d..125d48d855 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -53,6 +53,17 @@ static bool uffd_feature_thread_id;
*/
#define DIRTYLIMIT_TOLERANCE_RANGE 25 /* MB/s */
+/*
+ * On Windows the QEMU executable is created via CreateProcess() and IO
+ * redirection does not work, so we need to set MigrateStart::hide_stderr
+ * to false to disable adding IO redirection to the command line.
+ */
+#ifndef _WIN32
+# define HIDE_STDERR true
+#else
+# define HIDE_STDERR false
+#endif
+
#if defined(__linux__)
#include
#include
@@ -1186,7 +1197,7 @@ static void test_postcopy_recovery_common(MigrateCommon
*args)
g_autofree char *uri = NULL;
/* Always hide errors for postcopy recover tests since they're expected */
-args->start.hide_stderr = true;
+args->start.hide_stderr = HIDE_STDERR;
if (migrate_postcopy_prepare(&from, &to, args)) {
return;
@@ -1287,7 +1298,7 @@ static void test_postcopy_preempt_all(void)
static void test_baddest(void)
{
MigrateStart args = {
-.hide_stderr = true
+.hide_stderr = HIDE_STDERR
};
QTestState *from, *to;
@@ -1410,7 +1421,7 @@ static void test_precopy_unix_tls_x509_default_host(void)
g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
MigrateCommon args = {
.start = {
-.hide_stderr = true,
+.hide_stderr = HIDE_STDERR,
},
.connect_uri = uri,
.listen_uri = uri,
@@ -1526,7 +1537,7 @@ static void test_precopy_tcp_tls_psk_mismatch(void)
{
MigrateCommon args = {
.start = {
-.hide_stderr = true,
+.hide_stderr = HIDE_STDERR,
},
.listen_uri = "tcp:127.0.0.1:0",
.start_hook = test_migrate_tls_psk_start_mismatch,
@@ -1564,7 +1575,7 @@ static void test_precopy_tcp_tls_x509_mismatch_host(void)
{
MigrateCommon args = {
.start = {
-.hide_stderr = true,
+.hide_stderr = HIDE_STDERR,
},
.listen_uri = "tcp:127.0.0.1:0",
.start_hook = test_migrate_tls_x509_start_mismatch_host,
@@ -1590,7 +1601,7 @@ static void test_precopy_tcp_tls_x509_hostile_client(void)
{
MigrateCommon args = {
.start = {
-.hide_stderr = true,
+.hide_stderr = HIDE_STDERR,
},
.listen_uri = "tcp:127.0.0.1:0",
.start_hook = test_migrate_tls_x509_start_hostile_client,
@@ -1616,7 +1627,7 @@ static void
test_precopy_tcp_tls_x509_reject_anon_client(void)
{
MigrateCommon args = {
.start = {
-.hide_stderr = true,
+.hide_stderr = HIDE_STDERR,
},
.listen_uri = "tcp:127.0.0.1:0",
.start_hook = test_migrate_tls_x509_start_reject_anon_client,
@@ -1747,7 +1758,7 @@ static void test_validate_uuid_error(void)
MigrateStart args = {
.opts_source = "-uuid ----",
.opts_target = "-uuid ----",
-.hide_stderr = true,
+.hide_stderr = HIDE_STDERR,
};
do_test_validate_uuid(&args, true);
@@ -1757,7 +1768,7 @@ static void test_validate_uuid_src_not_set(void)
{
MigrateStart args = {
.opts_target = "-uuid ----",
-.hide_stderr = true,
+.hide_stderr = HIDE_STDERR,
};
do_test_validate_uuid(&args, false);
@@ -1767,7 +1778,7 @@ static void test_validate_uuid_dst_not_set(void)
{
MigrateStart args = {
.opts_source = "-uuid ----",
-.hide_stderr = true,
+.hide_stderr = HIDE_STDERR,
};
do_test_validate_uuid(&args, false);
@@ -1990,7 +2001,7 @@ static void test_multifd_tcp_tls_psk_mismatch(void)
{
MigrateCommon args = {
.start = {
-.hide_stderr = true,
+.hide_stderr = HIDE_STDERR,
},
.listen_uri = "defer",
.start_hook = test_migrate_multifd_tcp_tls_psk_start_mismatch,
@@ -2038,7 +2049,7 @@ static void test_multifd_tcp_tls_x509_mismatch_host(void)
*/
MigrateCommon args = {
.start = {
-.hide_stderr = true,
+.hide_stderr = HIDE_STDERR,
},
.listen_uri = "defer",
.start_hook = test_migrate_multifd_tls_x509_start_mismatch_host,
@@ -2062,7 +2073,7 @@ static void
test_multifd_tcp_tls_x509_reject_anon_client(void)
{
MigrateCommon args = {
.start = {
-.hide_std
[PATCH 08/51] block/vvfat: Unify the mkdir() call
From: Bin Meng
There is a difference in the mkdir() call for win32 and non-win32
platforms, and currently is handled in the codes with #ifdefs.
glib provides a portable g_mkdir_with_parents() API and we can use
it to unify the codes without #ifdefs.
Signed-off-by: Bin Meng
---
block/vvfat.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/block/vvfat.c b/block/vvfat.c
index d6dd919683..9c389ce5ea 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -2726,13 +2726,9 @@ static int handle_renames_and_mkdirs(BDRVVVFATState* s)
mapping_t* mapping;
int j, parent_path_len;
-#ifdef __MINGW32__
-if (mkdir(commit->path))
+if (g_mkdir_with_parents(commit->path, 0755)) {
return -5;
-#else
-if (mkdir(commit->path, 0755))
-return -5;
-#endif
+}
mapping = insert_mapping(s, commit->param.mkdir.cluster,
commit->param.mkdir.cluster + 1);
--
2.34.1
[PATCH 25/51] tests/qtest: libqos: Rename malloc.h to libqos-malloc.h
From: Xuzhou Cheng
The qtest/libqos directory is included via the "-I" option to search
for header files when building qtest. Unfortunately the malloc.h has
a name conflict with the standard libc header, leading to a build
failure on the Windows host, due to the MinGW libc stdlib.h header
file includes malloc.h and it now gets wrongly pointed to the one
in the qtest/libqos directory.
Rename "qtest/libqos/malloc.h" to "qtest/libqos/libqos-malloc.h" to
avoid the namespace pollution.
Signed-off-by: Xuzhou Cheng
Signed-off-by: Bin Meng
---
tests/qtest/libqos/generic-pcihost.h | 2 +-
tests/qtest/libqos/{malloc.h => libqos-malloc.h} | 0
tests/qtest/libqos/libqos.h | 2 +-
tests/qtest/libqos/malloc-pc.h | 2 +-
tests/qtest/libqos/malloc-spapr.h| 2 +-
tests/qtest/libqos/pci-pc.h | 2 +-
tests/qtest/libqos/pci-spapr.h | 2 +-
tests/qtest/libqos/qgraph.h | 2 +-
tests/qtest/libqos/qos_external.h| 2 +-
tests/qtest/libqos/rtas.h| 2 +-
tests/qtest/libqos/virtio.h | 2 +-
tests/qtest/e1000e-test.c| 2 +-
tests/qtest/fuzz/qos_fuzz.c | 2 +-
tests/qtest/libqos/aarch64-xlnx-zcu102-machine.c | 2 +-
tests/qtest/libqos/arm-imx25-pdk-machine.c | 2 +-
tests/qtest/libqos/arm-n800-machine.c| 2 +-
tests/qtest/libqos/arm-raspi2-machine.c | 2 +-
tests/qtest/libqos/arm-sabrelite-machine.c | 2 +-
tests/qtest/libqos/arm-smdkc210-machine.c| 2 +-
tests/qtest/libqos/arm-virt-machine.c| 2 +-
tests/qtest/libqos/arm-xilinx-zynq-a9-machine.c | 2 +-
tests/qtest/libqos/e1000e.c | 2 +-
tests/qtest/libqos/{malloc.c => libqos-malloc.c} | 2 +-
tests/qtest/libqos/qos_external.c| 2 +-
tests/qtest/libqos/virtio-mmio.c | 2 +-
tests/qtest/libqos/virtio-pci.c | 2 +-
tests/qtest/qos-test.c | 2 +-
tests/qtest/libqos/meson.build | 2 +-
28 files changed, 27 insertions(+), 27 deletions(-)
rename tests/qtest/libqos/{malloc.h => libqos-malloc.h} (100%)
rename tests/qtest/libqos/{malloc.c => libqos-malloc.c} (99%)
diff --git a/tests/qtest/libqos/generic-pcihost.h
b/tests/qtest/libqos/generic-pcihost.h
index c693c769df..6493a8712a 100644
--- a/tests/qtest/libqos/generic-pcihost.h
+++ b/tests/qtest/libqos/generic-pcihost.h
@@ -14,7 +14,7 @@
#define LIBQOS_GENERIC_PCIHOST_H
#include "pci.h"
-#include "malloc.h"
+#include "libqos-malloc.h"
#include "qgraph.h"
typedef struct QGenericPCIBus {
diff --git a/tests/qtest/libqos/malloc.h b/tests/qtest/libqos/libqos-malloc.h
similarity index 100%
rename from tests/qtest/libqos/malloc.h
rename to tests/qtest/libqos/libqos-malloc.h
diff --git a/tests/qtest/libqos/libqos.h b/tests/qtest/libqos/libqos.h
index ba7df448ca..9b4dd509f0 100644
--- a/tests/qtest/libqos/libqos.h
+++ b/tests/qtest/libqos/libqos.h
@@ -3,7 +3,7 @@
#include "../libqtest.h"
#include "pci.h"
-#include "malloc.h"
+#include "libqos-malloc.h"
typedef struct QOSState QOSState;
diff --git a/tests/qtest/libqos/malloc-pc.h b/tests/qtest/libqos/malloc-pc.h
index d8d79853c8..e531473601 100644
--- a/tests/qtest/libqos/malloc-pc.h
+++ b/tests/qtest/libqos/malloc-pc.h
@@ -13,7 +13,7 @@
#ifndef LIBQOS_MALLOC_PC_H
#define LIBQOS_MALLOC_PC_H
-#include "malloc.h"
+#include "libqos-malloc.h"
void pc_alloc_init(QGuestAllocator *s, QTestState *qts, QAllocOpts flags);
diff --git a/tests/qtest/libqos/malloc-spapr.h
b/tests/qtest/libqos/malloc-spapr.h
index f99572fd71..f544c0d611 100644
--- a/tests/qtest/libqos/malloc-spapr.h
+++ b/tests/qtest/libqos/malloc-spapr.h
@@ -8,7 +8,7 @@
#ifndef LIBQOS_MALLOC_SPAPR_H
#define LIBQOS_MALLOC_SPAPR_H
-#include "malloc.h"
+#include "libqos-malloc.h"
void spapr_alloc_init(QGuestAllocator *s, QTestState *qts, QAllocOpts flags);
diff --git a/tests/qtest/libqos/pci-pc.h b/tests/qtest/libqos/pci-pc.h
index 49ec9507f2..849bd493de 100644
--- a/tests/qtest/libqos/pci-pc.h
+++ b/tests/qtest/libqos/pci-pc.h
@@ -14,7 +14,7 @@
#define LIBQOS_PCI_PC_H
#include "pci.h"
-#include "malloc.h"
+#include "libqos-malloc.h"
#include "qgraph.h"
typedef struct QPCIBusPC {
diff --git a/tests/qtest/libqos/pci-spapr.h b/tests/qtest/libqos/pci-spapr.h
index 20a43718b7..3dbf1e58ae 100644
--- a/tests/qtest/libqos/pci-spapr.h
+++ b/tests/qtest/libqos/pci-spapr.h
@@ -8,7 +8,7 @@
#ifndef LIBQOS_PCI_SPAPR_H
#define LIBQOS_PCI_SPAPR_H
-#include "malloc.h"
+#include "libqos-malloc.h"
#include "pci.h"
#include "qgraph.h"
diff --git a/tests/qtest/libqos/qgraph.h b/tests/qtest/libqos/qgraph.h
index 871740c0dc..6e94824d09 100644
--- a/tests/qtest/libqos/qgraph.h
+++ b/tests/qtest/libqos/qgraph.h
@@ -21,7 +21,7 @@
#include
#include "qemu/module.h"
-#include "malloc.h"
+#include "libqos-malloc.h"
[PATCH 14/51] backends/tpm: Exclude headers and macros that don't exist on win32
From: Bin Meng
These headers and macros do not exist on Windows. Exclude them.
Signed-off-by: Bin Meng
---
backends/tpm/tpm_ioctl.h | 4
1 file changed, 4 insertions(+)
diff --git a/backends/tpm/tpm_ioctl.h b/backends/tpm/tpm_ioctl.h
index bd6c12cb86..d67bf0283b 100644
--- a/backends/tpm/tpm_ioctl.h
+++ b/backends/tpm/tpm_ioctl.h
@@ -9,8 +9,10 @@
#ifndef TPM_IOCTL_H
#define TPM_IOCTL_H
+#ifndef _WIN32
#include
#include
+#endif
#ifdef HAVE_SYS_IOCCOM_H
#include
@@ -222,6 +224,7 @@ typedef struct ptm_setbuffersize ptm_setbuffersize;
#define PTM_CAP_SET_DATAFD (1 << 12)
#define PTM_CAP_SET_BUFFERSIZE (1 << 13)
+#ifndef _WIN32
enum {
PTM_GET_CAPABILITY = _IOR('P', 0, ptm_cap),
PTM_INIT = _IOWR('P', 1, ptm_init),
@@ -241,6 +244,7 @@ enum {
PTM_SET_DATAFD = _IOR('P', 15, ptm_res),
PTM_SET_BUFFERSIZE = _IOWR('P', 16, ptm_setbuffersize),
};
+#endif
/*
* Commands used by the non-CUSE TPMs
--
2.34.1
[PATCH 40/51] chardev/char-file: Add FILE_SHARE_WRITE when openning the file for win32
From: Xuzhou Cheng
The combination of GENERIC_WRITE and FILE_SHARE_READ options does
not allow the same file to be opened again by CreateFile() from
another QEMU process with the same options when the previous QEMU
process still holds the file handle openned.
As per [1] we should add FILE_SHARE_WRITE to the share mode to allow
such use case. This change makes the behavior be consisten with the
POSIX platforms.
[1]
https://docs.microsoft.com/en-us/windows/win32/fileio/creating-and-opening-files
Signed-off-by: Xuzhou Cheng
Signed-off-by: Bin Meng
---
chardev/char-file.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/chardev/char-file.c b/chardev/char-file.c
index 2fd80707e5..66385211eb 100644
--- a/chardev/char-file.c
+++ b/chardev/char-file.c
@@ -60,8 +60,8 @@ static void qmp_chardev_open_file(Chardev *chr,
flags = CREATE_ALWAYS;
}
-out = CreateFile(file->out, accessmode, FILE_SHARE_READ, NULL, flags,
- FILE_ATTRIBUTE_NORMAL, NULL);
+out = CreateFile(file->out, accessmode, FILE_SHARE_READ | FILE_SHARE_WRITE,
+ NULL, flags, FILE_ATTRIBUTE_NORMAL, NULL);
if (out == INVALID_HANDLE_VALUE) {
error_setg(errp, "open %s failed", file->out);
return;
--
2.34.1
[PATCH 17/51] tests/qtest: Build virtio-net-test for posix only
From: Bin Meng
All of the virtio-net-test test cases require socketpair() to do the
test setup.
Signed-off-by: Bin Meng
---
tests/qtest/virtio-net-test.c | 6 --
tests/qtest/meson.build | 3 +--
2 files changed, 1 insertion(+), 8 deletions(-)
diff --git a/tests/qtest/virtio-net-test.c b/tests/qtest/virtio-net-test.c
index 6ded252901..d44c3d9666 100644
--- a/tests/qtest/virtio-net-test.c
+++ b/tests/qtest/virtio-net-test.c
@@ -26,8 +26,6 @@
#define QVIRTIO_NET_TIMEOUT_US (30 * 1000 * 1000)
#define VNET_HDR_SIZE sizeof(struct virtio_net_hdr_mrg_rxbuf)
-#ifndef _WIN32
-
static void rx_test(QVirtioDevice *dev,
QGuestAllocator *alloc, QVirtQueue *vq,
int socket)
@@ -165,8 +163,6 @@ static void stop_cont_test(void *obj, void *data,
QGuestAllocator *t_alloc)
rx_stop_cont_test(dev, t_alloc, rx, sv[0]);
}
-#endif
-
static void hotplug(void *obj, void *data, QGuestAllocator *t_alloc)
{
QVirtioPCIDevice *dev = obj;
@@ -324,10 +320,8 @@ static void register_virtio_net_test(void)
};
qos_add_test("hotplug", "virtio-net-pci", hotplug, &opts);
-#ifndef _WIN32
qos_add_test("basic", "virtio-net", send_recv_test, &opts);
qos_add_test("rx_stop_cont", "virtio-net", stop_cont_test, &opts);
-#endif
qos_add_test("announce-self", "virtio-net", announce_self, &opts);
/* These tests do not need a loopback backend. */
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 9d0f82bf1c..72bb9e21f3 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -259,7 +259,6 @@ qos_test_ss.add(
'usb-hcd-ohci-test.c',
'virtio-test.c',
'virtio-blk-test.c',
- 'virtio-net-test.c',
'virtio-rng-test.c',
'virtio-scsi-test.c',
'virtio-serial-test.c',
@@ -267,7 +266,7 @@ qos_test_ss.add(
'vmxnet3-test.c',
)
if config_host.has_key('CONFIG_POSIX')
- qos_test_ss.add(files('e1000e-test.c'))
+ qos_test_ss.add(files('e1000e-test.c', 'virtio-net-test.c'))
endif
if have_virtfs
qos_test_ss.add(files('virtio-9p-test.c'))
--
2.34.1
[PATCH 27/51] tests/qtest: Use send/recv for socket communication
From: Xuzhou Cheng
Socket communication in the libqtest and libqmp codes uses read()
and write() which work on any file descriptor on *nix, and sockets
in *nix are an example of a file descriptor.
However sockets on Windows do not use *nix-style file descriptors,
so read() and write() cannot be used on sockets on Windows.
Switch over to use send() and recv() instead which work on both
Windows and *nix.
Signed-off-by: Xuzhou Cheng
Signed-off-by: Bin Meng
---
tests/qtest/libqmp.c | 4 ++--
tests/qtest/libqtest.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/tests/qtest/libqmp.c b/tests/qtest/libqmp.c
index ade26c15f0..995a39c1f8 100644
--- a/tests/qtest/libqmp.c
+++ b/tests/qtest/libqmp.c
@@ -36,7 +36,7 @@ typedef struct {
static void socket_send(int fd, const char *buf, size_t size)
{
-size_t res = qemu_write_full(fd, buf, size);
+ssize_t res = send(fd, buf, size, 0);
assert(res == size);
}
@@ -69,7 +69,7 @@ QDict *qmp_fd_receive(int fd)
ssize_t len;
char c;
-len = read(fd, &c, 1);
+len = recv(fd, &c, 1, 0);
if (len == -1 && errno == EINTR) {
continue;
}
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 909583dad3..b7b7c9c541 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -438,7 +438,7 @@ void qtest_quit(QTestState *s)
static void socket_send(int fd, const char *buf, size_t size)
{
-size_t res = qemu_write_full(fd, buf, size);
+ssize_t res = send(fd, buf, size, 0);
assert(res == size);
}
@@ -470,7 +470,7 @@ static GString *qtest_client_socket_recv_line(QTestState *s)
ssize_t len;
char buffer[1024];
-len = read(s->fd, buffer, sizeof(buffer));
+len = recv(s->fd, buffer, sizeof(buffer), 0);
if (len == -1 && errno == EINTR) {
continue;
}
--
2.34.1
[PATCH 41/51] tests/qtest: migration-test: Kill "to" after migration is canceled
From: Xuzhou Cheng
Make sure QEMU process "to" is killed before launching another target
for migration in the test_multifd_tcp_cancel case.
Signed-off-by: Xuzhou Cheng
Signed-off-by: Bin Meng
---
tests/qtest/migration-test.c | 4
1 file changed, 4 insertions(+)
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 125d48d855..18ec079abf 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -2132,6 +2132,10 @@ static void test_multifd_tcp_cancel(void)
wait_for_migration_pass(from);
migrate_cancel(from);
+/* Make sure QEMU process "to" is killed */
+if (qtest_probe_child(to)) {
+qtest_kill_qemu(to);
+}
args = (MigrateStart){
.only_target = true,
--
2.34.1
[PATCH 23/51] accel/qtest: Support qtest accelerator for Windows
From: Xuzhou Cheng
Currently signal SIGIPI [=SIGUSR1] is used to kick the dummy CPU
when qtest accelerator is used. However SIGUSR1 is unsupported on
Windows. To support Windows, we add a QemuSemaphore CPUState::sem
to kick the dummy CPU instead for Windows.
Signed-off-by: Xuzhou Cheng
Signed-off-by: Bin Meng
---
include/hw/core/cpu.h | 1 +
accel/dummy-cpus.c | 14 --
softmmu/cpus.c | 9 +
accel/meson.build | 1 +
accel/qtest/meson.build | 1 +
5 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 500503da13..c564108877 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -325,6 +325,7 @@ struct CPUState {
struct QemuThread *thread;
#ifdef _WIN32
HANDLE hThread;
+QemuSemaphore sem;
#endif
int thread_id;
bool running, has_waiter;
diff --git a/accel/dummy-cpus.c b/accel/dummy-cpus.c
index 10429fdfb2..d6a1b8d0a2 100644
--- a/accel/dummy-cpus.c
+++ b/accel/dummy-cpus.c
@@ -21,8 +21,6 @@
static void *dummy_cpu_thread_fn(void *arg)
{
CPUState *cpu = arg;
-sigset_t waitset;
-int r;
rcu_register_thread();
@@ -32,8 +30,13 @@ static void *dummy_cpu_thread_fn(void *arg)
cpu->can_do_io = 1;
current_cpu = cpu;
+#ifndef _WIN32
+sigset_t waitset;
+int r;
+
sigemptyset(&waitset);
sigaddset(&waitset, SIG_IPI);
+#endif
/* signal CPU creation */
cpu_thread_signal_created(cpu);
@@ -41,6 +44,7 @@ static void *dummy_cpu_thread_fn(void *arg)
do {
qemu_mutex_unlock_iothread();
+#ifndef _WIN32
do {
int sig;
r = sigwait(&waitset, &sig);
@@ -49,6 +53,9 @@ static void *dummy_cpu_thread_fn(void *arg)
perror("sigwait");
exit(1);
}
+#else
+qemu_sem_wait(&cpu->sem);
+#endif
qemu_mutex_lock_iothread();
qemu_wait_io_event(cpu);
} while (!cpu->unplug);
@@ -69,4 +76,7 @@ void dummy_start_vcpu_thread(CPUState *cpu)
cpu->cpu_index);
qemu_thread_create(cpu->thread, thread_name, dummy_cpu_thread_fn, cpu,
QEMU_THREAD_JOINABLE);
+#ifdef _WIN32
+qemu_sem_init(&cpu->sem, 0);
+#endif
}
diff --git a/softmmu/cpus.c b/softmmu/cpus.c
index 23b30484b2..fd10db927a 100644
--- a/softmmu/cpus.c
+++ b/softmmu/cpus.c
@@ -437,18 +437,19 @@ void qemu_wait_io_event(CPUState *cpu)
void cpus_kick_thread(CPUState *cpu)
{
-#ifndef _WIN32
-int err;
-
if (cpu->thread_kicked) {
return;
}
cpu->thread_kicked = true;
-err = pthread_kill(cpu->thread->thread, SIG_IPI);
+
+#ifndef _WIN32
+int err = pthread_kill(cpu->thread->thread, SIG_IPI);
if (err && err != ESRCH) {
fprintf(stderr, "qemu:%s: %s", __func__, strerror(err));
exit(1);
}
+#else
+qemu_sem_post(&cpu->sem);
#endif
}
diff --git a/accel/meson.build b/accel/meson.build
index b9a963cf80..b21c85dc0a 100644
--- a/accel/meson.build
+++ b/accel/meson.build
@@ -17,4 +17,5 @@ dummy_ss.add(files(
))
specific_ss.add_all(when: ['CONFIG_SOFTMMU', 'CONFIG_POSIX'], if_true:
dummy_ss)
+specific_ss.add_all(when: ['CONFIG_WIN32'], if_true: dummy_ss)
specific_ss.add_all(when: ['CONFIG_XEN'], if_true: dummy_ss)
diff --git a/accel/qtest/meson.build b/accel/qtest/meson.build
index 4c65600293..a4876fc0f2 100644
--- a/accel/qtest/meson.build
+++ b/accel/qtest/meson.build
@@ -1,2 +1,3 @@
qtest_module_ss.add(when: ['CONFIG_SOFTMMU', 'CONFIG_POSIX'],
if_true: files('qtest.c'))
+qtest_module_ss.add(when: ['CONFIG_WIN32'], if_true: files('qtest.c'))
--
2.34.1
[PATCH 21/51] tests/qtest: migration-test: Skip running test_migrate_fd_proto on win32
From: Bin Meng
The test case 'test_migrate_fd_proto' calls socketpair() which does
not exist on win32. Exclude it. The helper function wait_command_fd()
is not needed anymore, hence exclude it too.
Signed-off-by: Bin Meng
---
tests/qtest/migration-helpers.h | 2 ++
tests/qtest/migration-helpers.c | 2 ++
tests/qtest/migration-test.c| 4
3 files changed, 8 insertions(+)
diff --git a/tests/qtest/migration-helpers.h b/tests/qtest/migration-helpers.h
index 59561898d0..db0684de48 100644
--- a/tests/qtest/migration-helpers.h
+++ b/tests/qtest/migration-helpers.h
@@ -17,8 +17,10 @@
extern bool got_stop;
+#ifndef _WIN32
G_GNUC_PRINTF(3, 4)
QDict *wait_command_fd(QTestState *who, int fd, const char *command, ...);
+#endif
G_GNUC_PRINTF(2, 3)
QDict *wait_command(QTestState *who, const char *command, ...);
diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
index c6fbeb3974..f6f3c6680f 100644
--- a/tests/qtest/migration-helpers.c
+++ b/tests/qtest/migration-helpers.c
@@ -34,6 +34,7 @@ static void check_stop_event(QTestState *who)
}
}
+#ifndef _WIN32
/*
* Events can get in the way of responses we are actually waiting for.
*/
@@ -58,6 +59,7 @@ QDict *wait_command_fd(QTestState *who, int fd, const char
*command, ...)
return ret;
}
+#endif
/*
* Events can get in the way of responses we are actually waiting for.
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index af9250750b..2ae7498d5d 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -1629,6 +1629,7 @@ static void
test_precopy_tcp_tls_x509_reject_anon_client(void)
#endif /* CONFIG_TASN1 */
#endif /* CONFIG_GNUTLS */
+#ifndef _WIN32
static void *test_migrate_fd_start_hook(QTestState *from,
QTestState *to)
{
@@ -1697,6 +1698,7 @@ static void test_migrate_fd_proto(void)
};
test_precopy_common(&args);
}
+#endif /* _WIN32 */
static void do_test_validate_uuid(MigrateStart *args, bool should_fail)
{
@@ -2531,7 +2533,9 @@ int main(int argc, char **argv)
#endif /* CONFIG_GNUTLS */
/* qtest_add_func("/migration/ignore_shared", test_ignore_shared); */
+#ifndef _WIN32
qtest_add_func("/migration/fd_proto", test_migrate_fd_proto);
+#endif
qtest_add_func("/migration/validate_uuid", test_validate_uuid);
qtest_add_func("/migration/validate_uuid_error", test_validate_uuid_error);
qtest_add_func("/migration/validate_uuid_src_not_set",
--
2.34.1
[PATCH 48/51] io/channel-watch: Drop a superfluous '#ifdef WIN32'
From: Bin Meng In the win32 version qio_channel_create_socket_watch() body there is no need to do a '#ifdef WIN32'. Signed-off-by: Bin Meng --- io/channel-watch.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/io/channel-watch.c b/io/channel-watch.c index 0289b3647c..89f3c8a88a 100644 --- a/io/channel-watch.c +++ b/io/channel-watch.c @@ -285,11 +285,9 @@ GSource *qio_channel_create_socket_watch(QIOChannel *ioc, GSource *source; QIOChannelSocketSource *ssource; -#ifdef WIN32 WSAEventSelect(socket, ioc->event, FD_READ | FD_ACCEPT | FD_CLOSE | FD_CONNECT | FD_WRITE | FD_OOB); -#endif source = g_source_new(&qio_channel_socket_source_funcs, sizeof(QIOChannelSocketSource)); -- 2.34.1
[PATCH 22/51] tests/qtest: qmp-test: Skip running test_qmp_oob for win32
From: Bin Meng
The test_qmp_oob test case calls mkfifo() which does not exist on
win32. Exclude it.
Signed-off-by: Bin Meng
---
tests/qtest/qmp-test.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/tests/qtest/qmp-test.c b/tests/qtest/qmp-test.c
index b950dbafaf..4a165447f8 100644
--- a/tests/qtest/qmp-test.c
+++ b/tests/qtest/qmp-test.c
@@ -159,6 +159,8 @@ static void test_qmp_protocol(void)
qtest_quit(qts);
}
+#ifndef _WIN32
+
/* Out-of-band tests */
char *tmpdir;
@@ -279,6 +281,8 @@ static void test_qmp_oob(void)
qtest_quit(qts);
}
+#endif /* _WIN32 */
+
/* Preconfig tests */
static void test_qmp_preconfig(void)
@@ -338,7 +342,9 @@ int main(int argc, char *argv[])
g_test_init(&argc, &argv, NULL);
qtest_add_func("qmp/protocol", test_qmp_protocol);
+#ifndef _WIN32
qtest_add_func("qmp/oob", test_qmp_oob);
+#endif
qtest_add_func("qmp/preconfig", test_qmp_preconfig);
qtest_add_func("qmp/missing-any-arg", test_qmp_missing_any_arg);
--
2.34.1
[PATCH 28/51] tests/qtest: libqtest: Exclude the *_fds APIs for win32
From: Bin Meng
libqmp.c::qmp_fd_vsend_fds() is not available on Windows, hence any
APIs in libqtest that call libqmp.c::qmp_fd_vsend_fds() should be
excluded for win32 too. This includes the following:
* qtest_qmp_vsend_fds()
* qtest_vqmp_fds()
* qtest_qmp_fds()
* qtest_qmp_add_client()
Note qtest_qmp_vsend() was wrongly written to call qmp_fd_vsend_fds()
previously, but it should call the non fds version API qmp_fd_vsend().
Signed-off-by: Bin Meng
---
tests/qtest/libqtest.h | 8
tests/qtest/libqtest.c | 10 +-
2 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
index 94b187837d..3abc75964d 100644
--- a/tests/qtest/libqtest.h
+++ b/tests/qtest/libqtest.h
@@ -94,6 +94,7 @@ void qtest_kill_qemu(QTestState *s);
*/
void qtest_quit(QTestState *s);
+#ifndef _WIN32
/**
* qtest_qmp_fds:
* @s: #QTestState instance to operate on.
@@ -108,6 +109,7 @@ void qtest_quit(QTestState *s);
QDict *qtest_qmp_fds(QTestState *s, int *fds, size_t fds_num,
const char *fmt, ...)
G_GNUC_PRINTF(4, 5);
+#endif /* _WIN32 */
/**
* qtest_qmp:
@@ -152,6 +154,7 @@ void qtest_qmp_send_raw(QTestState *s, const char *fmt, ...)
*/
int qtest_socket_server(const char *socket_path);
+#ifndef _WIN32
/**
* qtest_vqmp_fds:
* @s: #QTestState instance to operate on.
@@ -167,6 +170,7 @@ int qtest_socket_server(const char *socket_path);
QDict *qtest_vqmp_fds(QTestState *s, int *fds, size_t fds_num,
const char *fmt, va_list ap)
G_GNUC_PRINTF(4, 0);
+#endif /* _WIN32 */
/**
* qtest_vqmp:
@@ -181,6 +185,7 @@ QDict *qtest_vqmp_fds(QTestState *s, int *fds, size_t
fds_num,
QDict *qtest_vqmp(QTestState *s, const char *fmt, va_list ap)
G_GNUC_PRINTF(2, 0);
+#ifndef _WIN32
/**
* qtest_qmp_vsend_fds:
* @s: #QTestState instance to operate on.
@@ -196,6 +201,7 @@ QDict *qtest_vqmp(QTestState *s, const char *fmt, va_list
ap)
void qtest_qmp_vsend_fds(QTestState *s, int *fds, size_t fds_num,
const char *fmt, va_list ap)
G_GNUC_PRINTF(4, 0);
+#endif /* _WIN32 */
/**
* qtest_qmp_vsend:
@@ -743,6 +749,7 @@ void qtest_qmp_device_add_qdict(QTestState *qts, const char
*drv,
void qtest_qmp_device_add(QTestState *qts, const char *driver, const char *id,
const char *fmt, ...) G_GNUC_PRINTF(4, 5);
+#ifndef _WIN32
/**
* qtest_qmp_add_client:
* @qts: QTestState instance to operate on
@@ -752,6 +759,7 @@ void qtest_qmp_device_add(QTestState *qts, const char
*driver, const char *id,
* Call QMP ``getfd`` followed by ``add_client`` with the given @fd.
*/
void qtest_qmp_add_client(QTestState *qts, const char *protocol, int fd);
+#endif /* _WIN32 */
/**
* qtest_qmp_device_del:
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index b7b7c9c541..1b24a4f1f7 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -594,17 +594,20 @@ int qtest_socket_server(const char *socket_path)
return sock;
}
+#ifndef _WIN32
void qtest_qmp_vsend_fds(QTestState *s, int *fds, size_t fds_num,
const char *fmt, va_list ap)
{
qmp_fd_vsend_fds(s->qmp_fd, fds, fds_num, fmt, ap);
}
+#endif
void qtest_qmp_vsend(QTestState *s, const char *fmt, va_list ap)
{
-qmp_fd_vsend_fds(s->qmp_fd, NULL, 0, fmt, ap);
+qmp_fd_vsend(s->qmp_fd, fmt, ap);
}
+#ifndef _WIN32
QDict *qtest_vqmp_fds(QTestState *s, int *fds, size_t fds_num,
const char *fmt, va_list ap)
{
@@ -613,6 +616,7 @@ QDict *qtest_vqmp_fds(QTestState *s, int *fds, size_t
fds_num,
/* Receive reply */
return qtest_qmp_receive(s);
}
+#endif
QDict *qtest_vqmp(QTestState *s, const char *fmt, va_list ap)
{
@@ -622,6 +626,7 @@ QDict *qtest_vqmp(QTestState *s, const char *fmt, va_list
ap)
return qtest_qmp_receive(s);
}
+#ifndef _WIN32
QDict *qtest_qmp_fds(QTestState *s, int *fds, size_t fds_num,
const char *fmt, ...)
{
@@ -633,6 +638,7 @@ QDict *qtest_qmp_fds(QTestState *s, int *fds, size_t
fds_num,
va_end(ap);
return response;
}
+#endif
QDict *qtest_qmp(QTestState *s, const char *fmt, ...)
{
@@ -1329,6 +1335,7 @@ void qtest_qmp_device_add(QTestState *qts, const char
*driver, const char *id,
qobject_unref(args);
}
+#ifndef _WIN32
void qtest_qmp_add_client(QTestState *qts, const char *protocol, int fd)
{
QDict *resp;
@@ -1348,6 +1355,7 @@ void qtest_qmp_add_client(QTestState *qts, const char
*protocol, int fd)
g_assert(!qdict_haskey(resp, "error"));
qobject_unref(resp);
}
+#endif
/*
* Generic hot-unplugging test via the device_del QMP command.
--
2.34.1
[PATCH 26/51] tests/qtest: libqtest: Move global_qtest definition back to libqtest.c
From: Xuzhou Cheng
Commit dd2107497275 ("tests/libqtest: Use libqtest-single.h in tests that
require global_qtest")
moved global_qtest to libqtest-single.h, by declaring global_qtest
attribute to be common and weak.
This trick unfortunately does not work on Windows, and building
qtest test cases results in multiple definition errors of the weak
symbol global_qtest, as Windows PE does not have the concept of
the so-called weak symbol like ELF in the *nix world.
Let's move the definition of global_qtest back to libqtest.c.
Signed-off-by: Xuzhou Cheng
Signed-off-by: Bin Meng
---
tests/qtest/libqtest-single.h | 2 +-
tests/qtest/libqtest.c| 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/tests/qtest/libqtest-single.h b/tests/qtest/libqtest-single.h
index 4e7d0ae1dc..3294985d7b 100644
--- a/tests/qtest/libqtest-single.h
+++ b/tests/qtest/libqtest-single.h
@@ -13,7 +13,7 @@
#include "libqtest.h"
-QTestState *global_qtest __attribute__((common, weak));
+extern QTestState *global_qtest;
/**
* qtest_start:
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 34744ace7c..909583dad3 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -65,6 +65,8 @@ struct QTestState
GList *pending_events;
};
+QTestState *global_qtest;
+
static GHookList abrt_hooks;
static struct sigaction sigact_old;
--
2.34.1
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On Tue, Aug 23, 2022 at 04:05:27PM +, Sean Christopherson wrote: > On Tue, Aug 23, 2022, David Hildenbrand wrote: > > On 19.08.22 05:38, Hugh Dickins wrote: > > > On Fri, 19 Aug 2022, Sean Christopherson wrote: > > >> On Thu, Aug 18, 2022, Kirill A . Shutemov wrote: > > >>> On Wed, Aug 17, 2022 at 10:40:12PM -0700, Hugh Dickins wrote: > > On Wed, 6 Jul 2022, Chao Peng wrote: > > But since then, TDX in particular has forced an effort into preventing > > (by flags, seals, notifiers) almost everything that makes it > > shmem/tmpfs. > > > > Are any of the shmem.c mods useful to existing users of shmem.c? No. > > Is MFD_INACCESSIBLE useful or comprehensible to memfd_create() users? > > No. > > >> > > >> But QEMU and other VMMs are users of shmem and memfd. The new features > > >> certainly > > >> aren't useful for _all_ existing users, but I don't think it's fair to > > >> say that > > >> they're not useful for _any_ existing users. > > > > > > Okay, I stand corrected: there exist some users of memfd_create() > > > who will also have use for "INACCESSIBLE" memory. > > > > As raised in reply to the relevant patch, I'm not sure if we really have > > to/want to expose MFD_INACCESSIBLE to user space. I feel like this is a > > requirement of specific memfd_notifer (memfile_notifier) implementations > > -- such as TDX that will convert the memory and MCE-kill the machine on > > ordinary write access. We might be able to set/enforce this when > > registering a notifier internally instead, and fail notifier > > registration if a condition isn't met (e.g., existing mmap). > > > > So I'd be curious, which other users of shmem/memfd would benefit from > > (MMU)-"INACCESSIBLE" memory obtained via memfd_create()? > > I agree that there's no need to expose the inaccessible behavior via uAPI. > Making > it a kernel-internal thing that's negotiated/resolved when KVM binds to the fd > would align INACCESSIBLE with the UNMOVABLE and UNRECLAIMABLE flags (and any > other > flags that get added in the future). > > AFAICT, the user-visible flag is a holdover from the early RFCs and doesn't > provide > any unique functionality. That's also what I'm thinking. And I don't see problem immediately if user has populated the fd at the binding time. Actually that looks an advantage for previously discussed guest payload pre-loading. > > If we go that route, we might want to have shmem/memfd require INACCESSIBLE > to be > set for the initial implementation. I.e. disallow binding without > INACCESSIBLE > until there's a use case. I can do that. Chao
[PATCH 43/51] tests/qtest: npcm7xx_emc-test: Skip running test_{tx, rx} on win32
From: Bin Meng
The test cases 'test_{tx,rx}' call socketpair() which does not exist
on win32. Exclude them.
Signed-off-by: Bin Meng
---
tests/qtest/npcm7xx_emc-test.c | 8
1 file changed, 8 insertions(+)
diff --git a/tests/qtest/npcm7xx_emc-test.c b/tests/qtest/npcm7xx_emc-test.c
index a353fef0ca..c373d24e1e 100644
--- a/tests/qtest/npcm7xx_emc-test.c
+++ b/tests/qtest/npcm7xx_emc-test.c
@@ -209,6 +209,7 @@ static int emc_module_index(const EMCModule *mod)
return diff;
}
+#ifndef _WIN32
static void packet_test_clear(void *sockets)
{
int *test_sockets = sockets;
@@ -243,6 +244,7 @@ static int *packet_test_init(int module_num, GString
*cmd_line)
g_test_queue_destroy(packet_test_clear, test_sockets);
return test_sockets;
}
+#endif /* _WIN32 */
static uint32_t emc_read(QTestState *qts, const EMCModule *mod,
NPCM7xxPWMRegister regno)
@@ -250,6 +252,7 @@ static uint32_t emc_read(QTestState *qts, const EMCModule
*mod,
return qtest_readl(qts, mod->base_addr + regno * sizeof(uint32_t));
}
+#ifndef _WIN32
static void emc_write(QTestState *qts, const EMCModule *mod,
NPCM7xxPWMRegister regno, uint32_t value)
{
@@ -339,6 +342,7 @@ static bool emc_soft_reset(QTestState *qts, const EMCModule
*mod)
g_message("%s: Timeout expired", __func__);
return false;
}
+#endif /* _WIN32 */
/* Check emc registers are reset to default value. */
static void test_init(gconstpointer test_data)
@@ -387,6 +391,7 @@ static void test_init(gconstpointer test_data)
qtest_quit(qts);
}
+#ifndef _WIN32
static bool emc_wait_irq(QTestState *qts, const EMCModule *mod, int step,
bool is_tx)
{
@@ -843,6 +848,7 @@ static void test_rx(gconstpointer test_data)
qtest_quit(qts);
}
+#endif /* _WIN32 */
static void emc_add_test(const char *name, const TestData* td,
GTestDataFunc fn)
@@ -865,8 +871,10 @@ int main(int argc, char **argv)
td->module = &emc_module_list[i];
add_test(init, td);
+#ifndef _WIN32
add_test(tx, td);
add_test(rx, td);
+#endif
}
return g_test_run();
--
2.34.1
[PATCH 24/51] tests/qtest: libqos: Drop inclusion of
From: Xuzhou Cheng There is no in the Windows build environment. Actually this is not needed in the non-win32 builds too. Drop it. Signed-off-by: Xuzhou Cheng Signed-off-by: Bin Meng --- tests/qtest/libqos/libqos.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/qtest/libqos/libqos.c b/tests/qtest/libqos/libqos.c index 85c7641add..5ffda080ec 100644 --- a/tests/qtest/libqos/libqos.c +++ b/tests/qtest/libqos/libqos.c @@ -1,6 +1,4 @@ #include "qemu/osdep.h" -#include - #include "../libqtest.h" #include "libqos.h" #include "pci.h" -- 2.34.1
[PATCH 31/51] tests/qtest: Support libqtest to build and run on Windows
From: Bin Meng
At present the libqtest codes were written to depend on several
POSIX APIs, including fork(), kill() and waitpid(). Unfortunately
these APIs are not available on Windows.
This commit implements the corresponding functionalities using
win32 native APIs. With this change, all qtest cases can build
successfully on a Windows host, and we can start qtest testing
on Windows now.
Signed-off-by: Xuzhou Cheng
Signed-off-by: Bin Meng
---
tests/qtest/libqtest.c | 101 +++-
tests/qtest/meson.build | 5 +-
2 files changed, 101 insertions(+), 5 deletions(-)
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 70d7578740..99e52ff571 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -16,9 +16,11 @@
#include "qemu/osdep.h"
+#ifndef _WIN32
#include
#include
#include
+#endif /* _WIN32 */
#ifdef __linux__
#include
#endif /* __linux__ */
@@ -27,6 +29,7 @@
#include "libqmp.h"
#include "qemu/ctype.h"
#include "qemu/cutils.h"
+#include "qemu/sockets.h"
#include "qapi/qmp/qdict.h"
#include "qapi/qmp/qjson.h"
#include "qapi/qmp/qlist.h"
@@ -35,6 +38,16 @@
#define MAX_IRQ 256
#define SOCKET_TIMEOUT 50
+#ifndef _WIN32
+# define CMD_EXEC "exec "
+# define DEV_STDERR "/dev/fd/2"
+# define DEV_NULL "/dev/null"
+#else
+# define CMD_EXEC ""
+# define DEV_STDERR "2"
+# define DEV_NULL "nul"
+#endif
+
typedef void (*QTestSendFn)(QTestState *s, const char *buf);
typedef void (*ExternalSendFn)(void *s, const char *buf);
typedef GString* (*QTestRecvFn)(QTestState *);
@@ -68,6 +81,9 @@ struct QTestState
QTestState *global_qtest;
static GHookList abrt_hooks;
+#ifdef _WIN32
+typedef void (*sighandler_t)(int);
+#endif
static sighandler_t sighandler_old;
static int qtest_query_target_endianness(QTestState *s);
@@ -120,10 +136,18 @@ bool qtest_probe_child(QTestState *s)
pid_t pid = s->qemu_pid;
if (pid != -1) {
+#ifndef _WIN32
pid = waitpid(pid, &s->wstatus, WNOHANG);
if (pid == 0) {
return true;
}
+#else
+DWORD exit_code;
+GetExitCodeProcess((HANDLE)pid, &exit_code);
+if (exit_code == STILL_ACTIVE) {
+return true;
+}
+#endif
s->qemu_pid = -1;
}
return false;
@@ -137,13 +161,23 @@ void qtest_set_expected_status(QTestState *s, int status)
void qtest_kill_qemu(QTestState *s)
{
pid_t pid = s->qemu_pid;
+#ifndef _WIN32
int wstatus;
+#else
+DWORD ret, exit_code;
+#endif
/* Skip wait if qtest_probe_child already reaped. */
if (pid != -1) {
+#ifndef _WIN32
kill(pid, SIGTERM);
TFR(pid = waitpid(s->qemu_pid, &s->wstatus, 0));
assert(pid == s->qemu_pid);
+#else
+TerminateProcess((HANDLE)pid, s->expected_status);
+ret = WaitForSingleObject((HANDLE)pid, INFINITE);
+assert(ret == WAIT_OBJECT_0);
+#endif
s->qemu_pid = -1;
}
@@ -151,6 +185,7 @@ void qtest_kill_qemu(QTestState *s)
* Check whether qemu exited with expected exit status; anything else is
* fishy and should be logged with as much detail as possible.
*/
+#ifndef _WIN32
wstatus = s->wstatus;
if (WIFEXITED(wstatus) && WEXITSTATUS(wstatus) != s->expected_status) {
fprintf(stderr, "%s:%d: kill_qemu() tried to terminate QEMU "
@@ -167,6 +202,16 @@ void qtest_kill_qemu(QTestState *s)
__FILE__, __LINE__, sig, signame, dump);
abort();
}
+#else
+GetExitCodeProcess((HANDLE)pid, &exit_code);
+CloseHandle((HANDLE)pid);
+if (exit_code != s->expected_status) {
+fprintf(stderr, "%s:%d: kill_qemu() tried to terminate QEMU "
+"process but encountered exit status %ld (expected %d)\n",
+__FILE__, __LINE__, exit_code, s->expected_status);
+abort();
+}
+#endif
}
static void kill_qemu_hook_func(void *s)
@@ -245,6 +290,38 @@ static const char *qtest_qemu_binary(void)
return qemu_bin;
}
+#ifdef _WIN32
+static pid_t qtest_create_process(char *cmd)
+{
+STARTUPINFO si;
+PROCESS_INFORMATION pi;
+BOOL ret;
+
+ZeroMemory(&si, sizeof(si));
+si.cb = sizeof(si);
+ZeroMemory(&pi, sizeof(pi));
+
+ret = CreateProcess(NULL, /* module name */
+cmd,/* command line */
+NULL, /* process handle not inheritable */
+NULL, /* thread handle not inheritable */
+FALSE, /* set handle inheritance to FALSE */
+0, /* No creation flags */
+NULL, /* use parent's environment block */
+NULL, /* use parent's starting directory */
+&si,/* pointer to STARTUPINFO structure */
+&pi /* pointer to PROCESS_INFORMATION structure */
+);
+if (ret == 0) {
+fprintf(stderr, "%s:%d: una
[PATCH v5 0/2] block: add missed block_acct_setup with new block device init procedure
Commit 5f76a7aac156ca75680dad5df4a385fd0b58f6b1 is looking harmless from the first glance, but it has changed things a lot. 'libvirt' uses it to detect that it should follow new initialization way and this changes things considerably. With this procedure followed, blockdev_init() is not called anymore and thus block_acct_setup() helper is not called. This means in particular that defaults for block accounting statistics are changed and account_invalid/account_failed are actually initialized as false instead of true originally. This commit changes things to match original world. There are the following constraints: * new default value in block_acct_init() is set to true * block_acct_setup() inside blockdev_init() is called before blkconf_apply_backend_options() * thus newly created option in block device properties has precedence if specified Changes from v4: * removed hunk to QAPI which was used to test old initialization path * added R-b: Vladimir Changes from v3: * fixed accidentally wrong submission. Contains changes which should be sent as v3 Changes from v2: * called bool_from_onoffauto(account_..., true) in the first patch to preserve original semantics before patch 2 Changes from v1: * set account_invalid/account_failed to true by default * pass OnOffAuto to block_acct_init() to handle double initialization (patch 1) * changed properties on BLK device to OnOffAuto Signed-off-by: Denis V. Lunev CC: Peter Krempa CC: Markus Armbruster CC: John Snow CC: Kevin Wolf CC: Hanna Reitz CC: Vladimir Sementsov-Ogievskiy
[PATCH 29/51] tests/qtest: libqtest: Install signal handler via signal()
From: Bin Meng
At present the codes uses sigaction() to install signal handler with
a flag SA_RESETHAND. Such usage can be covered by the signal() API
that is a simplified interface to the general sigaction() facility.
Update to use signal() to install the signal handler, as it is
avaiable on Windows which we are going to support.
Signed-off-by: Bin Meng
---
tests/qtest/libqtest.c | 14 +++---
1 file changed, 3 insertions(+), 11 deletions(-)
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 1b24a4f1f7..70d7578740 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -68,7 +68,7 @@ struct QTestState
QTestState *global_qtest;
static GHookList abrt_hooks;
-static struct sigaction sigact_old;
+static sighandler_t sighandler_old;
static int qtest_query_target_endianness(QTestState *s);
@@ -181,20 +181,12 @@ static void sigabrt_handler(int signo)
static void setup_sigabrt_handler(void)
{
-struct sigaction sigact;
-
-/* Catch SIGABRT to clean up on g_assert() failure */
-sigact = (struct sigaction){
-.sa_handler = sigabrt_handler,
-.sa_flags = SA_RESETHAND,
-};
-sigemptyset(&sigact.sa_mask);
-sigaction(SIGABRT, &sigact, &sigact_old);
+sighandler_old = signal(SIGABRT, sigabrt_handler);
}
static void cleanup_sigabrt_handler(void)
{
-sigaction(SIGABRT, &sigact_old, NULL);
+signal(SIGABRT, sighandler_old);
}
static bool hook_list_is_empty(GHookList *hook_list)
--
2.34.1
[PATCH 49/51] io/channel-watch: Fix socket watch on Windows
From: Bin Meng
Random failure was observed when running qtests on Windows due to
"Broken pipe" detected by qmp_fd_receive(). What happened is that
the qtest executable sends testing data over a socket to the QEMU
under test but no response is received. The errno of the recv()
call from the qtest executable indicates ETIMEOUT, due to the qmp
chardev's tcp_chr_read() is never called to receive testing data
hence no response is sent to the other side.
tcp_chr_read() is registered as the callback of the socket watch
GSource. The reason of the callback not being called by glib, is
that the source check fails to indicate the source is ready. There
are two socket watch sources created to monitor the same socket
event object from the char-socket backend in update_ioc_handlers().
During the source check phase, qio_channel_socket_source_check()
calls WSAEnumNetworkEvents() to discovers occurrences of network
events for the indicated socket, clear internal network event records,
and reset the event object. Testing shows that if we don't reset the
event object by not passing the event handle to WSAEnumNetworkEvents()
the symptom goes away and qtest runs very stably.
It looks we don't need to call WSAEnumNetworkEvents() at all, as we
don't parse the result of WSANETWORKEVENTS returned from this API.
We use select() to poll the socket status. Fix this instability by
dropping the WSAEnumNetworkEvents() call.
Signed-off-by: Bin Meng
---
During the testing, I removed the following codes in update_ioc_handlers():
remove_hup_source(s);
s->hup_source = qio_channel_create_watch(s->ioc, G_IO_HUP);
g_source_set_callback(s->hup_source, (GSourceFunc)tcp_chr_hup,
chr, NULL);
g_source_attach(s->hup_source, chr->gcontext);
and such change also makes the symptom go away.
And if I moved the above codes to the beginning, before the call to
io_add_watch_poll(), the symptom also goes away.
It seems two sources watching on the same socket event object is
the key that leads to the instability. The order of adding a source
watch seems to also play a role but I can't explain why.
Hopefully a Windows and glib expert could explain this behavior.
io/channel-watch.c | 4
1 file changed, 4 deletions(-)
diff --git a/io/channel-watch.c b/io/channel-watch.c
index 89f3c8a88a..e34d86e810 100644
--- a/io/channel-watch.c
+++ b/io/channel-watch.c
@@ -115,17 +115,13 @@ static gboolean
qio_channel_socket_source_check(GSource *source)
{
static struct timeval tv0;
-
QIOChannelSocketSource *ssource = (QIOChannelSocketSource *)source;
-WSANETWORKEVENTS ev;
fd_set rfds, wfds, xfds;
if (!ssource->condition) {
return 0;
}
-WSAEnumNetworkEvents(ssource->socket, ssource->ioc->event, &ev);
-
FD_ZERO(&rfds);
FD_ZERO(&wfds);
FD_ZERO(&xfds);
--
2.34.1
Re: [PULL 1/6] tests/avocado: push default timeout to QemuBaseTest
On 24/08/2022 11.19, Alex Bennée wrote: Richard Henderson writes: On 8/23/22 08:25, Alex Bennée wrote: All of the QEMU tests eventually end up derrived from this class. Move the default timeout from LinuxTest to ensure we catch them all. As 15 minutes is fairly excessive we drop the default down to 2 minutes which is a more reasonable target for tests to aim for. Signed-off-by: Alex Bennée Reviewed-by: Richard Henderson Message-Id: <20220822165608.2980552-2-alex.ben...@linaro.org> diff --git a/tests/avocado/avocado_qemu/__init__.py b/tests/avocado/avocado_qemu/__init__.py index ed4853c805..0efd2bd212 100644 --- a/tests/avocado/avocado_qemu/__init__.py +++ b/tests/avocado/avocado_qemu/__init__.py @@ -227,6 +227,10 @@ def exec_command_and_wait_for_pattern(test, command, _console_interaction(test, success_message, failure_message, command + '\r') class QemuBaseTest(avocado.Test): + +# default timeout for all tests, can be overridden +timeout = 120 + def _get_unique_tag_val(self, tag_name): """ Gets a tag value, if unique for a key @@ -512,7 +516,6 @@ class LinuxTest(LinuxSSHMixIn, QemuSystemTest): to start with than the more vanilla `QemuSystemTest` class. """ -timeout = 900 distro = None username = 'root' password = 'password' Bah. https://gitlab.com/qemu-project/qemu/-/jobs/2923804714 Hmm weird - the avocado CFI job doesn't even appear on my CI list (even with push-ci-now). You likely have to set QEMU_CI_AVOCADO_TESTING in your gitlab settings, see docs/devel/ci-jobs.rst.inc. I think we really have to rework the way we run (or rather not run) the avocado tests - since with the current default behavior, they'll be ignored by most people by default. Thomas
[PATCH 50/51] .gitlab-ci.d/windows.yml: Increase the timeout to the runner limit
From: Bin Meng
commit 9f8e6cad65a6 ("gitlab-ci: Speed up the msys2-64bit job by using
--without-default-devices"
changed to compile QEMU with the --without-default-devices switch for
the msys2-64bit job, due to the build could not complete within the
project timeout (1h), and also mentioned that a bigger timeout was
getting ignored on the shared Gitlab-CI Windows runners.
However as of today it seems the shared Gitlab-CI Windows runners does
honor the job timeout, and the runner has the timeout limit of 2h, so
let's increase the timeout to the runner limit and drop the configure
switch "--without-default-devices" to get a larger build coverage.
As a result of this, the check-qtest starts running on Windows in CI.
Signed-off-by: Bin Meng
---
.gitlab-ci.d/windows.yml | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/.gitlab-ci.d/windows.yml b/.gitlab-ci.d/windows.yml
index c4bde758be..d4fd821b5f 100644
--- a/.gitlab-ci.d/windows.yml
+++ b/.gitlab-ci.d/windows.yml
@@ -10,7 +10,7 @@
- ${CI_PROJECT_DIR}/msys64/var/cache
needs: []
stage: build
- timeout: 70m
+ timeout: 2h
before_script:
- If ( !(Test-Path -Path msys64\var\cache ) ) {
mkdir msys64\var\cache
@@ -59,7 +59,7 @@ msys2-64bit:
- $env:MSYSTEM = 'MINGW64' # Start a 64 bit Mingw environment
- $env:MSYS = 'winsymlinks:native' # Enable native Windows symlink
- .\msys64\usr\bin\bash -lc './configure --target-list=x86_64-softmmu
- --enable-capstone --without-default-devices'
+ --enable-capstone'
- .\msys64\usr\bin\bash -lc "sed -i '/^ROMS=/d' build/config-host.mak"
- .\msys64\usr\bin\bash -lc 'make -j2'
- .\msys64\usr\bin\bash -lc 'make check'
--
2.34.1
[PATCH 30/51] tests: Skip iotests and qtest when '--without-default-devices'
From: Bin Meng
When QEMU is configured with '--without-default-devices', we should
not build and run iotests and qtest because devices used by these
test cases are not built in.
Signed-off-by: Bin Meng
---
tests/qemu-iotests/meson.build | 5 +
tests/qtest/meson.build| 5 +
2 files changed, 10 insertions(+)
diff --git a/tests/qemu-iotests/meson.build b/tests/qemu-iotests/meson.build
index 323a4acb6a..38d9a874d2 100644
--- a/tests/qemu-iotests/meson.build
+++ b/tests/qemu-iotests/meson.build
@@ -2,6 +2,11 @@ if not have_tools or targetos == 'windows' or
get_option('gprof')
subdir_done()
endif
+# Skip iotests if configured without a default selection of devices
+if not get_option('default_devices')
+ subdir_done()
+endif
+
foreach cflag: config_host['QEMU_CFLAGS'].split()
if cflag.startswith('-fsanitize') and \
not cflag.contains('safe-stack') and not cflag.contains('cfi-icall')
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index c97da5a062..0291b3966c 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -4,6 +4,11 @@ if not config_host.has_key('CONFIG_POSIX')
subdir_done()
endif
+# Skip QTests if configured without a default selection of devices
+if not get_option('default_devices')
+ subdir_done()
+endif
+
slow_qtests = {
'ahci-test' : 60,
'bios-tables-test' : 120,
--
2.34.1
[PATCH 33/51] tests/qtest: {ahci, ide}-test: Use relative path for temporary files
From: Bin Meng
These test cases uses "blkdebug:path/to/config:path/to/image" for
testing. On Windows, absolute file paths contain the delimiter ':'
which causes the blkdebug filename parser fail to parse filenames.
Signed-off-by: Bin Meng
---
tests/qtest/ahci-test.c | 19 ---
tests/qtest/ide-test.c | 18 --
2 files changed, 32 insertions(+), 5 deletions(-)
diff --git a/tests/qtest/ahci-test.c b/tests/qtest/ahci-test.c
index 0e88cd0eef..bce9ff770c 100644
--- a/tests/qtest/ahci-test.c
+++ b/tests/qtest/ahci-test.c
@@ -1848,7 +1848,7 @@ static void create_ahci_io_test(enum IOMode type, enum
AddrMode addr,
int main(int argc, char **argv)
{
-const char *arch;
+const char *arch, *base;
int ret;
int fd;
int c;
@@ -1886,8 +1886,21 @@ int main(int argc, char **argv)
return 0;
}
+/*
+ * "base" stores the starting point where we create temporary files.
+ *
+ * On Windows, this is set to the relative path of current working
+ * directory, because the absolute path causes the blkdebug filename
+ * parser fail to parse "blkdebug:path/to/config:path/to/image".
+ */
+#ifndef _WIN32
+base = g_get_tmp_dir();
+#else
+base = ".";
+#endif
+
/* Create a temporary image */
-tmp_path = g_strdup_printf("%s/qtest.XX", g_get_tmp_dir());
+tmp_path = g_strdup_printf("%s/qtest.XX", base);
fd = mkstemp(tmp_path);
g_assert(fd >= 0);
if (have_qemu_img()) {
@@ -1905,7 +1918,7 @@ int main(int argc, char **argv)
close(fd);
/* Create temporary blkdebug instructions */
-debug_path = g_strdup_printf("%s/qtest-blkdebug.XX", g_get_tmp_dir());
+debug_path = g_strdup_printf("%s/qtest-blkdebug.XX", base);
fd = mkstemp(debug_path);
g_assert(fd >= 0);
close(fd);
diff --git a/tests/qtest/ide-test.c b/tests/qtest/ide-test.c
index ebbf8e0126..c5cad6c0be 100644
--- a/tests/qtest/ide-test.c
+++ b/tests/qtest/ide-test.c
@@ -1011,17 +1011,31 @@ static void test_cdrom_dma(void)
int main(int argc, char **argv)
{
+const char *base;
int fd;
int ret;
+/*
+ * "base" stores the starting point where we create temporary files.
+ *
+ * On Windows, this is set to the relative path of current working
+ * directory, because the absolute path causes the blkdebug filename
+ * parser fail to parse "blkdebug:path/to/config:path/to/image".
+ */
+#ifndef _WIN32
+base = g_get_tmp_dir();
+#else
+base = ".";
+#endif
+
/* Create temporary blkdebug instructions */
-debug_path = g_strdup_printf("%s/qtest-blkdebug.XX", g_get_tmp_dir());
+debug_path = g_strdup_printf("%s/qtest-blkdebug.XX", base);
fd = mkstemp(debug_path);
g_assert(fd >= 0);
close(fd);
/* Create a temporary raw image */
-tmp_path = g_strdup_printf("%s/qtest.XX", g_get_tmp_dir());
+tmp_path = g_strdup_printf("%s/qtest.XX", base);
fd = mkstemp(tmp_path);
g_assert(fd >= 0);
ret = ftruncate(fd, TEST_IMAGE_SIZE);
--
2.34.1
Re: [PATCH v7 11/14] KVM: Register/unregister the guest private memory regions
On Fri, Aug 19, 2022 at 12:37:42PM -0700, Vishal Annapurve wrote:
> > ...
> > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> > index 230c8ff9659c..bb714c2a4b06 100644
> > --- a/virt/kvm/kvm_main.c
> > +++ b/virt/kvm/kvm_main.c
> > @@ -914,6 +914,35 @@ static int kvm_init_mmu_notifier(struct kvm *kvm)
> >
> > #endif /* CONFIG_MMU_NOTIFIER && KVM_ARCH_WANT_MMU_NOTIFIER */
> >
> > +#ifdef CONFIG_HAVE_KVM_PRIVATE_MEM
> > +#define KVM_MEM_ATTR_PRIVATE 0x0001
> > +static int kvm_vm_ioctl_set_encrypted_region(struct kvm *kvm, unsigned int
> > ioctl,
> > +struct kvm_enc_region *region)
> > +{
> > + unsigned long start, end;
> > + void *entry;
> > + int r;
> > +
> > + if (region->size == 0 || region->addr + region->size < region->addr)
> > + return -EINVAL;
> > + if (region->addr & (PAGE_SIZE - 1) || region->size & (PAGE_SIZE -
> > 1))
> > + return -EINVAL;
> > +
> > + start = region->addr >> PAGE_SHIFT;
> > + end = (region->addr + region->size - 1) >> PAGE_SHIFT;
> > +
> > + entry = ioctl == KVM_MEMORY_ENCRYPT_REG_REGION ?
> > + xa_mk_value(KVM_MEM_ATTR_PRIVATE) : NULL;
> > +
> > + r = xa_err(xa_store_range(&kvm->mem_attr_array, start, end,
> > + entry, GFP_KERNEL_ACCOUNT));
>
> xa_store_range seems to create multi-index entries by default.
> Subsequent xa_store_range call changes all the entries stored
> previously.
By using xa_store_range and storing them as multi-index entries I
expected to save some memory for continuous pages originally.
But sounds like the current multi-index store behaviour isn't quite
ready for our usage.
Chao
> xa_store needs to be used here instead of xa_store_range to achieve
> the intended behavior.
>
> > +
> > + kvm_zap_gfn_range(kvm, start, end + 1);
> > +
> > + return r;
> > +}
> > +#endif /* CONFIG_HAVE_KVM_PRIVATE_MEM */
> > +
> > ...
[PATCH 51/51] docs/devel: testing: Document writing portable test cases
From: Bin Meng Update the best practices of how to write portable test cases that can be built and run successfully on both Linux and Windows hosts. Signed-off-by: Bin Meng --- docs/devel/testing.rst | 30 ++ 1 file changed, 30 insertions(+) diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst index 3f6ebd5073..8fcabda30f 100644 --- a/docs/devel/testing.rst +++ b/docs/devel/testing.rst @@ -115,6 +115,36 @@ check-block are in the "auto" group). See the "QEMU iotests" section below for more information. +Writing portable test cases +~~~ +Both unit tests and qtests can run on a Linux host as well as a Windows host. +Care must be taken when writing portable test cases that can be built and run +successfully on both hosts. The following are some best practices: + +* Use portable APIs from glib whenever necessary, e.g.: g_setenv(), + g_mkdtemp(), g_mkdir_with_parents(). +* Avoid using hardcoded /tmp for temporary file directory. + Use g_get_tmp_dir() instead. +* Bear in mind that Windows has different special string representation for + stdin/stdout/stderr and null devices. For example if your test case uses + "/dev/fd/2" and "/dev/null" on Linux, remember to use "2" and "nul" on + Windows instead. Also IO redirection does not work on Windows, so avoid + using "2>nul" whenever necessary. +* If your test cases uses the blkdebug feature, use relative path to pass + the config and image file paths in the command line as Windows absolute + path contains the delimeter ":" which will confuse the blkdebug parser. +* Use double quotes in your extra QEMU commmand line in your test cases + instead of single quotes, as Windows does not drop single quotes when + passing the command line to QEMU. +* Windows opens a file in text mode by default, while a POSIX compliant + implementation treats text files and binary files the same. So if your + test cases opens a file to write some data and later wants to compare the + written data with the original one, be sure to pass the letter 'b' as + part of the mode string to fopen(), or O_BINARY flag for the open() call. +* If a certain test case can only run on POSIX or Linux hosts, use a proper + #ifdef in the codes. If the whole test suite cannot run on Windows, disable + the build in the meson.build file. + QEMU iotests -- 2.34.1
[PATCH 35/51] tests/qtest: device-plug-test: Reverse the usage of double/single quotes
From: Bin Meng
The usage of double/single quotes in test_pci_unplug_json_request()
should be reversed to work on both win32 and non-win32 platforms:
- The value of -device parameter needs to be surrounded by "" as
Windows does not drop '' when passing it to QEMU which causes
QEMU command line option parser failure.
- The JSON key/value pairs need to be surrounded by '' to make the
JSON parser happy on Windows.
Signed-off-by: Bin Meng
---
tests/qtest/device-plug-test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/qtest/device-plug-test.c b/tests/qtest/device-plug-test.c
index 2e3137843e..a1fb99c8ff 100644
--- a/tests/qtest/device-plug-test.c
+++ b/tests/qtest/device-plug-test.c
@@ -95,7 +95,7 @@ static void test_pci_unplug_json_request(void)
}
QTestState *qtest = qtest_initf(
-"%s -device '{\"driver\": \"virtio-mouse-pci\", \"id\": \"dev0\"}'",
+"%s -device \"{'driver': 'virtio-mouse-pci', 'id': 'dev0'}\"",
machine_addition);
/*
--
2.34.1
[PATCH 34/51] tests/qtest: bios-tables-test: Adapt the case for win32
From: Bin Meng
Single quotes in the arguments (oem_id='CRASH ') are not removed in
the Windows environment before it is passed to the QEMU executable.
The space in the argument causes the "-acpitable" option parser to
think that all of its parameters are done, hence it complains:
'-acpitable' requires one of 'data' or 'file'
Change to use double quotes which works fine on all platforms.
Also /dev/null does not work on win32, and nul should be used.
Signed-off-by: Bin Meng
---
tests/qtest/bios-tables-test.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index 36783966b0..0148ce388c 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -1615,6 +1615,12 @@ static void test_acpi_virt_viot(void)
free_test_data(&data);
}
+#ifndef _WIN32
+# define DEV_NULL "/dev/null"
+#else
+# define DEV_NULL "nul"
+#endif
+
static void test_acpi_q35_slic(void)
{
test_data data = {
@@ -1622,9 +1628,9 @@ static void test_acpi_q35_slic(void)
.variant = ".slic",
};
-test_acpi_one("-acpitable sig=SLIC,oem_id='CRASH ',oem_table_id='ME',"
- "oem_rev=2210,asl_compiler_id='qemu',"
- "asl_compiler_rev=,data=/dev/null",
+test_acpi_one("-acpitable sig=SLIC,oem_id=\"CRASH \",oem_table_id=ME,"
+ "oem_rev=2210,asl_compiler_id=qemu,"
+ "asl_compiler_rev=,data=" DEV_NULL,
&data);
free_test_data(&data);
}
--
2.34.1
[PATCH 2/2] block: add missed block_acct_setup with new block device init procedure
Commit 5f76a7aac156ca75680dad5df4a385fd0b58f6b1 is looking harmless from
the first glance, but it has changed things a lot. 'libvirt' uses it to
detect that it should follow new initialization way and this changes
things considerably. With this procedure followed, blockdev_init() is
not called anymore and thus block_acct_setup() helper is not called.
This means in particular that defaults for block accounting statistics
are changed and account_invalid/account_failed are actually initialized
as false instead of true originally.
This commit changes things to match original world. There are the following
constraints:
* new default value in block_acct_init() is set to true
* block_acct_setup() inside blockdev_init() is called before
blkconf_apply_backend_options()
* thus newly created option in block device properties has precedence if
specified
Signed-off-by: Denis V. Lunev
Reviewed-by: Vladimir Sementsov-Ogievskiy
CC: Peter Krempa
CC: Markus Armbruster
CC: John Snow
CC: Kevin Wolf
CC: Hanna Reitz
---
block/accounting.c | 8 +++-
hw/block/block.c | 2 +
include/hw/block/block.h | 7 +++-
tests/qemu-iotests/172.out | 76 ++
4 files changed, 90 insertions(+), 3 deletions(-)
diff --git a/block/accounting.c b/block/accounting.c
index 6b300c5129..2829745377 100644
--- a/block/accounting.c
+++ b/block/accounting.c
@@ -38,6 +38,8 @@ void block_acct_init(BlockAcctStats *stats)
if (qtest_enabled()) {
clock_type = QEMU_CLOCK_VIRTUAL;
}
+stats->account_invalid = true;
+stats->account_failed = true;
}
static bool bool_from_onoffauto(OnOffAuto val, bool def)
@@ -57,8 +59,10 @@ static bool bool_from_onoffauto(OnOffAuto val, bool def)
void block_acct_setup(BlockAcctStats *stats, enum OnOffAuto account_invalid,
enum OnOffAuto account_failed)
{
-stats->account_invalid = bool_from_onoffauto(account_invalid, true);
-stats->account_failed = bool_from_onoffauto(account_failed, true);
+stats->account_invalid = bool_from_onoffauto(account_invalid,
+ stats->account_invalid);
+stats->account_failed = bool_from_onoffauto(account_failed,
+stats->account_failed);
}
void block_acct_cleanup(BlockAcctStats *stats)
diff --git a/hw/block/block.c b/hw/block/block.c
index 04279166ee..f9c4fe6767 100644
--- a/hw/block/block.c
+++ b/hw/block/block.c
@@ -205,6 +205,8 @@ bool blkconf_apply_backend_options(BlockConf *conf, bool
readonly,
blk_set_enable_write_cache(blk, wce);
blk_set_on_error(blk, rerror, werror);
+block_acct_setup(blk_get_stats(blk), conf->account_invalid,
+ conf->account_failed);
return true;
}
diff --git a/include/hw/block/block.h b/include/hw/block/block.h
index 5902c0440a..15fff66435 100644
--- a/include/hw/block/block.h
+++ b/include/hw/block/block.h
@@ -31,6 +31,7 @@ typedef struct BlockConf {
uint32_t lcyls, lheads, lsecs;
OnOffAuto wce;
bool share_rw;
+OnOffAuto account_invalid, account_failed;
BlockdevOnError rerror;
BlockdevOnError werror;
} BlockConf;
@@ -61,7 +62,11 @@ static inline unsigned int get_physical_block_exp(BlockConf
*conf)
_conf.discard_granularity, -1), \
DEFINE_PROP_ON_OFF_AUTO("write-cache", _state, _conf.wce, \
ON_OFF_AUTO_AUTO), \
-DEFINE_PROP_BOOL("share-rw", _state, _conf.share_rw, false)
+DEFINE_PROP_BOOL("share-rw", _state, _conf.share_rw, false),\
+DEFINE_PROP_ON_OFF_AUTO("account-invalid", _state, \
+_conf.account_invalid, ON_OFF_AUTO_AUTO), \
+DEFINE_PROP_ON_OFF_AUTO("account-failed", _state, \
+_conf.account_failed, ON_OFF_AUTO_AUTO)
#define DEFINE_BLOCK_PROPERTIES(_state, _conf) \
DEFINE_PROP_DRIVE("drive", _state, _conf.blk), \
diff --git a/tests/qemu-iotests/172.out b/tests/qemu-iotests/172.out
index 9479b92185..07eebf3583 100644
--- a/tests/qemu-iotests/172.out
+++ b/tests/qemu-iotests/172.out
@@ -28,6 +28,8 @@ Formatting 'TEST_DIR/t.IMGFMT.3', fmt=IMGFMT size=737280
discard_granularity = 4294967295 (4 GiB)
write-cache = "auto"
share-rw = false
+account-invalid = "auto"
+account-failed = "auto"
drive-type = "288"
@@ -55,6 +57,8 @@ Testing: -fda TEST_DIR/t.qcow2
discard_granularity = 4294967295 (4 GiB)
write-cache = "auto"
share-rw = false
+account-invalid = "auto"
+account-failed = "auto"
drive-type = "144"
floppy0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
Attached to: /machine/unattached/devi
[PATCH 38/51] tests/qtest: {ahci,ide}-test: Open file in binary mode
From: Xuzhou Cheng By default Windows opens file in text mode, while a POSIX compliant implementation treats text files and binary files the same. The fopen() 'mode' string can include the letter 'b' to indicate binary mode shall be used. POSIX spec says the character 'b' shall have no effect, but is allowed for ISO C standard conformance. Let's add the letter 'b' which works on both POSIX and Windows. Similar situation applies to the open() 'flags' where O_BINARY is used for binary mode. Signed-off-by: Xuzhou Cheng Signed-off-by: Bin Meng --- tests/qtest/ahci-test.c | 2 +- tests/qtest/ide-test.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/qtest/ahci-test.c b/tests/qtest/ahci-test.c index bce9ff770c..be11508c75 100644 --- a/tests/qtest/ahci-test.c +++ b/tests/qtest/ahci-test.c @@ -1453,7 +1453,7 @@ static int prepare_iso(size_t size, unsigned char **buf, char **name) * Close the file and reopen it. */ close(fd); -fd = open(cdrom_path, O_WRONLY); +fd = open(cdrom_path, O_WRONLY | O_BINARY); g_assert(fd != -1); #endif diff --git a/tests/qtest/ide-test.c b/tests/qtest/ide-test.c index c5cad6c0be..ee03dea4fa 100644 --- a/tests/qtest/ide-test.c +++ b/tests/qtest/ide-test.c @@ -892,7 +892,7 @@ static void cdrom_pio_impl(int nblocks) /* Prepopulate the CDROM with an interesting pattern */ generate_pattern(pattern, patt_len, ATAPI_BLOCK_SIZE); -fh = fopen(tmp_path, "w+"); +fh = fopen(tmp_path, "wb+"); ret = fwrite(pattern, ATAPI_BLOCK_SIZE, patt_blocks, fh); g_assert_cmpint(ret, ==, patt_blocks); fclose(fh); @@ -993,7 +993,7 @@ static void test_cdrom_dma(void) prdt[0].size = cpu_to_le32(len | PRDT_EOT); generate_pattern(pattern, ATAPI_BLOCK_SIZE * 16, ATAPI_BLOCK_SIZE); -fh = fopen(tmp_path, "w+"); +fh = fopen(tmp_path, "wb+"); ret = fwrite(pattern, ATAPI_BLOCK_SIZE, 16, fh); g_assert_cmpint(ret, ==, 16); fclose(fh); -- 2.34.1
[PATCH 46/51] tests/qtest: libqtest: Replace the call to close a socket with closesocket()
From: Bin Meng
close() is a *nix function. It works on any file descriptor, and
sockets in *nix are an example of a file descriptor.
closesocket() is a Windows-specific function, which works only
specifically with sockets. Sockets on Windows do not use *nix-style
file descriptors, and socket() returns a handle to a kernel object
instead, so it must be closed with closesocket().
In QEMU there is already a logic to handle such platform difference
in os-posix.h and os-win32.h, that:
* closesocket maps to close on POSIX
* closesocket maps to a wrapper that calls the real closesocket()
on Windows
Replace the call to close a socket with closesocket() instead.
Signed-off-by: Bin Meng
---
tests/qtest/libqtest.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 99e52ff571..918f4657ed 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -115,7 +115,7 @@ static int socket_accept(int sock)
(void *)&timeout, sizeof(timeout))) {
fprintf(stderr, "%s failed to set SO_RCVTIMEO: %s\n",
__func__, strerror(errno));
-close(sock);
+closesocket(sock);
return -1;
}
@@ -126,7 +126,7 @@ static int socket_accept(int sock)
if (ret == -1) {
fprintf(stderr, "%s failed: %s\n", __func__, strerror(errno));
}
-close(sock);
+closesocket(sock);
return ret;
}
@@ -512,8 +512,8 @@ void qtest_quit(QTestState *s)
qtest_remove_abrt_handler(s);
qtest_kill_qemu(s);
-close(s->fd);
-close(s->qmp_fd);
+closesocket(s->fd);
+closesocket(s->qmp_fd);
g_string_free(s->rx, true);
for (GList *it = s->pending_events; it != NULL; it = it->next) {
--
2.34.1
[PATCH 39/51] tests/qtest: virtio-net-failover: Disable migration tests for win32
From: Xuzhou Cheng
These tests use the exec migration protocol, which is unsupported
on Windows as of today. Disable these tests for now.
Signed-off-by: Xuzhou Cheng
Signed-off-by: Bin Meng
---
tests/qtest/virtio-net-failover.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/tests/qtest/virtio-net-failover.c
b/tests/qtest/virtio-net-failover.c
index 443ee56de9..4a809590bf 100644
--- a/tests/qtest/virtio-net-failover.c
+++ b/tests/qtest/virtio-net-failover.c
@@ -588,6 +588,7 @@ static void test_hotplug_2_reverse(void)
machine_stop(qts);
}
+#ifndef _WIN32
static QDict *migrate_status(QTestState *qts)
{
QDict *resp, *ret;
@@ -1827,6 +1828,7 @@ static void test_multi_in(gconstpointer opaque)
machine_stop(qts);
}
+#endif /* _WIN32 */
int main(int argc, char **argv)
{
@@ -1857,7 +1859,11 @@ int main(int argc, char **argv)
qtest_add_func("failover-virtio-net/hotplug/2_reverse",
test_hotplug_2_reverse);
-/* migration tests */
+#ifndef _WIN32
+/*
+ * These migration tests cases use the exec migration protocol,
+ * which is unsupported on Windows.
+ */
qtest_add_data_func("failover-virtio-net/migrate/on/out", tmpfile,
test_migrate_out);
qtest_add_data_func("failover-virtio-net/migrate/on/in", tmpfile,
@@ -1886,6 +1892,7 @@ int main(int argc, char **argv)
tmpfile, test_multi_out);
qtest_add_data_func("failover-virtio-net/migrate/multi/in",
tmpfile, test_multi_in);
+#endif /* _WIN32 */
ret = g_test_run();
--
2.34.1
[PATCH 42/51] hw/ppc: spapr: Use qemu_vfree() to free spapr->htab
From: Xuzhou Cheng
spapr->htab is allocated by qemu_memalign(), hence we should use
qemu_vfree() to free it.
Fixes: c5f54f3e31bf ("pseries: Move hash page table allocation to reset time")
Fixes: b4db54132ffe ("target/ppc: Implement H_REGISTER_PROCESS_TABLE H_CALL"")
Signed-off-by: Xuzhou Cheng
Signed-off-by: Bin Meng
---
hw/ppc/spapr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index bc9ba6e6dc..4034f4d130 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1522,7 +1522,7 @@ int spapr_hpt_shift_for_ramsize(uint64_t ramsize)
void spapr_free_hpt(SpaprMachineState *spapr)
{
-g_free(spapr->htab);
+qemu_vfree(spapr->htab);
spapr->htab = NULL;
spapr->htab_shift = 0;
close_htab_fd(spapr);
--
2.34.1
Re: [PATCH v7 01/14] mm: Add F_SEAL_AUTO_ALLOCATE seal to memfd
On Tue, Aug 23, 2022 at 09:36:57AM +0200, David Hildenbrand wrote: > On 18.08.22 01:41, Kirill A. Shutemov wrote: > > On Fri, Aug 05, 2022 at 07:55:38PM +0200, Paolo Bonzini wrote: > >> On 7/21/22 11:44, David Hildenbrand wrote: > >>> > >>> Also, I*think* you can place pages via userfaultfd into shmem. Not > >>> sure if that would count "auto alloc", but it would certainly bypass > >>> fallocate(). > >> > >> Yeah, userfaultfd_register would probably have to forbid this for > >> F_SEAL_AUTO_ALLOCATE vmas. Maybe the memfile_node can be reused for this, > >> adding a new MEMFILE_F_NO_AUTO_ALLOCATE flags? Then userfault_register > >> would do something like memfile_node_get_flags(vma->vm_file) and check the > >> result. > > > > I donno, memory allocation with userfaultfd looks pretty intentional to > > me. Why would F_SEAL_AUTO_ALLOCATE prevent it? > > > > Can't we say the same about a write()? > > > Maybe we would need it in the future for post-copy migration or something? > > > > Or existing practises around userfaultfd touch memory randomly and > > therefore incompatible with F_SEAL_AUTO_ALLOCATE intent? > > > > Note, that userfaultfd is only relevant for shared memory as it requires > > VMA which we don't have for MFD_INACCESSIBLE. > > This feature (F_SEAL_AUTO_ALLOCATE) is independent of all the lovely > encrypted VM stuff, so it doesn't matter how it relates to MFD_INACCESSIBLE. Right, this patch is for normal user accssible fd. In KVM this flag is expected to be set on the shared part of the memslot, while all other patches in this series are for private part of the memslot. Private memory doesn't have this need because it's totally inaccissible from userspace so no chance for userspace to write to the fd and cause allocation by accident. While for shared memory, malicious/buggy guest OS may cause userspace to write to any range of the shared fd and cause memory allocation, even that range should the private memory not the shared memory be visible to guest OS. Chao > > -- > Thanks, > > David / dhildenb >
[PATCH 44/51] tests/qtest: microbit-test: Fix socket access for win32
From: Bin Meng
Sockets on Windows do not use *nix-style file descriptors, so
write()/read()/close() do not work on Windows.
Switch over to use send()/recv()/closesocket() which work with
sockets on all platforms.
Signed-off-by: Bin Meng
---
tests/qtest/microbit-test.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/tests/qtest/microbit-test.c b/tests/qtest/microbit-test.c
index b71daae9a9..4bc267020b 100644
--- a/tests/qtest/microbit-test.c
+++ b/tests/qtest/microbit-test.c
@@ -51,7 +51,7 @@ static void uart_rw_to_rxd(QTestState *qts, int sock_fd,
const char *in,
{
int i, in_len = strlen(in);
-g_assert_true(write(sock_fd, in, in_len) == in_len);
+g_assert_true(send(sock_fd, in, in_len, 0) == in_len);
for (i = 0; i < in_len; i++) {
g_assert_true(uart_wait_for_event(qts, NRF51_UART_BASE +
A_UART_RXDRDY));
@@ -77,7 +77,7 @@ static void test_nrf51_uart(void)
char s[10];
QTestState *qts = qtest_init_with_serial("-M microbit", &sock_fd);
-g_assert_true(write(sock_fd, "c", 1) == 1);
+g_assert_true(send(sock_fd, "c", 1, 0) == 1);
g_assert_cmphex(qtest_readl(qts, NRF51_UART_BASE + A_UART_RXD), ==, 0x00);
qtest_writel(qts, NRF51_UART_BASE + A_UART_ENABLE, 0x04);
@@ -97,17 +97,17 @@ static void test_nrf51_uart(void)
qtest_writel(qts, NRF51_UART_BASE + A_UART_STARTTX, 0x01);
uart_w_to_txd(qts, "d");
-g_assert_true(read(sock_fd, s, 10) == 1);
+g_assert_true(recv(sock_fd, s, 10, 0) == 1);
g_assert_cmphex(s[0], ==, 'd');
qtest_writel(qts, NRF51_UART_BASE + A_UART_SUSPEND, 0x01);
qtest_writel(qts, NRF51_UART_BASE + A_UART_TXD, 'h');
qtest_writel(qts, NRF51_UART_BASE + A_UART_STARTTX, 0x01);
uart_w_to_txd(qts, "world");
-g_assert_true(read(sock_fd, s, 10) == 5);
+g_assert_true(recv(sock_fd, s, 10, 0) == 5);
g_assert_true(memcmp(s, "world", 5) == 0);
-close(sock_fd);
+closesocket(sock_fd);
qtest_quit(qts);
}
--
2.34.1
[PATCH 45/51] tests/qtest: prom-env-test: Use double quotes to pass the prom-env option
From: Bin Meng
Single quotes like -prom-env 'nvramrc=cafec0de 4000 l!' in the arguments
are not removed in the Windows environment before it is passed to the
QEMU executable. Such argument causes a failure in the QEMU prom-env
option parser codes.
Change to use double quotes which works fine on all platforms.
Signed-off-by: Bin Meng
---
tests/qtest/prom-env-test.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/qtest/prom-env-test.c b/tests/qtest/prom-env-test.c
index c2b0448e55..39ccb59797 100644
--- a/tests/qtest/prom-env-test.c
+++ b/tests/qtest/prom-env-test.c
@@ -58,8 +58,8 @@ static void test_machine(const void *machine)
" -machine " PSERIES_DEFAULT_CAPABILITIES;
}
-qts = qtest_initf("-M %s -accel tcg %s -prom-env 'use-nvramrc?=true' "
- "-prom-env 'nvramrc=%x %x l!' ", (const char *)machine,
+qts = qtest_initf("-M %s -accel tcg %s -prom-env \"use-nvramrc?=true\" "
+ "-prom-env \"nvramrc=%x %x l!\" ", (const char *)machine,
extra_args, MAGIC, ADDRESS);
check_guest_memory(qts);
qtest_quit(qts);
--
2.34.1
[PATCH 47/51] tests/qtest: libqtest: Correct the timeout unit of blocking receive calls for win32
From: Bin Meng
Some qtest cases don't get response from the the QEMU executable
under test in time on Windows. It turns out that the socket receive
call got timeout before it receive the complete response.
The timeout value is supposed to be set to 50 seconds via the
setsockopt() call, but there is a difference among platforms.
The timeout unit of blocking receive calls is measured in
seconds on non-Windows platforms but milliseconds on Windows.
Signed-off-by: Bin Meng
---
tests/qtest/libqtest.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 918f4657ed..7b41971347 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -36,13 +36,14 @@
#include "qapi/qmp/qstring.h"
#define MAX_IRQ 256
-#define SOCKET_TIMEOUT 50
#ifndef _WIN32
+# define SOCKET_TIMEOUT 50
# define CMD_EXEC "exec "
# define DEV_STDERR "/dev/fd/2"
# define DEV_NULL "/dev/null"
#else
+# define SOCKET_TIMEOUT 5
# define CMD_EXEC ""
# define DEV_STDERR "2"
# define DEV_NULL "nul"
@@ -108,8 +109,16 @@ static int socket_accept(int sock)
struct sockaddr_un addr;
socklen_t addrlen;
int ret;
+/*
+ * timeout unit of blocking receive calls is different among platfoms.
+ * It's in seconds on non-Windows platforms but milliseconds on Windows.
+ */
+#ifndef _WIN32
struct timeval timeout = { .tv_sec = SOCKET_TIMEOUT,
.tv_usec = 0 };
+#else
+DWORD timeout = SOCKET_TIMEOUT;
+#endif
if (setsockopt(sock, SOL_SOCKET, SO_RCVTIMEO,
(void *)&timeout, sizeof(timeout))) {
--
2.34.1
Re: [PATCH 4/4] hw/nvme: add MSI-x mask handlers for irqfd
On Aug 23 22:43, Jinhao Fan wrote: > On 8/16/2022 6:46 PM, Klaus Jensen wrote: > > Did qtest work out for you for testing? If so, it would be nice to add a > > simple test case as well. > > Since MSI-x masking handlers are only implemented for IO queues, if we want > to use qtest we need to implement utilities for controller initialization > and IO queue creation. After that we can actually test the MSI-x masking > feature. Although we may reuse some code from virtio's tests, that is still > a large amount of work. > > Is it possible to get this patch merged without testing? If not, I guess > I'll have to take the hard work to implement something like > qtest/libqos/nvme.c > I'm not too happy about code that is completely untestable (worse, right now it is actually not even runnable). What are the implications if we drop it? That is, if we go back to your version that did not include this? If it doesnt impact the kvm irqchip logic, then I'd rather that we rip it out and leave the device without masking/unmasking support, keeping irqfd support as an experimental feature until we can sort this out. signature.asc Description: PGP signature
Re: [PATCH 01/51] tests/qtest: Use g_setenv()
On 24/08/2022 11.39, Bin Meng wrote: From: Bin Meng Windows does not provide a setenv() API, but glib does. Replace setenv() call with the glib version. Signed-off-by: Bin Meng --- tests/qtest/fuzz/generic_fuzz.c | 8 tests/qtest/libqtest.c | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) Reviewed-by: Thomas Huth
[PATCH 1/2] block: pass OnOffAuto instead of bool to block_acct_setup()
We would have one more place for block_acct_setup() calling, which should
not corrupt original value.
Signed-off-by: Denis V. Lunev
Reviewed-by: Vladimir Sementsov-Ogievskiy
CC: Peter Krempa
CC: Markus Armbruster
CC: John Snow
CC: Kevin Wolf
CC: Hanna Reitz
---
block/accounting.c | 22 ++
blockdev.c | 17 ++---
include/block/accounting.h | 6 +++---
3 files changed, 35 insertions(+), 10 deletions(-)
diff --git a/block/accounting.c b/block/accounting.c
index 2030851d79..6b300c5129 100644
--- a/block/accounting.c
+++ b/block/accounting.c
@@ -40,11 +40,25 @@ void block_acct_init(BlockAcctStats *stats)
}
}
-void block_acct_setup(BlockAcctStats *stats, bool account_invalid,
- bool account_failed)
+static bool bool_from_onoffauto(OnOffAuto val, bool def)
{
-stats->account_invalid = account_invalid;
-stats->account_failed = account_failed;
+switch (val) {
+case ON_OFF_AUTO_AUTO:
+return def;
+case ON_OFF_AUTO_ON:
+return true;
+case ON_OFF_AUTO_OFF:
+return false;
+default:
+abort();
+}
+}
+
+void block_acct_setup(BlockAcctStats *stats, enum OnOffAuto account_invalid,
+ enum OnOffAuto account_failed)
+{
+stats->account_invalid = bool_from_onoffauto(account_invalid, true);
+stats->account_failed = bool_from_onoffauto(account_failed, true);
}
void block_acct_cleanup(BlockAcctStats *stats)
diff --git a/blockdev.c b/blockdev.c
index 9230888e34..392d9476e6 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -455,6 +455,17 @@ static void extract_common_blockdev_options(QemuOpts
*opts, int *bdrv_flags,
}
}
+static OnOffAuto account_get_opt(QemuOpts *opts, const char *name)
+{
+if (!qemu_opt_find(opts, name)) {
+return ON_OFF_AUTO_AUTO;
+}
+if (qemu_opt_get_bool(opts, name, true)) {
+return ON_OFF_AUTO_ON;
+}
+return ON_OFF_AUTO_OFF;
+}
+
/* Takes the ownership of bs_opts */
static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
Error **errp)
@@ -462,7 +473,7 @@ static BlockBackend *blockdev_init(const char *file, QDict
*bs_opts,
const char *buf;
int bdrv_flags = 0;
int on_read_error, on_write_error;
-bool account_invalid, account_failed;
+OnOffAuto account_invalid, account_failed;
bool writethrough, read_only;
BlockBackend *blk;
BlockDriverState *bs;
@@ -496,8 +507,8 @@ static BlockBackend *blockdev_init(const char *file, QDict
*bs_opts,
/* extract parameters */
snapshot = qemu_opt_get_bool(opts, "snapshot", 0);
-account_invalid = qemu_opt_get_bool(opts, "stats-account-invalid", true);
-account_failed = qemu_opt_get_bool(opts, "stats-account-failed", true);
+account_invalid = account_get_opt(opts, "stats-account-invalid");
+account_failed = account_get_opt(opts, "stats-account-failed");
writethrough = !qemu_opt_get_bool(opts, BDRV_OPT_CACHE_WB, true);
diff --git a/include/block/accounting.h b/include/block/accounting.h
index 878b4c3581..b9caad60d5 100644
--- a/include/block/accounting.h
+++ b/include/block/accounting.h
@@ -27,7 +27,7 @@
#include "qemu/timed-average.h"
#include "qemu/thread.h"
-#include "qapi/qapi-builtin-types.h"
+#include "qapi/qapi-types-common.h"
typedef struct BlockAcctTimedStats BlockAcctTimedStats;
typedef struct BlockAcctStats BlockAcctStats;
@@ -100,8 +100,8 @@ typedef struct BlockAcctCookie {
} BlockAcctCookie;
void block_acct_init(BlockAcctStats *stats);
-void block_acct_setup(BlockAcctStats *stats, bool account_invalid,
- bool account_failed);
+void block_acct_setup(BlockAcctStats *stats, enum OnOffAuto account_invalid,
+ enum OnOffAuto account_failed);
void block_acct_cleanup(BlockAcctStats *stats);
void block_acct_add_interval(BlockAcctStats *stats, unsigned interval_length);
BlockAcctTimedStats *block_acct_interval_next(BlockAcctStats *stats,
--
2.32.0
[RFC] hw/net/vmxnet3: allow VMXNET3_MAX_MTU itself as a value
Fixes: d05dcd94ae ("net: vmxnet3: validate configuration values during activate
(CVE-2021-20203)")
Signed-off-by: Fiona Ebner
---
I'm not familiar with this code, so really I'm asking: is the change
justified?
I tested the change and it seems to work, but I only have some rough
rationale for it, which is also why there's no commit message yet.
In the Linux kernel's net/core/dev.c, in dev_validate_mtu(), the upper
limit itself is a valid value:
if (dev->max_mtu > 0 && new_mtu > dev->max_mtu) {
NL_SET_ERR_MSG(extack, "mtu greater than device maximum");
return -EINVAL;
}
and AFAICT in the case of the vmxnet3 driver, max_mtu is set to
VMXNET3_MAX_MTU (as defined in the kernel, which is 9000, same as in
QEMU).
Reported by one of our users running into the failing assert():
https://forum.proxmox.com/threads/114011/#post-492916
hw/net/vmxnet3.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index 0b7acf7f89..a2037583bf 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -1441,7 +1441,7 @@ static void vmxnet3_activate_device(VMXNET3State *s)
vmxnet3_setup_rx_filtering(s);
/* Cache fields from shared memory */
s->mtu = VMXNET3_READ_DRV_SHARED32(d, s->drv_shmem, devRead.misc.mtu);
-assert(VMXNET3_MIN_MTU <= s->mtu && s->mtu < VMXNET3_MAX_MTU);
+assert(VMXNET3_MIN_MTU <= s->mtu && s->mtu <= VMXNET3_MAX_MTU);
VMW_CFPRN("MTU is %u", s->mtu);
s->max_rx_frags =
--
2.30.2
Re: [PATCH v2 07/24] virtio-pci: support queue enable
在 2022/8/24 16:59, Jason Wang 写道:
在 2022/8/23 16:20, Kangjie Xu 写道:
在 2022/8/23 15:44, Jason Wang 写道:
在 2022/8/16 09:06, Kangjie Xu 写道:
PCI devices support vq enable.
Nit: it might be "support device specific vq enable"
Get it.
Based on this function, the driver can re-enable the virtqueue
after the
virtqueue is reset.
Signed-off-by: Kangjie Xu
Signed-off-by: Xuan Zhuo
---
hw/virtio/virtio-pci.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index ec8e92052f..3d560e45ad 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1335,6 +1335,7 @@ static void virtio_pci_common_write(void
*opaque, hwaddr addr,
proxy->vqs[vdev->queue_sel].avail[0],
((uint64_t)proxy->vqs[vdev->queue_sel].used[1]) << 32 |
proxy->vqs[vdev->queue_sel].used[0]);
+ virtio_queue_enable(vdev, vdev->queue_sel);
proxy->vqs[vdev->queue_sel].enabled = 1;
proxy->vqs[vdev->queue_sel].reset = 0;
Any reason we do it before the assignment of 1? It probably means
the device specific method can't depend on virtio_queue_enabled()?
Thanks
Sorry, I don't get why device specific method can't depend on
virtio_queue_enabled().
I meant if the device specific method call virtio_queue_enabled() it
will return false in this case, is this intended?
Yes, I intend it to behave in this way.
Before virtio_queue_enable() is done, virtqueue should always be not
ready and disabled.
Otherwise, If we put it after the assignment of enabled to 1, the
virtqueue may be accessed illegally and may cause panic, because the
virtqueue is still being intialized and being configured.
How? Shouldn't we make transport ready before making device
virtqueue(device) ready?
Thanks
I am not experienced in this field, could you tell me why we should make
the transport ready first?
I make the transport ready later than making device ready for two aspects:
1. In QEMU, the virtio_queue_enabled() is used only when we start the
device/queue pair (vhost_dev_start_one), or reading
VIRTIO_PCI_COMMON_Q_ENABLE. These two operations and resetting the queue
will *be synchronized* using iothread lock, so we do not need to worry
about the case currently.
2. Suppose we use virtio_queue_enabled() or access the enabled status
asynchronously, and we make the transport ready first.
After enabled set to true, and before virtio_queue_enable() is finished,
somewhere that call virtio_queue_enabled() or access the enabled
status of VirtIOPCIQueue. Then the caller will consider the virtqueue as
enabled(enabled = true in VirtIOPCIQueue). The caller might access the
virtqueue(access avail ring / desc table). But I think *the access here
is illegal* because the virtqueue might still be *unintialized* status.
Thus, from my perspective, to prevent illegal access, we need to make
transport ready after virtio_queue_enable().
Thanks
Thanks
} else {
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On Sun, Aug 21, 2022 at 11:27:44AM +0100, Matthew Wilcox wrote: > On Thu, Aug 18, 2022 at 08:00:41PM -0700, Hugh Dickins wrote: > > tmpfs and hugetlbfs and page cache are designed around sharing memory: > > TDX is designed around absolutely not sharing memory; and the further > > uses which Sean foresees appear not to need it as page cache either. > > > > Except perhaps for page migration reasons. It's somewhat incidental, > > but of course page migration knows how to migrate page cache, so > > masquerading as page cache will give a short cut to page migration, > > when page migration becomes at all possible. > > I haven't read the patch series, and I'm not taking a position one way > or the other on whether this is better implemented as a shmem addition > or a shim that asks shmem for memory. Page migration can be done for > driver memory by using PageMovable. I just rewrote how it works, so > the details are top of my mind at the moment if anyone wants something > explained. Commit 68f2736a8583 is the key one to look at. Thanks Matthew. That is helpful to understand the current code. Chao
Re: [PATCH] virtiofsd: use g_date_time_get_microsecond to get subsecond
* Yusuke Okada (yokada@gmail.com) wrote:
> From: Yusuke Okada
>
> The "%f" specifier in g_date_time_format() is only available in glib
> 2.65.2 or later. If combined with older glib, the function returns null
> and the timestamp displayed as "(null)".
Well spotted; thanks for the patch.
I notice there's also a use in rocker and qga (Copying Dan and
Marc-Andre in who added them)
> For backward compatibility, g_date_time_get_microsecond should be used
> to retrieve subsecond.
>
> In this patch the g_date_time_format() leaves subsecond field as "%06d"
> and let next snprintf to format with g_date_time_get_microsecond.
>
> Signed-off-by: Yusuke Okada
Reviewed-by: Dr. David Alan Gilbert
> ---
> tools/virtiofsd/passthrough_ll.c | 7 +--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/tools/virtiofsd/passthrough_ll.c
> b/tools/virtiofsd/passthrough_ll.c
> index 371a7bead6..20f0f41f99 100644
> --- a/tools/virtiofsd/passthrough_ll.c
> +++ b/tools/virtiofsd/passthrough_ll.c
> @@ -4185,6 +4185,7 @@ static void setup_nofile_rlimit(unsigned long
> rlimit_nofile)
> static void log_func(enum fuse_log_level level, const char *fmt, va_list ap)
> {
> g_autofree char *localfmt = NULL;
> +char buf[64];
>
> if (current_log_level < level) {
> return;
> @@ -4197,9 +4198,11 @@ static void log_func(enum fuse_log_level level, const
> char *fmt, va_list ap)
> fmt);
> } else {
> g_autoptr(GDateTime) now = g_date_time_new_now_utc();
> -g_autofree char *nowstr = g_date_time_format(now, "%Y-%m-%d
> %H:%M:%S.%f%z");
> +g_autofree char *nowstr = g_date_time_format(now,
> + "%Y-%m-%d %H:%M:%S.%%06d%z");
> +snprintf(buf, 64, nowstr, g_date_time_get_microsecond(now));
> localfmt = g_strdup_printf("[%s] [ID: %08ld] %s",
> - nowstr, syscall(__NR_gettid), fmt);
> + buf, syscall(__NR_gettid), fmt);
> }
> fmt = localfmt;
> }
> --
> 2.31.1
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
