Re: Out-of-Process Device Emulation session at KVM Forum 2020
On 2020/11/2 下午6:13, Stefan Hajnoczi wrote: On Mon, Nov 02, 2020 at 10:51:18AM +0800, Jason Wang wrote: On 2020/10/30 下午9:15, Stefan Hajnoczi wrote: On Fri, Oct 30, 2020 at 12:08 PM Jason Wang wrote: On 2020/10/30 下午7:13, Stefan Hajnoczi wrote: On Fri, Oct 30, 2020 at 9:46 AM Jason Wang wrote: On 2020/10/30 下午2:21, Stefan Hajnoczi wrote: On Fri, Oct 30, 2020 at 3:04 AM Alex Williamson wrote: It's great to revisit ideas, but proclaiming a uAPI is bad solely because the data transfer is opaque, without defining why that's bad, evaluating the feasibility and implementation of defining a well specified data format rather than protocol, including cross-vendor support, or proposing any sort of alternative is not so helpful imo. The migration approaches in VFIO and vDPA/vhost were designed for different requirements and I think this is why there are different perspectives on this. Here is a comparison and how VFIO could be extended in the future. I see 3 levels of device state compatibility: 1. The device cannot save/load state blobs, instead userspace fetches and restores specific values of the device's runtime state (e.g. last processed ring index). This is the vhost approach. 2. The device can save/load state in a standard format. This is similar to #1 except that there is a single read/write blob interface instead of fine-grained get_FOO()/set_FOO() interfaces. This approach pushes the migration state parsing into the device so that userspace doesn't need knowledge of every device type. With this approach it is possible for a device from vendor A to migrate to a device from vendor B, as long as they both implement the same standard migration format. The limitation of this approach is that vendor-specific state cannot be transferred. 3. The device can save/load opaque blobs. This is the initial VFIO approach. I still don't get why it must be opaque. If the device state format needs to be in the VMM then each device needs explicit enablement in each VMM (QEMU, cloud-hypervisor, etc). Let's invert the question: why does the VMM need to understand the device state of a _passthrough_ device? For better manageability, compatibility and debug-ability. If we depends on a opaque structure, do we encourage device to implement its own migration protocol? It would be very challenge. For VFIO in the kernel, I suspect a uAPI that may result a opaque data to be read or wrote from guest violates the Linux uAPI principle. It will be very hard to maintain uABI or even impossible. It looks to me VFIO is the first subsystem that is trying to do this. I think our concepts of uAPI are different. The uAPI of read(2) and write(2) does not define the structure of the data buffers. VFIO device regions are exactly the same, the structure of the data is not defined by the kernel uAPI. I think we're talking about different things. It's not about the data structure, it's about whether to data that reads from kernel can be understood by userspace. Maybe microcode and firmware loading is an example we agree on? I think not. They are bytecodes that have 1) strict ABI definitions 2) understood by userspace No, they can be proprietary formats that neither the Linux kernel nor userspace can parse. For example, look at linux-firmware (https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/about/) it's just a collection of binary blobs. The format is not necessarily public. The only restriction on that repo is that the binary blob must be redistributable and users must be allowed to run them (i.e. proprietary licenses can be used). I think not. Obviously each firmware should have its own ABI no matter whether its public or proprietary. For proprietary firmware, it should be understood by the proprietary userspace counterpart. Or look at other passthrough device interfaces like /dev/i2c or libusb. They expose data to userspace without requiring a defined format. It's the same as VFIO. Again, it should have an ABI there (either device or spec) no matter whether or not it's a transport layer. And there will be an endpoint in the userspace know all the format. In addition, look at kernel uAPIs where userspace acts simply as a data transport for opaque data (e.g. where a userspace helper facilitates communication but has no visibility of the data). I imagine that memory encryption relies on this because the host kernel and userspace do not have access to encrypted memory or associated state - but they need to help migrate them to other hosts. Which uAPI do you mean here? I hope these examples show that such APIs don't pose a problem for the Linux uAPI and are already in use. VFIO device state isn't doing anything new here. I feel that you tried to explain "why it can be" but not "why it must be". Trying to find one or two subsystems that have opaque uAPI without ABI (though I suspect there will be one) may not convince here. Thanks A device from vendor A
[PATCH] target/microblaze: Fix possible array out of bounds in mmu_write()
The size of env->mmu.regs is 3, but the range of 'rn' is [0, 5].
To avoid data access out of bounds, only if 'rn' is less than 3, we
can print env->mmu.regs[rn]. In other cases, we can print
env->mmu.regs[MMU_R_TLBX].
Reported-by: Euler Robot
Signed-off-by: Alex Chen
---
target/microblaze/mmu.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/target/microblaze/mmu.c b/target/microblaze/mmu.c
index 1dbbb271c4..917ad6d69e 100644
--- a/target/microblaze/mmu.c
+++ b/target/microblaze/mmu.c
@@ -234,7 +234,8 @@ void mmu_write(CPUMBState *env, bool ext, uint32_t rn,
uint32_t v)
unsigned int i;
qemu_log_mask(CPU_LOG_MMU,
- "%s rn=%d=%x old=%x\n", __func__, rn, v, env->mmu.regs[rn]);
+ "%s rn=%d=%x old=%x\n", __func__, rn, v,
+ rn < 3 ? env->mmu.regs[rn] : env->mmu.regs[MMU_R_TLBX]);
if (cpu->cfg.mmu < 2 || !cpu->cfg.mmu_tlb_access) {
qemu_log_mask(LOG_GUEST_ERROR, "MMU access on MMU-less system\n");
--
2.19.1
Re: [PULL 1/1] linux-user: Support futex_time64
Le 02/11/2020 à 19:15, Peter Maydell a écrit :
> On Mon, 30 Mar 2020 at 11:31, Laurent Vivier wrote:
>>
>> From: Alistair Francis
>>
>> Add support for host and target futex_time64. If futex_time64 exists on
>> the host we try that first before falling back to the standard futex
>> syscall.
>
> Hi; I dunno why Coverity's only just noticed this, but in
> CID 1432339 it points out:
>
>> +#if defined(TARGET_NR_futex_time64)
>> +static int do_futex_time64(target_ulong uaddr, int op, int val,
>> target_ulong timeout,
>> + target_ulong uaddr2, int val3)
>> +{
>> +struct timespec ts, *pts;
>> +int base_op;
>> +
>> +/* ??? We assume FUTEX_* constants are the same on both host
>> + and target. */
>> +#ifdef FUTEX_CMD_MASK
>> +base_op = op & FUTEX_CMD_MASK;
>> +#else
>> +base_op = op;
>> +#endif
>> +switch (base_op) {
>> +case FUTEX_WAIT:
>> +case FUTEX_WAIT_BITSET:
>> +if (timeout) {
>> +pts =
>> +target_to_host_timespec64(pts, timeout);
>
> ...that here we call target_to_host_timespec64(), which can
> fail with -TARGET_EFAULT, but (unlike all the other times we call
> the function) we aren't checking its return value.
> Is there missing error handling code here ?
>
I think the code is like that because this is a cut of function
do_futex() witl "s/timespec/timespec64/".
And yes I think we should check for the return value.
I'm going to fix that.
Thanks,
Laurent
Re: [PATCH for-5.2] tcg: Remove assert from set_jmp_reset_offset
On 11/3/20 4:39 AM, Richard Henderson wrote:
> The range check done here is done later, more appropriately,
> at the end of tcg_gen_code.
Maybe mention commit 6e6c4efed99 ("tcg: Restart after TB code generation
overflow")? (no need to repost).
> There, a failing range check
> results in a returned error code, which causes the TB to be
> restarted at half the size.
>
> Reported-by: Sai Pavan Boddu
> Signed-off-by: Richard Henderson
> ---
>
> Sai, would you try this against your failing testcase?
>
>
> r~
>
> ---
> tcg/tcg.c | 9 +
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/tcg/tcg.c b/tcg/tcg.c
> index f49f1a7f35..43c6cf8f52 100644
> --- a/tcg/tcg.c
> +++ b/tcg/tcg.c
> @@ -335,10 +335,11 @@ static bool tcg_resolve_relocs(TCGContext *s)
>
> static void set_jmp_reset_offset(TCGContext *s, int which)
> {
> -size_t off = tcg_current_code_size(s);
> -s->tb_jmp_reset_offset[which] = off;
> -/* Make sure that we didn't overflow the stored offset. */
> -assert(s->tb_jmp_reset_offset[which] == off);
> +/*
> + * We will check for overflow at the end of the opcode loop in
> + * tcg_gen_code, where we bound tcg_current_code_size to UINT16_MAX.
> + */
> +s->tb_jmp_reset_offset[which] = tcg_current_code_size(s);
> }
>
> #include "tcg-target.c.inc"
>
Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH-for-5.2 v3] hw/mips: Remove the 'r4k' machine
On 11/3/20 7:58 AM, Thomas Huth wrote: > On 02/11/2020 21.13, Philippe Mathieu-Daudé wrote: >> We deprecated the support for the 'r4k' machine for the 5.0 release >> (commit d32dc61421), which means that our deprecation policy allows >> us to drop it in release 5.2. Remove the code. >> >> To repeat the rationale from the deprecation note: >> - this virtual machine has no specification >> - the Linux kernel dropped support for it 10 years ago >> >> Users are recommended to use the Malta board instead. >> >> Acked-by: Richard Henderson >> Signed-off-by: Philippe Mathieu-Daudé >> --- >> v3: Move to "Recently removed features" section >> --- >> docs/system/deprecated.rst| 12 +- >> .../devices/mips-softmmu-common.mak | 1 - >> hw/mips/r4k.c | 318 -- >> MAINTAINERS | 6 - >> hw/mips/Kconfig | 13 - >> hw/mips/meson.build | 1 - >> 6 files changed, 6 insertions(+), 345 deletions(-) >> delete mode 100644 hw/mips/r4k.c >> >> diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst >> index 0ebce37a191..e5b7cf274d3 100644 >> --- a/docs/system/deprecated.rst >> +++ b/docs/system/deprecated.rst >> @@ -327,12 +327,6 @@ The 'scsi-disk' device is deprecated. Users should use >> 'scsi-hd' or >> System emulator machines >> >> >> -mips ``r4k`` platform (since 5.0) >> -' >> - >> -This machine type is very old and unmaintained. Users should use the >> ``malta`` >> -machine type instead. >> - >> mips ``fulong2e`` machine (since 5.1) >> ' >> >> @@ -575,6 +569,12 @@ The version specific Spike machines have been removed >> in favour of the >> generic ``spike`` machine. If you need to specify an older version of the >> RISC-V >> spec you can use the ``-cpu rv64gcsu,priv_spec=v1.10.0`` command line >> argument. >> >> +mips ``r4k`` platform (removed in 5.2) >> +'' >> + >> +This machine type is very old and unmaintained. Users should use the >> ``malta`` >> +machine type instead. > > Ah, just spotted this v3 - that's better now indeed :-) > Maybe change "is very old" into "was very old"? OK. > > Anyway: > Reviewed-by: Thomas Huth Thanks!
Re: [PATCH-for-5.2 v3] hw/mips: Remove the 'r4k' machine
On 02/11/2020 21.13, Philippe Mathieu-Daudé wrote: > We deprecated the support for the 'r4k' machine for the 5.0 release > (commit d32dc61421), which means that our deprecation policy allows > us to drop it in release 5.2. Remove the code. > > To repeat the rationale from the deprecation note: > - this virtual machine has no specification > - the Linux kernel dropped support for it 10 years ago > > Users are recommended to use the Malta board instead. > > Acked-by: Richard Henderson > Signed-off-by: Philippe Mathieu-Daudé > --- > v3: Move to "Recently removed features" section > --- > docs/system/deprecated.rst| 12 +- > .../devices/mips-softmmu-common.mak | 1 - > hw/mips/r4k.c | 318 -- > MAINTAINERS | 6 - > hw/mips/Kconfig | 13 - > hw/mips/meson.build | 1 - > 6 files changed, 6 insertions(+), 345 deletions(-) > delete mode 100644 hw/mips/r4k.c > > diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst > index 0ebce37a191..e5b7cf274d3 100644 > --- a/docs/system/deprecated.rst > +++ b/docs/system/deprecated.rst > @@ -327,12 +327,6 @@ The 'scsi-disk' device is deprecated. Users should use > 'scsi-hd' or > System emulator machines > > > -mips ``r4k`` platform (since 5.0) > -' > - > -This machine type is very old and unmaintained. Users should use the > ``malta`` > -machine type instead. > - > mips ``fulong2e`` machine (since 5.1) > ' > > @@ -575,6 +569,12 @@ The version specific Spike machines have been removed in > favour of the > generic ``spike`` machine. If you need to specify an older version of the > RISC-V > spec you can use the ``-cpu rv64gcsu,priv_spec=v1.10.0`` command line > argument. > > +mips ``r4k`` platform (removed in 5.2) > +'' > + > +This machine type is very old and unmaintained. Users should use the > ``malta`` > +machine type instead. Ah, just spotted this v3 - that's better now indeed :-) Maybe change "is very old" into "was very old"? Anyway: Reviewed-by: Thomas Huth
Re: [PATCH-for-5.2] hw/mips: Remove the 'r4k' machine
On 02/11/2020 11.26, Philippe Mathieu-Daudé wrote: > We deprecated the support for the 'r4k' machine for the 5.0 release > (commit d32dc61421), which means that our deprecation policy allows > us to drop it in release 5.2. Remove the code. > > To repeat the rationale from the deprecation note: > - this virtual machine has no specification > - the Linux kernel dropped support for it 10 years ago > > Users are recommended to use the Malta board instead. > > Signed-off-by: Philippe Mathieu-Daudé > --- > docs/system/deprecated.rst| 2 +- > .../devices/mips-softmmu-common.mak | 1 - > hw/mips/r4k.c | 318 -- > MAINTAINERS | 6 - > hw/mips/Kconfig | 13 - > hw/mips/meson.build | 1 - > 6 files changed, 1 insertion(+), 340 deletions(-) > delete mode 100644 hw/mips/r4k.c > > diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst > index 0ebce37a191..0e83ea2ca0a 100644 > --- a/docs/system/deprecated.rst > +++ b/docs/system/deprecated.rst > @@ -327,7 +327,7 @@ The 'scsi-disk' device is deprecated. Users should use > 'scsi-hd' or > System emulator machines > > > -mips ``r4k`` platform (since 5.0) > +mips ``r4k`` platform (removed in 5.2) > ' You should move the paragraph to the "Recently removed features" section instead. Thomas
RE: [PATCH for-5.2] tcg: Remove assert from set_jmp_reset_offset
Hi Richard
> -Original Message-
> From: Richard Henderson
> Sent: Tuesday, November 3, 2020 9:10 AM
> To: qemu-devel@nongnu.org
> Cc: Sai Pavan Boddu
> Subject: [PATCH for-5.2] tcg: Remove assert from set_jmp_reset_offset
>
> The range check done here is done later, more appropriately, at the end of
> tcg_gen_code. There, a failing range check results in a returned error code,
> which causes the TB to be restarted at half the size.
>
> Reported-by: Sai Pavan Boddu
> Signed-off-by: Richard Henderson
> ---
>
> Sai, would you try this against your failing testcase?
[Sai Pavan Boddu] Thanks, this patch fixes the test. Thanks for the support.
Tested-by: Sai Pavan Boddu
Regards,
Sai Pavan
>
>
> r~
>
> ---
> tcg/tcg.c | 9 +
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/tcg/tcg.c b/tcg/tcg.c
> index f49f1a7f35..43c6cf8f52 100644
> --- a/tcg/tcg.c
> +++ b/tcg/tcg.c
> @@ -335,10 +335,11 @@ static bool tcg_resolve_relocs(TCGContext *s)
>
> static void set_jmp_reset_offset(TCGContext *s, int which) {
> -size_t off = tcg_current_code_size(s);
> -s->tb_jmp_reset_offset[which] = off;
> -/* Make sure that we didn't overflow the stored offset. */
> -assert(s->tb_jmp_reset_offset[which] == off);
> +/*
> + * We will check for overflow at the end of the opcode loop in
> + * tcg_gen_code, where we bound tcg_current_code_size to UINT16_MAX.
> + */
> +s->tb_jmp_reset_offset[which] = tcg_current_code_size(s);
> }
>
> #include "tcg-target.c.inc"
> --
> 2.25.1
Re: [PATCH v2 11/11] sockets: Make abstract UnixSocketAddress depend on CONFIG_LINUX
Eric Blake writes:
> On 11/2/20 3:44 AM, Markus Armbruster wrote:
>> The abstract socket namespace is a non-portable Linux extension. An
>> attempt to use it elsewhere should fail with ENOENT (the abstract
>> address looks like a "" pathname, which does not resolve). We report
>> this failure like
>>
>> Failed to connect socket abc: No such file or directory
>>
>> Tolerable, although ENOTSUP would be better.
>>
>> However, introspection lies: it has @abstract regardless of host
>> support. Easy enough to fix: since Linux provides them since 2.2,
>> 'if': 'defined(CONFIG_LINUX)' should do.
>>
>> The above failure becomes
>>
>> Parameter 'backend.data.addr.data.abstract' is unexpected
>>
>> I consider this an improvement.
>>
>
> Commit message lacks mention of the fact that we are now explicitly not
> outputting 'strict' for non-abstract sockets (in fact, that change could
I trust you mean 'tight'.
> be squashed in 9/11 if you wanted to do it there).
Less churn. I'll do it if I need to respin.
> But as this cleans
> up the code I mentioned in 9/11, I'll leave it up to Dan if the commit
> message needs a tweak; the end result is fine if we don't feel like a v3
> spin just for moving hunks around.
Neglecting to mention the change in the commit message isn't *too* bad,
because the change "only" corrects something new in this series, which
makes a future question "why did this go away?" relatively unlikely.
That said, I'm happy to respin if you think it's worthwhile. Just ask.
> Reviewed-by: Eric Blake
>
>> +++ b/chardev/char-socket.c
>> @@ -444,14 +444,20 @@ static char *qemu_chr_socket_address(SocketChardev *s,
>> const char *prefix)
>> break;
>> case SOCKET_ADDRESS_TYPE_UNIX:
>> {
>> +const char *tight = "", *abstract = "";
>> UnixSocketAddress *sa = >addr->u.q_unix;
>>
>> -return g_strdup_printf("%sunix:%s%s%s%s", prefix,
>> - s->addr->u.q_unix.path,
>> - sa->has_abstract && sa->abstract
>> - ? ",abstract" : "",
>> - sa->has_tight && sa->tight
>> - ? ",tight" : "",
>
> Unconditional output if tight is true (which is its stated default)...
>
>> +#ifdef CONFIG_LINUX
>> +if (sa->has_abstract && sa->abstract) {
>> +abstract = ",abstract";
>> +if (sa->has_tight && sa->tight) {
>> +tight = ",tight";
>> +}
>> +}
>> +#endif
>> +
>> +return g_strdup_printf("%sunix:%s%s%s%s", prefix, sa->path,
>> + abstract, tight,
>
> ...vs. the now-nicer conditional where tight is only present if abstract.
[PATCH-for-5.2 v2] hw/virtio/vhost-backend: Fix Coverity CID 1432871
Fix uninitialized value issues reported by Coverity:
Field 'msg.reserved' is uninitialized when calling write().
While the 'struct vhost_msg' does not have a 'reserved' field,
we still initialize it to have the two parts of the function
consistent.
Reported-by: Coverity (CID 1432864: UNINIT)
Fixes: c471ad0e9bd ("vhost_net: device IOTLB support")
Reviewed-by: Peter Maydell
Signed-off-by: Philippe Mathieu-Daudé
---
v2: comment 'struct vhost_msg' is also initialized (Peter)
---
hw/virtio/vhost-backend.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c
index 88c8ecc9e03..222bbcc62de 100644
--- a/hw/virtio/vhost-backend.c
+++ b/hw/virtio/vhost-backend.c
@@ -257,7 +257,7 @@ static int vhost_kernel_send_device_iotlb_msg(struct
vhost_dev *dev,
struct vhost_iotlb_msg *imsg)
{
if (dev->backend_cap & (1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2)) {
-struct vhost_msg_v2 msg;
+struct vhost_msg_v2 msg = {};
msg.type = VHOST_IOTLB_MSG_V2;
msg.iotlb = *imsg;
@@ -267,7 +267,7 @@ static int vhost_kernel_send_device_iotlb_msg(struct
vhost_dev *dev,
return -EFAULT;
}
} else {
-struct vhost_msg msg;
+struct vhost_msg msg = {};
msg.type = VHOST_IOTLB_MSG;
msg.iotlb = *imsg;
--
2.26.2
Re: [PATCH] hw/9pfs: virtio-9p: Ensure config space is a multiple of 4 bytes
On Tue, Nov 3, 2020 at 2:26 PM Bin Meng wrote: > > Hi Michael, > > On Fri, Oct 30, 2020 at 5:29 PM Michael S. Tsirkin wrote: > > > > On Thu, Oct 29, 2020 at 04:25:41PM +0800, Bin Meng wrote: > > > From: Bin Meng > > > > > > At present the virtio device config space access is handled by the > > > virtio_config_readX() and virtio_config_writeX() APIs. They perform > > > a sanity check on the result of address plus size against the config > > > space size before the access occurs. > > > > > > For unaligned access, the last converted naturally aligned access > > > will fail the sanity check on 9pfs. For example, with a mount_tag > > > `p9fs`, if guest software tries to read the mount_tag via a 4 byte > > > read at the mount_tag offset which is not 4 byte aligned, the read > > > result will be `p9\377\377`, which is wrong. > > > > > > This changes the size of device config space to be a multiple of 4 > > > bytes so that correct result can be returned in all circumstances. > > > > > > Signed-off-by: Bin Meng > > > > > > > > The patch is ok, but I'd like to clarify the commit log. > > Thanks for the review. > > > > > If I understand correctly, what happens is: > > - tag is set to a value that is not a multiple of 4 bytes > > It's not about the mount_tag value, but the length of the mount_tag is 4. > > > - guest attempts to read the last 4 bytes of the tag > > Yep. So the config space of a 9pfs looks like the following: > > offset: 0x14, size: 2 bytes indicating the length of the following mount_tag > offset: 0x16, size: value of (offset 0x14). > > When a 4-byte mount_tag is given, guest software is subject to read 4 > bytes (value read from offset 0x14) at offset 0x16. > > > - access returns -1 > > > > The access will be split into 2 accesses, either by hardware or > software. On RISC-V such unaligned access is emulated by M-mode > firmware. On ARM I believe it's supported by the CPU. So the first > converted aligned access is to read 4 byte at 0x14 and the second > converted aligned access is to read 4 byte at 0x16, and drop the bytes Oops, typo. The 2nd access happens at offset 0x18 > that are not needed, assemble the remaining bytes and return the > result to the guest software. The second aligned access will fail the > sanity check and return -1, but not the first access, hence the result > will be `p9\377\377`. > > > > > What I find confusing in the above description: > > - reference to unaligned access - I don't think these > > are legal or allowed by QEMU > > - reference to `p9\377\377` - I think returned value will be -1 > > Regards, Bin
Re: [PATCH-for-5.2? 2/5] tests/acceptance: Restore MIPS Malta multicore tests
On 11/2/20 3:42 PM, Philippe Mathieu-Daudé wrote:
> Since 42a052333a6 ("hw/misc/mips_cpc: Start vCPU when powered on")
> the multicore support of the MIPS Malta board has been fixed.
>
> This reverts commit 61bbce96fe4c8e3a2b7df5a67ba7dc6ba418e54b.
>
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> tests/acceptance/machine_mips_malta.py | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/tests/acceptance/machine_mips_malta.py
> b/tests/acceptance/machine_mips_malta.py
> index 7c9a4ee4d2d..eea046141d6 100644
> --- a/tests/acceptance/machine_mips_malta.py
> +++ b/tests/acceptance/machine_mips_malta.py
> @@ -100,7 +100,6 @@ def test_mips_malta_i6400_framebuffer_logo_1core(self):
> """
> self.do_test_i6400_framebuffer_logo(1)
>
> -@skipIf(os.getenv('GITLAB_CI'), 'Running on GitLab')
> def test_mips_malta_i6400_framebuffer_logo_7cores(self):
> """
> :avocado: tags=arch:mips64el
> @@ -110,7 +109,6 @@ def test_mips_malta_i6400_framebuffer_logo_7cores(self):
> """
> self.do_test_i6400_framebuffer_logo(7)
>
> -@skipIf(os.getenv('GITLAB_CI'), 'Running on GitLab')
> def test_mips_malta_i6400_framebuffer_logo_8cores(self):
And this still doesn't work... :(
https://gitlab.com/philmd/qemu/-/jobs/826406235#L218
Re: [PATCH v2 09/11] char-socket: Fix qemu_chr_socket_address() for abstract sockets
Eric Blake writes:
> On 11/2/20 3:44 AM, Markus Armbruster wrote:
>> Commit 776b97d360 "qemu-sockets: add abstract UNIX domain socket
>> support" neglected to update qemu_chr_socket_address(). It shows
>> shows neither @abstract nor @tight. Fix that.
>>
>> Reviewed-by: Paolo Bonzini
>> Signed-off-by: Markus Armbruster
>> ---
>> chardev/char-socket.c | 10 +-
>> 1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
>> index 1ee5a8c295..dc1cf86ecf 100644
>> --- a/chardev/char-socket.c
>> +++ b/chardev/char-socket.c
>> @@ -443,10 +443,18 @@ static char *qemu_chr_socket_address(SocketChardev *s,
>> const char *prefix)
>> s->is_listen ? ",server" : "");
>> break;
>> case SOCKET_ADDRESS_TYPE_UNIX:
>> -return g_strdup_printf("%sunix:%s%s", prefix,
>> +{
>> +UnixSocketAddress *sa = >addr->u.q_unix;
>> +
>> +return g_strdup_printf("%sunix:%s%s%s%s", prefix,
>> s->addr->u.q_unix.path,
>> + sa->has_abstract && sa->abstract
>> + ? ",abstract" : "",
>> + sa->has_tight && sa->tight
>> + ? ",tight" : "",
>> s->is_listen ? ",server" : "");
>
> Gets modified again in 11/11, so I can accept this as a strict
> improvement, even if it is not the final form.
You're right, PATCH 11's change is better done here already. Will tidy
up if I need to respin for some other reason.
> Reviewed-by: Eric Blake
Thanks!
>
>> break;
>> +}
>> case SOCKET_ADDRESS_TYPE_FD:
>> return g_strdup_printf("%sfd:%s%s", prefix, s->addr->u.fd.str,
>> s->is_listen ? ",server" : "");
>>
Re: [PATCH] hw/9pfs: virtio-9p: Ensure config space is a multiple of 4 bytes
Hi Michael, On Fri, Oct 30, 2020 at 5:29 PM Michael S. Tsirkin wrote: > > On Thu, Oct 29, 2020 at 04:25:41PM +0800, Bin Meng wrote: > > From: Bin Meng > > > > At present the virtio device config space access is handled by the > > virtio_config_readX() and virtio_config_writeX() APIs. They perform > > a sanity check on the result of address plus size against the config > > space size before the access occurs. > > > > For unaligned access, the last converted naturally aligned access > > will fail the sanity check on 9pfs. For example, with a mount_tag > > `p9fs`, if guest software tries to read the mount_tag via a 4 byte > > read at the mount_tag offset which is not 4 byte aligned, the read > > result will be `p9\377\377`, which is wrong. > > > > This changes the size of device config space to be a multiple of 4 > > bytes so that correct result can be returned in all circumstances. > > > > Signed-off-by: Bin Meng > > > > The patch is ok, but I'd like to clarify the commit log. Thanks for the review. > > If I understand correctly, what happens is: > - tag is set to a value that is not a multiple of 4 bytes It's not about the mount_tag value, but the length of the mount_tag is 4. > - guest attempts to read the last 4 bytes of the tag Yep. So the config space of a 9pfs looks like the following: offset: 0x14, size: 2 bytes indicating the length of the following mount_tag offset: 0x16, size: value of (offset 0x14). When a 4-byte mount_tag is given, guest software is subject to read 4 bytes (value read from offset 0x14) at offset 0x16. > - access returns -1 > The access will be split into 2 accesses, either by hardware or software. On RISC-V such unaligned access is emulated by M-mode firmware. On ARM I believe it's supported by the CPU. So the first converted aligned access is to read 4 byte at 0x14 and the second converted aligned access is to read 4 byte at 0x16, and drop the bytes that are not needed, assemble the remaining bytes and return the result to the guest software. The second aligned access will fail the sanity check and return -1, but not the first access, hence the result will be `p9\377\377`. > > What I find confusing in the above description: > - reference to unaligned access - I don't think these > are legal or allowed by QEMU > - reference to `p9\377\377` - I think returned value will be -1 > Regards, Bin
Re: [PULL 09/12] qga: add ssh-{add,remove}-authorized-keys
Michael Roth writes: > On Mon, Nov 02, 2020 at 04:49:27PM +0100, Markus Armbruster wrote: [...] >> checkpatch ERROR: Use g_assert or g_assert_not_reached >> >> See commit 6e9389563e "checkpatch: Disallow glib asserts in main code" >> for rationale. > > Thanks for the pointer, I couldn't figure out what the issue was and You can always ask :) > assumed it was just noise. Wish I noticed this message before I sent out > v2... v3 incoming. Thanks! [...]
Re: [PATCH v2 1/3] softmmu: Do not use C99 // comments
chaihaoyu writes: > Hi, recently I found some code style problems while using checkpatch.pl > tool,please review. > > Date: Tue, 3 Nov 2020 11:01:40 +0800 > signed-off-by: Haoyu Chai > --- > softmmu/memory.c | 2 +- > softmmu/memory_mapping.c | 2 +- > softmmu/physmem.c| 2 +- > 3 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/softmmu/memory.c b/softmmu/memory.c > index 107ce0a4f8..5fb591b001 100644 > --- a/softmmu/memory.c > +++ b/softmmu/memory.c > @@ -36,7 +36,7 @@ > #include "hw/boards.h" > #include "migration/vmstate.h" > > -//#define DEBUG_UNASSIGNED > +/* #define DEBUG_UNASSIGNED */ > > static unsigned memory_region_transaction_depth; > static bool memory_region_update_pending; > diff --git a/softmmu/memory_mapping.c b/softmmu/memory_mapping.c > index 18d0b8067c..f64053499e 100644 > --- a/softmmu/memory_mapping.c > +++ b/softmmu/memory_mapping.c > @@ -19,7 +19,7 @@ > #include "exec/memory.h" > #include "exec/address-spaces.h" > > -//#define DEBUG_GUEST_PHYS_REGION_ADD > +/* #define DEBUG_GUEST_PHYS_REGION_ADD */ > > static void memory_mapping_list_add_mapping_sorted(MemoryMappingList *list, > MemoryMapping *mapping) > diff --git a/softmmu/physmem.c b/softmmu/physmem.c > index 44ffb60b5d..78c1b6580a 100644 > --- a/softmmu/physmem.c > +++ b/softmmu/physmem.c > @@ -75,7 +75,7 @@ > #include > #endif > > -//#define DEBUG_SUBPAGE > +/* #define DEBUG_SUBPAGE */ > > /* ram_list is read under rcu_read_lock()/rcu_read_unlock(). Writes > * are protected by the ramlist lock. I recommend to leave these alone. CODING_STYLE.rst: Rationale: The // form is valid in C99, so this is purely a matter of consistency of style. The checkpatch script will warn you about this. For "real" comments, we overwhelmingly use /* */, and avoiding // makes sense. Most exceptions are in code we copy from elsewhere, such as disas/libvixl/. For commenting out *code*, we use both forms. Here are the counts for commenting out macro definitions: $ git-grep '^/\* *# *define' | wc -l 125 $ git-grep '^// *# *define' | wc -l 192
Re: [Bug 1902470] Re: migration with TLS-MultiFD is stuck when the dst-libvirtd service restarts
On 2020/11/3 4:16, Dr. David Alan Gilbert wrote:
> * zhengchuan (zhengch...@huawei.com) wrote:
>> Anyone who could help this would be appreciated since we have stuck for
>> three days:(
>>
>> IIUC, the client (Src) has sent first hello message to sever(Dst), however
>> due to something happened while restarted libvirtd,
>> The messages is lost, and both of them are waiting which leading to hang
>> forever, but I could find out how for now.
>
> If you need to un-break things, I suggest killing the destination might
> free it; but I'm not sure.
>
Hi, Dave.
Unfortunately, no. After killing the destination, it left Src main migration
thread stuck at multifd_send_sync_main().
> An interesting question is if we can make migration-cancel work in this
> case.
>
> Dave
>
Bad thing happened, since the main qemu thread is stuck at recvmsg(), qemu
could not respond for libvirt qmp_migrate_cancel:(
During the time, I also found another question is that the Dst socket
connections are not closed after migration-cancel,
multifd channel would be left with status of CLOSE-WAIT if we look at them
though 'ss' command.
This is because the multifd_save_cleanup() is simply call
socket_send_channel_destroy and unref the ioc other than calling
qio_channel_shutdown() in multifd_recv_terminate_threads(), It is not working
for tls channel.
Simply working around by adding qio_channel_shutdown like this
for (i = 0; i < migrate_multifd_channels(); i++) {
MultiFDSendParams *p = _send_state->params[i];
+ qio_channel_shutdown(p->c, QIO_CHANNEL_SHUTDOWN_BOTH, NULL);
socket_send_channel_destroy(p->c);
}
The residual socket is closed, but i doubt if it is the correct solution...
Back to the problem described in this issue, it is still not resolved after
this working around, but i think it is also a similiar
cleanup issue, and i will dig it out more further...
>> -Original Message-
>> From: Qemu-devel
>> [mailto:qemu-devel-bounces+zhengchuan=huawei@nongnu.org] On Behalf Of
>> Yan Jin
>> Sent: 2020年11月2日 11:12
>> To: qemu-devel@nongnu.org
>> Subject: [Bug 1902470] Re: migration with TLS-MultiFD is stuck when the
>> dst-libvirtd service restarts
>>
>> ** Description changed:
>>
>> hi,
>>
>> I found that the multi-channel TLS-handshake will be stuck when the dst-
>> libvirtd restarts, both the src and dst sockets are blocked in recvmsg.
>> In the meantime, live_migration thread is blocked in
>> multifd_send_sync_main, so migration cannot be cancelled though src-
>> libvirt has delivered the QMP command.
>>
>> Is there any way to exit migration when the multi-channel TLS-handshake
>> - is stuck? Does setting TLS handshake timeout function take effect?
>> + is stuck? Does setting TLS-handshake timeout function take effect?
>>
>> The stack trace are as follows:
>>
>> =src qemu-system-aar stack=:
>> #0 0x87d6f28c in recvmsg () from target:/usr/lib64/libpthread.so.0
>> #1 0xe3817424 in qio_channel_socket_readv (ioc=0xe9e30a30,
>> iov=0xdb58e8a8, niov=1, fds=0x0, nfds=0x0, errp=0x0) at
>> ../io/channel-socket.c:502
>> #2 0xe380f468 in qio_channel_readv_full (ioc=0xe9e30a30,
>> iov=0xdb58e8a8, niov=1, fds=0x0, nfds=0x0, errp=0x0) at
>> ../io/channel.c:66
>> #3 0xe380f9e8 in qio_channel_read (ioc=0xe9e30a30,
>> buf=0xea204e9b "\026\003\001\001L\001", buflen=5, errp=0x0) at
>> ../io/channel.c:217
>> #4 0xe380e7d4 in qio_channel_tls_read_handler (buf=0xea204e9b
>> "\026\003\001\001L\001", len=5, opaque=0xfffd38001190) at
>> ../io/channel-tls.c:53
>> #5 0xe3801114 in qcrypto_tls_session_pull (opaque=0xe99d5700,
>> buf=0xea204e9b, len=5) at ../crypto/tlssession.c:89
>> #6 0x8822ed30 in _gnutls_stream_read (ms=0xdb58eaac,
>> pull_func=0xfffd38001870, size=5, bufel=,
>> session=0xe983cd60) at buffers.c:346
>> #7 _gnutls_read (ms=0xdb58eaac, pull_func=0xfffd38001870, size=5,
>> bufel=, session=0xe983cd60) at buffers.c:426
>> #8 _gnutls_io_read_buffered (session=session@entry=0xe983cd60,
>> total=5, recv_type=recv_type@entry=4294967295, ms=0xdb58eaac) at
>> buffers.c:581
>> #9 0x88224954 in recv_headers (ms=,
>> record=0x883cd000 ,
>> htype=65535, type=2284006288, record_params=0xe9e22a60,
>> session=0xe983cd60) at record.c:1163
>> #10 _gnutls_recv_in_buffers (session=session@entry=0xe983cd60,
>> type=2284006288, type@entry=GNUTLS_HANDSHAKE, htype=65535,
>> htype@entry=GNUTLS_HANDSHAKE_HELLO_RETRY_REQUEST, ms=,
>> ms@entry=0) at record.c:1302
>> #11 0x88230568 in _gnutls_handshake_io_recv_int
>> (session=session@entry=0xe983cd60,
>> htype=htype@entry=GNUTLS_HANDSHAKE_HELLO_RETRY_REQUEST,
>> hsk=hsk@entry=0xdb58ec38, optional=optional@entry=1) at buffers.c:1445
>> #12 0x88232b90 in _gnutls_recv_handshake
>>
Re: [PULL v3 23/32] s390x/pci: Add routine to get the vfio dma available count
Hi Matthew,
On 11/1/20 10:02 PM, Alex Williamson wrote:
> From: Matthew Rosato
>
> Create new files for separating out vfio-specific work for s390
> pci. Add the first such routine, which issues VFIO_IOMMU_GET_INFO
> ioctl to collect the current dma available count.
>
> Signed-off-by: Matthew Rosato
> Reviewed-by: Cornelia Huck
> [aw: Fix non-Linux build with CONFIG_LINUX]
> Signed-off-by: Alex Williamson
> ---
> hw/s390x/meson.build |1 +
> hw/s390x/s390-pci-vfio.c | 54
> ++
> include/hw/s390x/s390-pci-vfio.h | 24 +
> 3 files changed, 79 insertions(+)
> create mode 100644 hw/s390x/s390-pci-vfio.c
> create mode 100644 include/hw/s390x/s390-pci-vfio.h
>
> diff --git a/hw/s390x/meson.build b/hw/s390x/meson.build
> index 948ceae7a7b3..f4663a835514 100644
> --- a/hw/s390x/meson.build
> +++ b/hw/s390x/meson.build
> @@ -27,6 +27,7 @@ s390x_ss.add(when: 'CONFIG_KVM', if_true: files(
> ))
> s390x_ss.add(when: 'CONFIG_S390_CCW_VIRTIO', if_true:
> files('s390-virtio-ccw.c'))
> s390x_ss.add(when: 'CONFIG_TERMINAL3270', if_true: files('3270-ccw.c'))
> +s390x_ss.add(when: 'CONFIG_LINUX', if_true: files('s390-pci-vfio.c'))
>
> virtio_ss = ss.source_set()
> virtio_ss.add(files('virtio-ccw.c'))
> diff --git a/hw/s390x/s390-pci-vfio.c b/hw/s390x/s390-pci-vfio.c
> new file mode 100644
> index ..cb3f4d98adf8
> --- /dev/null
> +++ b/hw/s390x/s390-pci-vfio.c
> @@ -0,0 +1,54 @@
> +/*
> + * s390 vfio-pci interfaces
> + *
> + * Copyright 2020 IBM Corp.
> + * Author(s): Matthew Rosato
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or (at
> + * your option) any later version. See the COPYING file in the top-level
> + * directory.
> + */
> +
> +#include
> +
> +#include "qemu/osdep.h"
> +#include "hw/s390x/s390-pci-vfio.h"
> +#include "hw/vfio/vfio-common.h"
> +
> +/*
> + * Get the current DMA available count from vfio. Returns true if vfio is
> + * limiting DMA requests, false otherwise. The current available count read
> + * from vfio is returned in avail.
> + */
> +bool s390_pci_update_dma_avail(int fd, unsigned int *avail)
> +{
> +g_autofree struct vfio_iommu_type1_info *info;
> +uint32_t argsz;
> +
> +assert(avail);
> +
> +argsz = sizeof(struct vfio_iommu_type1_info);
> +info = g_malloc0(argsz);
> +
> +/*
> + * If the specified argsz is not large enough to contain all capabilities
> + * it will be updated upon return from the ioctl. Retry until we have
> + * a big enough buffer to hold the entire capability chain.
> + */
> +retry:
> +info->argsz = argsz;
> +
> +if (ioctl(fd, VFIO_IOMMU_GET_INFO, info)) {
> +return false;
> +}
> +
> +if (info->argsz > argsz) {
> +argsz = info->argsz;
> +info = g_realloc(info, argsz);
> +goto retry;
> +}
> +
> +/* If the capability exists, update with the current value */
> +return vfio_get_info_dma_avail(info, avail);
> +}
--without-default-devices configuration is broken, bisected
to this commit:
hw/s390x/s390-pci-vfio.c:52: undefined reference to
`vfio_get_info_dma_avail'
Can you have a look please?
Thanks.
RE: [PATCH] Revert "vhost-blk: set features before setting inflight feature"
Hi Stefan,
I have sent a version 2 and it should fix this issue.
I also test it several times and I did not meet this issue again.
Thanks for your report.
Best Regards
Jin
> -Original Message-
> From: Stefan Hajnoczi
> Sent: Tuesday, November 3, 2020 12:57 AM
> To: qemu-devel@nongnu.org
> Cc: qemu-bl...@nongnu.org; Raphael Norwitz
> ; Max Reitz ; Kevin
> Wolf ; Michael S. Tsirkin ; Stefan
> Hajnoczi ; Yu, Jin
> Subject: [PATCH] Revert "vhost-blk: set features before setting inflight
> feature"
>
> This reverts commit adb29c027341ba095a3ef4beef6aaef86d3a520e.
>
> The commit broke -device vhost-user-blk-pci because the
> vhost_dev_prepare_inflight() function it introduced segfaults in
> vhost_dev_set_features() when attempting to access struct vhost_dev's vdev
> pointer before it has been assigned.
>
> To reproduce the segfault simply launch a vhost-user-blk device with the
> contrib vhost-user-blk device backend:
>
> $ build/contrib/vhost-user-blk/vhost-user-blk -s /tmp/vhost-user-blk.sock -r
> -b /var/tmp/foo.img
> $ build/qemu-system-x86_64 \
> -device vhost-user-blk-pci,id=drv0,chardev=char1,addr=4.0 \
> -object memory-backend-memfd,id=mem,size=1G,share=on \
> -M memory-backend=mem,accel=kvm \
> -chardev socket,id=char1,path=/tmp/vhost-user-blk.sock
> Segmentation fault (core dumped)
>
> Cc: Jin Yu
> Cc: Raphael Norwitz
> Cc: Michael S. Tsirkin
> Signed-off-by: Stefan Hajnoczi
> ---
> include/hw/virtio/vhost.h | 1 -
> hw/block/vhost-user-blk.c | 6 --
> hw/virtio/vhost.c | 18 --
> 3 files changed, 25 deletions(-)
>
> diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h index
> 839bfb153c..94585067f7 100644
> --- a/include/hw/virtio/vhost.h
> +++ b/include/hw/virtio/vhost.h
> @@ -141,7 +141,6 @@ void vhost_dev_reset_inflight(struct vhost_inflight
> *inflight); void vhost_dev_free_inflight(struct vhost_inflight *inflight);
> void
> vhost_dev_save_inflight(struct vhost_inflight *inflight, QEMUFile *f); int
> vhost_dev_load_inflight(struct vhost_inflight *inflight, QEMUFile *f); -int
> vhost_dev_prepare_inflight(struct vhost_dev *hdev); int
> vhost_dev_set_inflight(struct vhost_dev *dev,
> struct vhost_inflight *inflight); int
> vhost_dev_get_inflight(struct vhost_dev *dev, uint16_t queue_size, diff --git
> a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c index
> f67b29bbf3..a076b1e54d 100644
> --- a/hw/block/vhost-user-blk.c
> +++ b/hw/block/vhost-user-blk.c
> @@ -131,12 +131,6 @@ static int vhost_user_blk_start(VirtIODevice *vdev)
>
> s->dev.acked_features = vdev->guest_features;
>
> -ret = vhost_dev_prepare_inflight(>dev);
> -if (ret < 0) {
> -error_report("Error set inflight format: %d", -ret);
> -goto err_guest_notifiers;
> -}
> -
> if (!s->inflight->addr) {
> ret = vhost_dev_get_inflight(>dev, s->queue_size, s->inflight);
> if (ret < 0) {
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index
> f2482378c6..79b2be20df 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -1645,24 +1645,6 @@ int vhost_dev_load_inflight(struct vhost_inflight
> *inflight, QEMUFile *f)
> return 0;
> }
>
> -int vhost_dev_prepare_inflight(struct vhost_dev *hdev) -{
> -int r;
> -
> -if (hdev->vhost_ops->vhost_get_inflight_fd == NULL ||
> -hdev->vhost_ops->vhost_set_inflight_fd == NULL) {
> -return 0;
> -}
> -
> -r = vhost_dev_set_features(hdev, hdev->log_enabled);
> -if (r < 0) {
> -VHOST_OPS_DEBUG("vhost_dev_prepare_inflight failed");
> -return r;
> -}
> -
> -return 0;
> -}
> -
> int vhost_dev_set_inflight(struct vhost_dev *dev,
> struct vhost_inflight *inflight) {
> --
> 2.28.0
[PATCH v2] vhost-blk: set features before setting inflight feature
Virtqueue has split and packed, so before setting inflight,
you need to inform the back-end virtqueue format.
Signed-off-by: Jin Yu
Acked-by: Raphael Norwitz
---
v2:
* Fixed the segfault.
---
hw/block/vhost-user-blk.c | 6 ++
hw/virtio/vhost.c | 20
include/hw/virtio/vhost.h | 1 +
3 files changed, 27 insertions(+)
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index 39aec42dae..db079a89c0 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -131,6 +131,12 @@ static int vhost_user_blk_start(VirtIODevice *vdev)
s->dev.acked_features = vdev->guest_features;
+ret = vhost_dev_prepare_inflight(>dev, vdev);
+if (ret < 0) {
+error_report("Error set inflight format: %d", -ret);
+goto err_guest_notifiers;
+}
+
if (!s->inflight->addr) {
ret = vhost_dev_get_inflight(>dev, s->queue_size, s->inflight);
if (ret < 0) {
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 1a1384e7a6..6ffbfbfb9e 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1616,6 +1616,26 @@ int vhost_dev_load_inflight(struct vhost_inflight
*inflight, QEMUFile *f)
return 0;
}
+int vhost_dev_prepare_inflight(struct vhost_dev *hdev, VirtIODevice *vdev)
+{
+int r;
+
+if (hdev->vhost_ops->vhost_get_inflight_fd == NULL ||
+hdev->vhost_ops->vhost_set_inflight_fd == NULL) {
+return 0;
+}
+
+hdev->vdev = vdev;
+
+r = vhost_dev_set_features(hdev, hdev->log_enabled);
+if (r < 0) {
+VHOST_OPS_DEBUG("vhost_dev_prepare_inflight failed");
+return r;
+}
+
+return 0;
+}
+
int vhost_dev_set_inflight(struct vhost_dev *dev,
struct vhost_inflight *inflight)
{
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index 767a95ec0b..d25f0947f7 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -140,6 +140,7 @@ void vhost_dev_reset_inflight(struct vhost_inflight
*inflight);
void vhost_dev_free_inflight(struct vhost_inflight *inflight);
void vhost_dev_save_inflight(struct vhost_inflight *inflight, QEMUFile *f);
int vhost_dev_load_inflight(struct vhost_inflight *inflight, QEMUFile *f);
+int vhost_dev_prepare_inflight(struct vhost_dev *hdev, VirtIODevice *vdev);
int vhost_dev_set_inflight(struct vhost_dev *dev,
struct vhost_inflight *inflight);
int vhost_dev_get_inflight(struct vhost_dev *dev, uint16_t queue_size,
--
2.17.2
[PATCH v2 0/3] softmmu: some space-style problems while coding
Hi, recently I found some code style problems while using checkpatch.pl
tool,please review.
Date: Tue, 3 Nov 2020 11:19:37 +0800
Subject: [PATCH] space style
signed-off-by: Haoyu Chai
---
softmmu/physmem.c | 2 +-
softmmu/qdev-monitor.c | 4 ++--
softmmu/vl.c | 12 ++--
3 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index 78c1b6580a..44b068ee85 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -3457,7 +3457,7 @@ int qemu_target_page_bits_min(void)
bool cpu_physical_memory_is_io(hwaddr phys_addr)
{
-MemoryRegion*mr;
+MemoryRegion *mr;
hwaddr l = 1;
bool res;
diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
index e283d9c9c0..c2b218adce 100644
--- a/softmmu/qdev-monitor.c
+++ b/softmmu/qdev-monitor.c
@@ -500,7 +500,7 @@ static BusState *qbus_find(const char *path, Error **errp)
}
/* find device */
-if (sscanf(path+pos, "%127[^/]%n", elem, ) != 1) {
+if (sscanf(path + pos, "%127[^/]%n", elem, ) != 1) {
g_assert_not_reached();
elem[0] = len = 0;
}
@@ -535,7 +535,7 @@ static BusState *qbus_find(const char *path, Error **errp)
}
/* find bus */
-if (sscanf(path+pos, "%127[^/]%n", elem, ) != 1) {
+if (sscanf(path + pos, "%127[^/]%n", elem, ) != 1) {
g_assert_not_reached();
elem[0] = len = 0;
}
diff --git a/softmmu/vl.c b/softmmu/vl.c
index c4667d91fe..d37520a356 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2418,7 +2418,7 @@ static const QEMUOption *lookup_opt(int argc, char **argv,
r++;
}
popt = qemu_options;
-for(;;) {
+for (;;) {
if (!popt->name) {
error_report("invalid option");
exit(1);
@@ -3008,7 +3008,7 @@ void qemu_init(int argc, char **argv, char **envp)
/* second pass of option parsing */
optind = 1;
-for(;;) {
+for (;;) {
if (optind >= argc) {
break;
}
@@ -3023,7 +3023,7 @@ void qemu_init(int argc, char **argv, char **envp)
error_report("Option not supported for this target");
exit(1);
}
-switch(popt->index) {
+switch (popt->index) {
case QEMU_OPTION_cpu:
/* hw initialization will check this */
cpu_option = optarg;
@@ -3182,13 +3182,13 @@ void qemu_init(int argc, char **argv, char **envp)
#endif
case QEMU_OPTION_audio_help:
audio_legacy_help();
-exit (0);
+exit(0);
break;
case QEMU_OPTION_audiodev:
audio_parse_option(optarg);
break;
case QEMU_OPTION_soundhw:
-select_soundhw (optarg);
+select_soundhw(optarg);
break;
case QEMU_OPTION_h:
help(0);
@@ -4323,7 +4323,7 @@ void qemu_init(int argc, char **argv, char **envp)
if (watchdog) {
i = select_watchdog(watchdog);
if (i > 0)
-exit (i == 1 ? 1 : 0);
+exit(i == 1 ? 1 : 0);
}
/* This checkpoint is required by replay to separate prior clock
--
[PATCH v2 3/3] softmmu: braces {} are necessary for all arms of this statement
Hi, recently I found some code style problems while using checkpatch.pl
tool,please review.
Date: Tue, 3 Nov 2020 10:09:34 +0800
Subject: [PATCH] braces {} are necessary for all arms of this statement
signed-off-by: Haoyu Chai
---
---
softmmu/cpus.c | 6 --
softmmu/dma-helpers.c | 3 ++-
softmmu/memory.c | 3 ++-
softmmu/physmem.c | 15 -
softmmu/qdev-monitor.c | 15 +++--
softmmu/vl.c | 49 +++---
6 files changed, 59 insertions(+), 32 deletions(-)
diff --git a/softmmu/cpus.c b/softmmu/cpus.c
index e46ac68ad0..3e08a64d6b 100644
--- a/softmmu/cpus.c
+++ b/softmmu/cpus.c
@@ -743,8 +743,9 @@ void qmp_memsave(int64_t addr, int64_t size, const char
*filename,
while (size != 0) {
l = sizeof(buf);
-if (l > size)
+if (l > size) {
l = size;
+}
if (cpu_memory_rw_debug(cpu, addr, buf, l, 0) != 0) {
error_setg(errp, "Invalid addr 0x%016" PRIx64 "/size %" PRId64
" specified", orig_addr, orig_size);
@@ -777,8 +778,9 @@ void qmp_pmemsave(int64_t addr, int64_t size, const char
*filename,
while (size != 0) {
l = sizeof(buf);
-if (l > size)
+if (l > size) {
l = size;
+}
cpu_physical_memory_read(addr, buf, l);
if (fwrite(buf, 1, l, f) != l) {
error_setg(errp, QERR_IO_ERROR);
diff --git a/softmmu/dma-helpers.c b/softmmu/dma-helpers.c
index 03c92e0cc6..9194ebf681 100644
--- a/softmmu/dma-helpers.c
+++ b/softmmu/dma-helpers.c
@@ -164,8 +164,9 @@ static void dma_blk_cb(void *opaque, int ret)
}
}
}
-if (!mem)
+if (!mem) {
break;
+}
qemu_iovec_add(>iov, mem, cur_len);
dbs->sg_cur_byte += cur_len;
if (dbs->sg_cur_byte == dbs->sg->sg[dbs->sg_cur_index].len) {
diff --git a/softmmu/memory.c b/softmmu/memory.c
index 550cffe8f6..107ce0a4f8 100644
--- a/softmmu/memory.c
+++ b/softmmu/memory.c
@@ -664,8 +664,9 @@ void flatview_for_each_range(FlatView *fv, flatview_cb cb ,
void *opaque)
assert(cb);
FOR_EACH_FLAT_RANGE(fr, fv) {
-if (cb(fr->addr.start, fr->addr.size, fr->mr, opaque))
+if (cb(fr->addr.start, fr->addr.size, fr->mr, opaque)) {
break;
+}
}
}
diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index a9adedb9f8..44ffb60b5d 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -797,8 +797,9 @@ int cpu_watchpoint_insert(CPUState *cpu, vaddr addr, vaddr
len,
tlb_flush(cpu);
}
-if (watchpoint)
+if (watchpoint) {
*watchpoint = wp;
+}
return 0;
}
@@ -1210,8 +1211,9 @@ void flatview_add_to_dispatch(FlatView *fv,
MemoryRegionSection *section)
void qemu_flush_coalesced_mmio_buffer(void)
{
-if (kvm_enabled())
+if (kvm_enabled()) {
kvm_flush_coalesced_mmio_buffer();
+}
}
void qemu_mutex_lock_ramlist(void)
@@ -2495,8 +2497,9 @@ static int subpage_register(subpage_t *mmio, uint32_t
start, uint32_t end,
{
int idx, eidx;
-if (start >= TARGET_PAGE_SIZE || end >= TARGET_PAGE_SIZE)
+if (start >= TARGET_PAGE_SIZE || end >= TARGET_PAGE_SIZE) {
return -1;
+}
idx = SUBPAGE_IDX(start);
eidx = SUBPAGE_IDX(end);
#if defined(DEBUG_SUBPAGE)
@@ -3408,11 +3411,13 @@ int cpu_memory_rw_debug(CPUState *cpu, target_ulong
addr,
phys_addr = cpu_get_phys_page_attrs_debug(cpu, page, );
asidx = cpu_asidx_from_attrs(cpu, attrs);
/* if no physical page mapped, return an error */
-if (phys_addr == -1)
+if (phys_addr == -1) {
return -1;
+}
l = (page + TARGET_PAGE_SIZE) - addr;
-if (l > len)
+if (l > len) {
l = len;
+}
phys_addr += (addr & ~TARGET_PAGE_MASK);
if (is_write) {
res = address_space_write_rom(cpu->cpu_ases[asidx].as, phys_addr,
diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
index bcfb90a08f..e283d9c9c0 100644
--- a/softmmu/qdev-monitor.c
+++ b/softmmu/qdev-monitor.c
@@ -44,8 +44,7 @@
* Aliases were a bad idea from the start. Let's keep them
* from spreading further.
*/
-typedef struct QDevAlias
-{
+typedef struct QDevAlias {
const char *typename;
const char *alias;
uint32_t arch_mask;
@@ -180,10 +179,12 @@ static int set_property(void *opaque, const char *name,
const char *value,
{
Object *obj = opaque;
-if (strcmp(name, "driver") == 0)
+if (strcmp(name, "driver") == 0) {
return 0;
-if (strcmp(name, "bus") == 0)
+}
+if (strcmp(name, "bus") == 0) {
return 0;
+}
if (!object_property_parse(obj, name, value, errp)) {
return -1;
@@ -694,8 +695,9 @@ static void qbus_print(Monitor *mon, BusState *bus, int
indent);
static void qdev_print_props(Monitor *mon, DeviceState *dev,
[PATCH v2 2/3] softmmu: Don't use '#' flag of printf format
Hi, recently I found some code style problems while using checkpatch.pl
tool,please review.
Date: Tue, 3 Nov 2020 11:01:40 +0800
Subject: [PATCH] Don't use '#' flag of printf format
signed-off-by: Haoyu Chai
---
softmmu/device_tree.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/softmmu/device_tree.c b/softmmu/device_tree.c
index b335dae707..c70215ba6a 100644
--- a/softmmu/device_tree.c
+++ b/softmmu/device_tree.c
@@ -367,7 +367,7 @@ int qemu_fdt_setprop_cell(void *fdt, const char *node_path,
r = fdt_setprop_cell(fdt, findnode_nofail(fdt, node_path), property, val);
if (r < 0) {
-error_report("%s: Couldn't set %s/%s = %#08x: %s", __func__,
+error_report("%s: Couldn't set %s/%s = 0x%08x: %s", __func__,
node_path, property, val, fdt_strerror(r));
exit(1);
}
--
2.29.1.59.gf9b6481aed
Re: Does QEMU's coverity-scan run need to track coverity issues in dtb or slirp ?
On Mon, Nov 02, 2020 at 07:54:14PM +, Peter Maydell wrote: > Currently QEMU's Coverity-Scan project has a bunch of unresolved > issues in code in dtc/ and also in slirp/. (I suspect most of them > are actually false-positives that got re-reported when we switched > to Meson and the filenames changed, or some similar event.) > > Do dtc and slirp as upstream projects already track Coverity issues > (in which case we can just close the issues in the QEMU tracker as > irrelevant, or do we need to investigate these and potentially > forward them into whatever upstream bug tracker is appropriate? dtc is wired up to coverity_scan, and quite a few of the things it caught were fixed a while back. I must admit I don't re-examine the remaining warnings very frequently though. I *think* what's still there are false positives, but I'm not super confident about that. -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
[PATCH v2 1/3] softmmu: Do not use C99 // comments
Hi, recently I found some code style problems while using checkpatch.pl tool,please review. Date: Tue, 3 Nov 2020 11:01:40 +0800 signed-off-by: Haoyu Chai --- softmmu/memory.c | 2 +- softmmu/memory_mapping.c | 2 +- softmmu/physmem.c| 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/softmmu/memory.c b/softmmu/memory.c index 107ce0a4f8..5fb591b001 100644 --- a/softmmu/memory.c +++ b/softmmu/memory.c @@ -36,7 +36,7 @@ #include "hw/boards.h" #include "migration/vmstate.h" -//#define DEBUG_UNASSIGNED +/* #define DEBUG_UNASSIGNED */ static unsigned memory_region_transaction_depth; static bool memory_region_update_pending; diff --git a/softmmu/memory_mapping.c b/softmmu/memory_mapping.c index 18d0b8067c..f64053499e 100644 --- a/softmmu/memory_mapping.c +++ b/softmmu/memory_mapping.c @@ -19,7 +19,7 @@ #include "exec/memory.h" #include "exec/address-spaces.h" -//#define DEBUG_GUEST_PHYS_REGION_ADD +/* #define DEBUG_GUEST_PHYS_REGION_ADD */ static void memory_mapping_list_add_mapping_sorted(MemoryMappingList *list, MemoryMapping *mapping) diff --git a/softmmu/physmem.c b/softmmu/physmem.c index 44ffb60b5d..78c1b6580a 100644 --- a/softmmu/physmem.c +++ b/softmmu/physmem.c @@ -75,7 +75,7 @@ #include #endif -//#define DEBUG_SUBPAGE +/* #define DEBUG_SUBPAGE */ /* ram_list is read under rcu_read_lock()/rcu_read_unlock(). Writes * are protected by the ramlist lock. --
[PATCH for-5.2] tcg: Remove assert from set_jmp_reset_offset
The range check done here is done later, more appropriately,
at the end of tcg_gen_code. There, a failing range check
results in a returned error code, which causes the TB to be
restarted at half the size.
Reported-by: Sai Pavan Boddu
Signed-off-by: Richard Henderson
---
Sai, would you try this against your failing testcase?
r~
---
tcg/tcg.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/tcg/tcg.c b/tcg/tcg.c
index f49f1a7f35..43c6cf8f52 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -335,10 +335,11 @@ static bool tcg_resolve_relocs(TCGContext *s)
static void set_jmp_reset_offset(TCGContext *s, int which)
{
-size_t off = tcg_current_code_size(s);
-s->tb_jmp_reset_offset[which] = off;
-/* Make sure that we didn't overflow the stored offset. */
-assert(s->tb_jmp_reset_offset[which] == off);
+/*
+ * We will check for overflow at the end of the opcode loop in
+ * tcg_gen_code, where we bound tcg_current_code_size to UINT16_MAX.
+ */
+s->tb_jmp_reset_offset[which] = tcg_current_code_size(s);
}
#include "tcg-target.c.inc"
--
2.25.1
Re: [PATCH 0/4] qga: Fix some style problems
Kindly ping.
On 2020/10/26 17:05, AlexChen wrote:
> Fix some error style problems found by checkpatch.pl.
>
> alexchen (4):
> qga: Add spaces around operator
> qga: Delete redundant spaces
> qga: Open brace '{' following struct go on the same
> qga: switch and case should be at the same indent
>
> qga/channel-win32.c | 6 ++---
> qga/commands-posix.c | 4 +--
> qga/commands-win32.c | 28 ++---
> qga/commands.c | 4 +--
> qga/main.c | 59 ++--
> 5 files changed, 50 insertions(+), 51 deletions(-)
>
Re: [PATCH] crypto: Add spaces around operator
Patchew URL:
https://patchew.org/QEMU/bb45d748-01e4-2fde-9fe3-32a7db4b6...@huawei.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: bb45d748-01e4-2fde-9fe3-32a7db4b6...@huawei.com
Subject: [PATCH] crypto: Add spaces around operator
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
* [new tag] patchew/bb45d748-01e4-2fde-9fe3-32a7db4b6...@huawei.com ->
patchew/bb45d748-01e4-2fde-9fe3-32a7db4b6...@huawei.com
Switched to a new branch 'test'
53a011e crypto: Add spaces around operator
=== OUTPUT BEGIN ===
ERROR: braces {} are necessary for all arms of this statement
#24: FILE: crypto/aes.c:1083:
+if (bits == 128)
[...]
-else if (bits==192)
[...]
key->rounds = 12;
[...]
ERROR: braces {} are necessary for all arms of this statement
#27: FILE: crypto/aes.c:1085:
+else if (bits == 192)
[...]
else
[...]
ERROR: space prohibited after that open parenthesis '('
#49: FILE: crypto/desrfb.c:96:
+if( pcr[pc2[j + 24]] ) kn[n] |= bigbyte[j];
ERROR: space prohibited before that close parenthesis ')'
#49: FILE: crypto/desrfb.c:96:
+if( pcr[pc2[j + 24]] ) kn[n] |= bigbyte[j];
ERROR: space required before the open parenthesis '('
#49: FILE: crypto/desrfb.c:96:
+if( pcr[pc2[j + 24]] ) kn[n] |= bigbyte[j];
ERROR: trailing statements should be on next line
#49: FILE: crypto/desrfb.c:96:
+if( pcr[pc2[j + 24]] ) kn[n] |= bigbyte[j];
ERROR: braces {} are necessary for all arms of this statement
#49: FILE: crypto/desrfb.c:96:
+if( pcr[pc2[j + 24]] ) kn[n] |= bigbyte[j];
[...]
total: 7 errors, 0 warnings, 35 lines checked
Commit 53a011edf848 (crypto: Add spaces around operator) has style problems,
please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/bb45d748-01e4-2fde-9fe3-32a7db4b6...@huawei.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com
[PATCH] crypto: Add spaces around operator
I am reading crypto related code, find some code style problems while
using checkpatch.pl to check crypto folder. Fix the error style
problems.
Signed-off-by: Liyang Shi
---
crypto/aes.c | 6 +++---
crypto/desrfb.c | 2 +-
crypto/tlscredsx509.c | 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/crypto/aes.c b/crypto/aes.c
index 159800df65..af72ff7779 100644
--- a/crypto/aes.c
+++ b/crypto/aes.c
@@ -1080,9 +1080,9 @@ int AES_set_encrypt_key(const unsigned char *userKey,
const int bits,
rk = key->rd_key;
-if (bits==128)
+if (bits == 128)
key->rounds = 10;
-else if (bits==192)
+else if (bits == 192)
key->rounds = 12;
else
key->rounds = 14;
@@ -1182,7 +1182,7 @@ int AES_set_decrypt_key(const unsigned char *userKey,
const int bits,
rk = key->rd_key;
/* invert the order of the round keys: */
-for (i = 0, j = 4*(key->rounds); i < j; i += 4, j -= 4) {
+for (i = 0, j = 4 * (key->rounds); i < j; i += 4, j -= 4) {
temp = rk[i]; rk[i] = rk[j]; rk[j] = temp;
temp = rk[i + 1]; rk[i + 1] = rk[j + 1]; rk[j + 1] = temp;
temp = rk[i + 2]; rk[i + 2] = rk[j + 2]; rk[j + 2] = temp;
diff --git a/crypto/desrfb.c b/crypto/desrfb.c
index 3274c36510..14288fcf96 100644
--- a/crypto/desrfb.c
+++ b/crypto/desrfb.c
@@ -93,7 +93,7 @@ void deskey(unsigned char *key, int edf)
}
for( j = 0; j < 24; j++ ) {
if( pcr[pc2[j]] ) kn[m] |= bigbyte[j];
-if( pcr[pc2[j+24]] ) kn[n] |= bigbyte[j];
+if( pcr[pc2[j + 24]] ) kn[n] |= bigbyte[j];
}
}
cookey(kn);
diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
index dd7267ccdb..c89dd62435 100644
--- a/crypto/tlscredsx509.c
+++ b/crypto/tlscredsx509.c
@@ -143,7 +143,7 @@ qcrypto_tls_creds_check_cert_key_usage(QCryptoTLSCredsX509
*creds,
if (status < 0) {
if (status == GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE) {
usage = isCA ? GNUTLS_KEY_KEY_CERT_SIGN :
-GNUTLS_KEY_DIGITAL_SIGNATURE|GNUTLS_KEY_KEY_ENCIPHERMENT;
+GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT;
} else {
error_setg(errp,
"Unable to query certificate %s key usage: %s",
--
2.17.1.windows.2
Re: [RFC PATCH v2 07/13] hw/arm/virt-acpi-build: distinguish possible and present cpus Message
On 10/30/2020 1:20 AM, Andrew Jones wrote:
You need to remove 'Message' from the summary.
On Tue, Oct 20, 2020 at 09:14:34PM +0800, Ying Fang wrote:
When building ACPI tables regarding CPUs we should always build
them for the number of possible CPUs, not the number of present
CPUs. We then ensure only the present CPUs are enabled.
Signed-off-by: Andrew Jones
I guess my s-o-b is here because this is a rework of
https://github.com/rhdrjones/qemu/commit/b18d7a889f424b8a8679c43d7f4804fdeeeaf3fd
I think it changed enough you could just drop my authorship. A
based-on comment in the commit message would be more than enough.
Comment on the patch below.
Signed-off-by: Ying Fang
---
hw/arm/virt-acpi-build.c | 17 -
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index a222981737..fae5a26741 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -57,14 +57,18 @@
#define ARM_SPI_BASE 32
-static void acpi_dsdt_add_cpus(Aml *scope, int cpus)
+static void acpi_dsdt_add_cpus(Aml *scope, VirtMachineState *vms)
{
uint16_t i;
+CPUArchIdList *possible_cpus = MACHINE(vms)->possible_cpus;
-for (i = 0; i < cpus; i++) {
+for (i = 0; i < possible_cpus->len; i++) {
Aml *dev = aml_device("C%.03X", i);
aml_append(dev, aml_name_decl("_HID", aml_string("ACPI0007")));
aml_append(dev, aml_name_decl("_UID", aml_int(i)));
+if (possible_cpus->cpus[i].cpu == NULL) {
+aml_append(dev, aml_name_decl("_STA", aml_int(0)));
+}
aml_append(scope, dev);
}
}
@@ -470,6 +474,7 @@ build_madt(GArray *table_data, BIOSLinker *linker,
VirtMachineState *vms)
const int *irqmap = vms->irqmap;
AcpiMadtGenericDistributor *gicd;
AcpiMadtGenericMsiFrame *gic_msi;
+int possible_cpus = MACHINE(vms)->possible_cpus->len;
int i;
acpi_data_push(table_data, sizeof(AcpiMultipleApicTable));
@@ -480,7 +485,7 @@ build_madt(GArray *table_data, BIOSLinker *linker,
VirtMachineState *vms)
gicd->base_address = cpu_to_le64(memmap[VIRT_GIC_DIST].base);
gicd->version = vms->gic_version;
-for (i = 0; i < MACHINE(vms)->smp.cpus; i++) {
+for (i = 0; i < possible_cpus; i++) {
AcpiMadtGenericCpuInterface *gicc = acpi_data_push(table_data,
sizeof(*gicc));
ARMCPU *armcpu = ARM_CPU(qemu_get_cpu(i));
@@ -495,7 +500,9 @@ build_madt(GArray *table_data, BIOSLinker *linker,
VirtMachineState *vms)
gicc->cpu_interface_number = cpu_to_le32(i);
gicc->arm_mpidr = cpu_to_le64(armcpu->mp_affinity);
gicc->uid = cpu_to_le32(i);
-gicc->flags = cpu_to_le32(ACPI_MADT_GICC_ENABLED);
+if (i < MACHINE(vms)->smp.cpus) {
Shouldn't this be
if (possible_cpus->cpus[i].cpu != NULL) {
+gicc->flags = cpu_to_le32(ACPI_MADT_GICC_ENABLED);
+}
I now realized that I switched to use current cpu number as the limit to
make GIC flags enabled here.
However to judge NULL is much more suitable here.
Thanks,
Ying.
if (arm_feature(>env, ARM_FEATURE_PMU)) {
gicc->performance_interrupt = cpu_to_le32(PPI(VIRTUAL_PMU_IRQ));
@@ -599,7 +606,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
VirtMachineState *vms)
* the RTC ACPI device at all when using UEFI.
*/
scope = aml_scope("\\_SB");
-acpi_dsdt_add_cpus(scope, ms->smp.cpus);
+acpi_dsdt_add_cpus(scope, vms);
acpi_dsdt_add_uart(scope, [VIRT_UART],
(irqmap[VIRT_UART] + ARM_SPI_BASE));
if (vmc->acpi_expose_flash) {
--
2.23.0
Thanks,
drew
.
[PULL v3 08/12] glib-compat: add g_unix_get_passwd_entry_qemu()
From: Marc-André Lureau
The glib function was introduced in 2.64. It's a safer version of
getpwnam, and also simpler to use than getpwnam_r.
Currently, it's only use by the next patch in qemu-ga, which doesn't
(well well...) need the thread safety guarantees. Since the fallback
version is still unsafe, I would rather keep the _qemu postfix, to make
sure it's not being misused by mistake. When/if necessary, we can
implement a safer fallback and drop the _qemu suffix.
Signed-off-by: Marc-André Lureau
Reviewed-by: Michal Privoznik
*fix checkpatch warnings about newlines before/after block comments
Signed-off-by: Michael Roth
---
include/glib-compat.h | 28
1 file changed, 28 insertions(+)
diff --git a/include/glib-compat.h b/include/glib-compat.h
index 0b0ec76299..695a96f7ea 100644
--- a/include/glib-compat.h
+++ b/include/glib-compat.h
@@ -30,6 +30,11 @@
#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
#include
+#if defined(G_OS_UNIX)
+#include
+#include
+#include
+#endif
/*
* Note that because of the GLIB_VERSION_MAX_ALLOWED constant above, allowing
@@ -72,6 +77,29 @@
gint g_poll_fixed(GPollFD *fds, guint nfds, gint timeout);
#endif
+#if defined(G_OS_UNIX)
+/*
+ * Note: The fallback implementation is not MT-safe, and it returns a copy of
+ * the libc passwd (must be g_free() after use) but not the content. Because of
+ * these important differences the caller must be aware of, it's not #define
for
+ * GLib API substitution.
+ */
+static inline struct passwd *
+g_unix_get_passwd_entry_qemu(const gchar *user_name, GError **error)
+{
+#if GLIB_CHECK_VERSION(2, 64, 0)
+return g_unix_get_passwd_entry(user_name, error);
+#else
+struct passwd *p = getpwnam(user_name);
+if (!p) {
+g_set_error_literal(error, G_UNIX_ERROR, 0, g_strerror(errno));
+return NULL;
+}
+return (struct passwd *)g_memdup(p, sizeof(*p));
+#endif
+}
+#endif /* G_OS_UNIX */
+
#pragma GCC diagnostic pop
#endif
--
2.25.1
[PULL v3 05/12] qga: add command guest-get-disks
From: Tomáš Golembiovský
Add API and stubs for new guest-get-disks command.
The command guest-get-fsinfo can be used to list information about disks
and partitions but it is limited only to mounted disks with filesystem.
This new command should allow listing information about disks of the VM
regardles whether they are mounted or not. This can be usefull for
management applications for mapping virtualized devices or pass-through
devices to device names in the guest OS.
Signed-off-by: Tomáš Golembiovský
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Marc-André Lureau
Signed-off-by: Michael Roth
---
qga/commands-posix.c | 6 ++
qga/commands-win32.c | 6 ++
qga/qapi-schema.json | 31 +++
3 files changed, 43 insertions(+)
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 3bffee99d4..422144bcff 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -3051,3 +3051,9 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
return NULL;
}
+
+GuestDiskInfoList *qmp_guest_get_disks(Error **errp)
+{
+error_setg(errp, QERR_UNSUPPORTED);
+return NULL;
+}
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 0c33d48aaa..f7bdd5a8b5 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -2458,3 +2458,9 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
}
return head;
}
+
+GuestDiskInfoList *qmp_guest_get_disks(Error **errp)
+{
+error_setg(errp, QERR_UNSUPPORTED);
+return NULL;
+}
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index fe10631e4c..e123a000be 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -865,6 +865,37 @@
'bus': 'int', 'target': 'int', 'unit': 'int',
'*serial': 'str', '*dev': 'str'} }
+##
+# @GuestDiskInfo:
+#
+# @name: device node (Linux) or device UNC (Windows)
+# @partition: whether this is a partition or disk
+# @dependents: list of dependent devices; e.g. for LVs of the LVM this will
+# hold the list of PVs, for LUKS encrypted volume this will
+# contain the disk where the volume is placed. (Linux)
+# @address: disk address information (only for non-virtual devices)
+# @alias: optional alias assigned to the disk, on Linux this is a name assigned
+# by device mapper
+#
+# Since 5.2
+##
+{ 'struct': 'GuestDiskInfo',
+ 'data': {'name': 'str', 'partition': 'bool', 'dependents': ['str'],
+ '*address': 'GuestDiskAddress', '*alias': 'str'} }
+
+##
+# @guest-get-disks:
+#
+# Returns: The list of disks in the guest. For Windows these are only the
+# physical disks. On Linux these are all root block devices of
+# non-zero size including e.g. removable devices, loop devices,
+# NBD, etc.
+#
+# Since: 5.2
+##
+{ 'command': 'guest-get-disks',
+ 'returns': ['GuestDiskInfo'] }
+
##
# @GuestFilesystemInfo:
#
--
2.25.1
[PULL v3 07/12] qga: add implementation of guest-get-disks for Windows
From: Tomáš Golembiovský
The command lists all the physical disk drives. Unlike for Linux
partitions and virtual volumes are not listed.
Example output:
{
"return": [
{
"name": ".\\PhysicalDrive0",
"partition": false,
"address": {
"serial": "QM1",
"bus-type": "sata",
...
},
"dependents": []
}
]
}
Signed-off-by: Tomáš Golembiovský
Signed-off-by: Michael Roth
---
qga/commands-win32.c | 107 ---
1 file changed, 101 insertions(+), 6 deletions(-)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index f7bdd5a8b5..300b87c859 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -979,6 +979,101 @@ out:
return list;
}
+GuestDiskInfoList *qmp_guest_get_disks(Error **errp)
+{
+ERRP_GUARD();
+GuestDiskInfoList *new = NULL, *ret = NULL;
+HDEVINFO dev_info;
+SP_DEVICE_INTERFACE_DATA dev_iface_data;
+int i;
+
+dev_info = SetupDiGetClassDevs(_DEVINTERFACE_DISK, 0, 0,
+DIGCF_PRESENT | DIGCF_DEVICEINTERFACE);
+if (dev_info == INVALID_HANDLE_VALUE) {
+error_setg_win32(errp, GetLastError(), "failed to get device tree");
+return NULL;
+}
+
+g_debug("enumerating devices");
+dev_iface_data.cbSize = sizeof(SP_DEVICE_INTERFACE_DATA);
+for (i = 0;
+SetupDiEnumDeviceInterfaces(dev_info, NULL, _DEVINTERFACE_DISK,
+i, _iface_data);
+i++) {
+GuestDiskAddress *address = NULL;
+GuestDiskInfo *disk = NULL;
+Error *local_err = NULL;
+g_autofree PSP_DEVICE_INTERFACE_DETAIL_DATA
+pdev_iface_detail_data = NULL;
+STORAGE_DEVICE_NUMBER sdn;
+HANDLE dev_file;
+DWORD size = 0;
+BOOL result;
+int attempt;
+
+g_debug(" getting device path");
+for (attempt = 0, result = FALSE; attempt < 2 && !result; attempt++) {
+result = SetupDiGetDeviceInterfaceDetail(dev_info,
+_iface_data, pdev_iface_detail_data, size, , NULL);
+if (result) {
+break;
+}
+if (GetLastError() == ERROR_INSUFFICIENT_BUFFER) {
+pdev_iface_detail_data = g_realloc(pdev_iface_detail_data,
+size);
+pdev_iface_detail_data->cbSize =
+sizeof(*pdev_iface_detail_data);
+} else {
+g_debug("failed to get device interface details");
+break;
+}
+}
+if (!result) {
+g_debug("skipping device");
+continue;
+}
+
+g_debug(" device: %s", pdev_iface_detail_data->DevicePath);
+dev_file = CreateFile(pdev_iface_detail_data->DevicePath, 0,
+FILE_SHARE_READ, NULL, OPEN_EXISTING, 0, NULL);
+if (!DeviceIoControl(dev_file, IOCTL_STORAGE_GET_DEVICE_NUMBER,
+NULL, 0, , sizeof(sdn), , NULL)) {
+CloseHandle(dev_file);
+debug_error("failed to get storage device number");
+continue;
+}
+CloseHandle(dev_file);
+
+disk = g_new0(GuestDiskInfo, 1);
+disk->name = g_strdup_printf(".\\PhysicalDrive%lu",
+sdn.DeviceNumber);
+
+g_debug(" number: %lu", sdn.DeviceNumber);
+address = g_malloc0(sizeof(GuestDiskAddress));
+address->has_dev = true;
+address->dev = g_strdup(disk->name);
+get_single_disk_info(sdn.DeviceNumber, address, _err);
+if (local_err) {
+g_debug("failed to get disk info: %s",
+error_get_pretty(local_err));
+error_free(local_err);
+qapi_free_GuestDiskAddress(address);
+address = NULL;
+} else {
+disk->address = address;
+disk->has_address = true;
+}
+
+new = g_malloc0(sizeof(GuestDiskInfoList));
+new->value = disk;
+new->next = ret;
+ret = new;
+}
+
+SetupDiDestroyDeviceInfoList(dev_info);
+return ret;
+}
+
#else
static GuestDiskAddressList *build_guest_disk_info(char *guid, Error **errp)
@@ -986,6 +1081,12 @@ static GuestDiskAddressList *build_guest_disk_info(char
*guid, Error **errp)
return NULL;
}
+GuestDiskInfoList *qmp_guest_get_disks(Error **errp)
+{
+error_setg(errp, QERR_UNSUPPORTED);
+return NULL;
+}
+
#endif /* CONFIG_QGA_NTDDSCSI */
static GuestFilesystemInfo *build_guest_fsinfo(char *guid, Error **errp)
@@ -2458,9 +2559,3 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
}
return head;
}
-
-GuestDiskInfoList *qmp_guest_get_disks(Error **errp)
-{
-error_setg(errp, QERR_UNSUPPORTED);
-return NULL;
-}
--
2.25.1
[PULL v3 02/12] qga: Use common time encoding for guest-get-devices 'driver-date'
From: Markus Armbruster
guest-get-devices returns 'driver-date' as string in the format
-MM-DD. Goes back to recent commit 2e4211cee4 "qga: add command
guest-get-devices for reporting VirtIO devices".
We should avoid use of multiple encodings for the same kind of data.
Especially string encodings. Change it to return nanoseconds since
the epoch, like guest-get-time does.
Signed-off-by: Markus Armbruster
Reviewed-by: Marc-André Lureau
Signed-off-by: Michael Roth
---
qga/commands-win32.c | 19 +++
qga/qapi-schema.json | 4 ++--
2 files changed, 13 insertions(+), 10 deletions(-)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 879b02b6c3..b01616a992 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -1641,6 +1641,12 @@ out:
return head;
}
+static int64_t filetime_to_ns(const FILETIME *tf)
+{
+return int64_t)tf->dwHighDateTime << 32) | tf->dwLowDateTime)
+- W32_FT_OFFSET) * 100;
+}
+
int64_t qmp_guest_get_time(Error **errp)
{
SYSTEMTIME ts = {0};
@@ -1657,8 +1663,7 @@ int64_t qmp_guest_get_time(Error **errp)
return -1;
}
-return int64_t)tf.dwHighDateTime << 32) | tf.dwLowDateTime)
-- W32_FT_OFFSET) * 100;
+return filetime_to_ns();
}
void qmp_guest_set_time(bool has_time, int64_t time_ns, Error **errp)
@@ -2363,7 +2368,6 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
slog("enumerating devices");
for (i = 0; SetupDiEnumDeviceInfo(dev_info, i, _info_data); i++) {
bool skip = true;
-SYSTEMTIME utc_date;
g_autofree LPWSTR name = NULL;
g_autofree LPFILETIME date = NULL;
g_autofree LPWSTR version = NULL;
@@ -2434,13 +2438,12 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
slog("failed to get driver date");
continue;
}
-FileTimeToSystemTime(date, _date);
-device->driver_date = g_strdup_printf("%04d-%02d-%02d",
-utc_date.wYear, utc_date.wMonth, utc_date.wDay);
+device->driver_date = filetime_to_ns(date);
device->has_driver_date = true;
-slog("driver: %s\ndriver version: %s,%s\n", device->driver_name,
-device->driver_date, device->driver_version);
+slog("driver: %s\ndriver version: %" PRId64 ",%s\n",
+ device->driver_name, device->driver_date,
+ device->driver_version);
item = g_new0(GuestDeviceInfoList, 1);
item->value = g_steal_pointer();
if (!cur_item) {
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index f2c81cda2b..c7bfb8bf6a 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -1282,7 +1282,7 @@
# @GuestDeviceInfo:
#
# @driver-name: name of the associated driver
-# @driver-date: driver release date in format -MM-DD
+# @driver-date: driver release date, in nanoseconds since the epoch
# @driver-version: driver version
# @id: device ID
#
@@ -1291,7 +1291,7 @@
{ 'struct': 'GuestDeviceInfo',
'data': {
'driver-name': 'str',
- '*driver-date': 'str',
+ '*driver-date': 'int',
'*driver-version': 'str',
'*id': 'GuestDeviceId'
} }
--
2.25.1
[PULL v3 04/12] qga: Flatten simple union GuestDeviceId
From: Markus Armbruster
Simple unions are simpler than flat unions in the schema, but more
complicated in C and on the QMP wire: there's extra indirection in C
and extra nesting on the wire, both pointless. They should be avoided
in new code.
GuestDeviceId was recently added for guest-get-devices. Convert it to
a flat union.
Signed-off-by: Markus Armbruster
Reviewed-by: Marc-André Lureau
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Michael Roth
---
qga/commands-win32.c | 9 -
qga/qapi-schema.json | 8
2 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 1efe3ba076..0c33d48aaa 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -2400,16 +2400,15 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
}
skip = false;
-id = g_new0(GuestDeviceIdPCI, 1);
vendor_id = g_match_info_fetch(match_info, 1);
device_id = g_match_info_fetch(match_info, 2);
-id->vendor_id = g_ascii_strtoull(vendor_id, NULL, 16);
-id->device_id = g_ascii_strtoull(device_id, NULL, 16);
device->id = g_new0(GuestDeviceId, 1);
device->has_id = true;
-device->id->type = GUEST_DEVICE_ID_KIND_PCI;
-device->id->u.pci.data = id;
+device->id->type = GUEST_DEVICE_TYPE_PCI;
+id = >id->u.pci;
+id->vendor_id = g_ascii_strtoull(vendor_id, NULL, 16);
+id->device_id = g_ascii_strtoull(device_id, NULL, 16);
g_match_info_free(match_info);
break;
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index c7bfb8bf6a..fe10631e4c 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -1256,6 +1256,12 @@
{ 'command': 'guest-get-osinfo',
'returns': 'GuestOSInfo' }
+##
+# @GuestDeviceType:
+##
+{ 'enum': 'GuestDeviceType',
+ 'data': [ 'pci' ] }
+
##
# @GuestDeviceIdPCI:
#
@@ -1276,6 +1282,8 @@
# Since: 5.2
##
{ 'union': 'GuestDeviceId',
+ 'base': { 'type': 'GuestDeviceType' },
+ 'discriminator': 'type',
'data': { 'pci': 'GuestDeviceIdPCI' } }
##
--
2.25.1
[PULL v3 03/12] qga-win: Fix guest-get-devices error API violations
From: Markus Armbruster
The Error ** argument must be NULL, _abort, _fatal, or a
pointer to a variable containing NULL. Passing an argument of the
latter kind twice without clearing it in between is wrong: if the
first call sets an error, it no longer points to NULL for the second
call.
qmp_guest_get_devices() is wrong that way: it calls error_setg() in a
loop.
If no iteration fails, the function returns a value and sets no error.
Okay.
If exactly one iteration fails, the function returns a value and sets
an error. Wrong.
If multiple iterations fail, the function trips error_setv()'s
assertion.
Fix it to return immediately on error.
Perhaps the failure to convert the driver version to UTF-8 should not
be an error. We could simply not report the botched version string
instead.
Drop a superfluous continue while there.
Signed-off-by: Markus Armbruster
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Marc-André Lureau
Signed-off-by: Michael Roth
---
qga/commands-win32.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index b01616a992..1efe3ba076 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -2385,7 +2385,7 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
device->driver_name = g_utf16_to_utf8(name, -1, NULL, NULL, NULL);
if (device->driver_name == NULL) {
error_setg(errp, "conversion to utf8 failed (driver name)");
-continue;
+return NULL;
}
slog("querying device: %s", device->driver_name);
hw_ids = ga_get_hardware_ids(dev_info_data.DevInst);
@@ -2428,7 +2428,7 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
NULL, NULL);
if (device->driver_version == NULL) {
error_setg(errp, "conversion to utf8 failed (driver version)");
-continue;
+return NULL;
}
device->has_driver_version = true;
@@ -2452,7 +2452,6 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
cur_item->next = item;
cur_item = item;
}
-continue;
}
if (dev_info != INVALID_HANDLE_VALUE) {
--
2.25.1
[PULL v3 00/12] qemu-ga patch queue for soft-freeze
Hi Peter,
Sorry to get these out so late, for some inexplicable reason my email
client decided to flag all responses v1 as spam and I didn't notice
until I double-checked the archives this morning.
I've fixed the gcc-on-BSD and clang-on-linux issues you pointed out
(PATCH 6) and added proper test coverage for both.
Also, the qga-ssh-test unit test introduced in this series triggers a
failure in Gitlab CI build-oss-fuzz test. This seems to be due to a
memory leak in GLib itself when G_TEST_OPTION_ISOLATE_DIRS option is
passed to g_test_init(). I verified the unit test doesn't introduce any
leaks when run without g_test* harness. Since G_TEST_OPTION_ISOLATE_DIRS
is currently needed to safely run the qga-ssh-test, I've disabled it for
now (PATCH 9 and 12) until we figure out solution.
And finally (hopefully), I addressed the checkpatch warning regarding
disallowed use of various g_assert_* macros. I previously thought they
were just noise until Markus pointed out commit 6e9389563e.
Sorry for all the noise,
Mike
The following changes since commit 2c6605389c1f76973d92b69b85d40d94b8f1092c:
Merge remote-tracking branch 'remotes/awilliam/tags/vfio-update-20201101.0'
into staging (2020-11-02 09:54:00 +)
are available in the Git repository at:
git://github.com/mdroth/qemu.git tags/qga-pull-2020-10-27-v3-tag
for you to fetch changes up to cad97c08a1c17830d77a46780088bc0199df89d1:
qga: add ssh-get-authorized-keys (2020-11-02 20:04:13 -0600)
qemu-ga patch queue for soft-freeze
* add guest-get-disks for w32/linux
* add guest-{add,remove,get}-authorized-keys
* fix API violations and schema documentation inconsistencies with
recently-added guest-get-devices
v3:
- fix checkpatch errors regarding disallowed usages of g_assert*
macros and other warnings
v2:
- fix BSD build error due to missing stub for guest_get_disks
- fix clang build error on linux due to unused variable
- disable qga-ssh-test for now due to a memory leak within GLib when
G_TEST_OPTION_ISOLATE_DIRS is passed to g_test_init() since it
break Gitlab CI build-oss-fuzz test
- rebased and re-tested on master
Marc-André Lureau (4):
glib-compat: add g_unix_get_passwd_entry_qemu()
qga: add ssh-{add,remove}-authorized-keys
meson: minor simplification
qga: add ssh-get-authorized-keys
Markus Armbruster (4):
qga: Rename guest-get-devices return member 'address' to 'id'
qga: Use common time encoding for guest-get-devices 'driver-date'
qga-win: Fix guest-get-devices error API violations
qga: Flatten simple union GuestDeviceId
Michael Roth (1):
qga: add *reset argument to ssh-add-authorized-keys
Tomáš Golembiovský (3):
qga: add command guest-get-disks
qga: add implementation of guest-get-disks for Linux
qga: add implementation of guest-get-disks for Windows
include/glib-compat.h| 28 +++
qga/commands-posix-ssh.c | 516 +++
qga/commands-posix.c | 297 ++-
qga/commands-win32.c | 140 +++--
qga/meson.build | 39 +++-
qga/qapi-schema.json | 127 +++-
6 files changed, 1106 insertions(+), 41 deletions(-)
create mode 100644 qga/commands-posix-ssh.c
[PULL v3 10/12] qga: add *reset argument to ssh-add-authorized-keys
I prefer 'reset' over 'clear', since 'clear' and keys may have some
other relations or meaning.
Signed-off-by: Marc-André Lureau
*fix disallowed g_assert* usage reported by checkpatch
Signed-off-by: Michael Roth
---
qga/commands-posix-ssh.c | 53
qga/qapi-schema.json | 3 ++-
2 files changed, 50 insertions(+), 6 deletions(-)
diff --git a/qga/commands-posix-ssh.c b/qga/commands-posix-ssh.c
index f74d89679c..362c9e8816 100644
--- a/qga/commands-posix-ssh.c
+++ b/qga/commands-posix-ssh.c
@@ -168,6 +168,7 @@ read_authkeys(const char *path, Error **errp)
void
qmp_guest_ssh_add_authorized_keys(const char *username, strList *keys,
+ bool has_reset, bool reset,
Error **errp)
{
g_autofree struct passwd *p = NULL;
@@ -178,6 +179,7 @@ qmp_guest_ssh_add_authorized_keys(const char *username,
strList *keys,
size_t nkeys, nauthkeys;
ERRP_GUARD();
+reset = has_reset && reset;
if (!check_openssh_pub_keys(keys, , errp)) {
return;
@@ -191,7 +193,9 @@ qmp_guest_ssh_add_authorized_keys(const char *username,
strList *keys,
ssh_path = g_build_filename(p->pw_dir, ".ssh", NULL);
authkeys_path = g_build_filename(ssh_path, "authorized_keys", NULL);
-authkeys = read_authkeys(authkeys_path, NULL);
+if (!reset) {
+authkeys = read_authkeys(authkeys_path, NULL);
+}
if (authkeys == NULL) {
if (!g_file_test(ssh_path, G_FILE_TEST_IS_DIR) &&
!mkdir_for_user(ssh_path, p, 0700, errp)) {
@@ -318,7 +322,7 @@ test_invalid_user(void)
{
Error *err = NULL;
-qmp_guest_ssh_add_authorized_keys("", NULL, );
+qmp_guest_ssh_add_authorized_keys("", NULL, FALSE, FALSE, );
error_free_or_abort();
qmp_guest_ssh_remove_authorized_keys("", NULL, );
@@ -333,7 +337,8 @@ test_invalid_key(void)
};
Error *err = NULL;
-qmp_guest_ssh_add_authorized_keys(g_get_user_name(), , );
+qmp_guest_ssh_add_authorized_keys(g_get_user_name(), ,
+ FALSE, FALSE, );
error_free_or_abort();
qmp_guest_ssh_remove_authorized_keys(g_get_user_name(), , );
@@ -346,13 +351,17 @@ test_add_keys(void)
Error *err = NULL;
qmp_guest_ssh_add_authorized_keys(g_get_user_name(),
- (strList *)_key2, );
+ (strList *)_key2,
+ FALSE, FALSE,
+ );
g_assert(err == NULL);
test_authorized_keys_equal("algo key2 comments");
qmp_guest_ssh_add_authorized_keys(g_get_user_name(),
- (strList *)_key1_2, );
+ (strList *)_key1_2,
+ FALSE, FALSE,
+ );
g_assert(err == NULL);
/* key2 came first, and should'nt be duplicated */
@@ -360,6 +369,39 @@ test_add_keys(void)
"algo key1 comments");
}
+static void
+test_add_reset_keys(void)
+{
+Error *err = NULL;
+
+qmp_guest_ssh_add_authorized_keys(g_get_user_name(),
+ (strList *)_key1_2,
+ FALSE, FALSE,
+ );
+g_assert(err == NULL);
+
+/* reset with key2 only */
+test_authorized_keys_equal("algo key1 comments\n"
+ "algo key2 comments");
+
+qmp_guest_ssh_add_authorized_keys(g_get_user_name(),
+ (strList *)_key2,
+ TRUE, TRUE,
+ );
+g_assert(err == NULL);
+
+test_authorized_keys_equal("algo key2 comments");
+
+/* empty should clear file */
+qmp_guest_ssh_add_authorized_keys(g_get_user_name(),
+ (strList *)NULL,
+ TRUE, TRUE,
+ );
+g_assert(err == NULL);
+
+test_authorized_keys_equal("");
+}
+
static void
test_remove_keys(void)
{
@@ -393,6 +435,7 @@ int main(int argc, char *argv[])
g_test_add_func("/qga/ssh/invalid_user", test_invalid_user);
g_test_add_func("/qga/ssh/invalid_key", test_invalid_key);
g_test_add_func("/qga/ssh/add_keys", test_add_keys);
+g_test_add_func("/qga/ssh/add_reset_keys", test_add_reset_keys);
g_test_add_func("/qga/ssh/remove_keys", test_remove_keys);
return g_test_run();
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index a2727ed86b..4ddea898fa 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -1352,6 +1352,7 @@
#
# @username: the user account to add the authorized keys
# @keys: the public keys to add (in OpenSSH/sshd(8) authorized_keys format)
+# @reset: ignore the existing content, set it with the given keys only
#
[PULL v3 01/12] qga: Rename guest-get-devices return member 'address' to 'id'
From: Markus Armbruster
Member 'address' is union GuestDeviceAddress with a single branch
GuestDeviceAddressPCI, containing PCI vendor ID and device ID. This
is not a PCI address. Type GuestPCIAddress is. Messed up in recent
commit 2e4211cee4 "qga: add command guest-get-devices for reporting
VirtIO devices".
Rename type GuestDeviceAddressPCI to GuestDeviceIdPCI, type
GuestDeviceAddress to GuestDeviceId, and member 'address' to 'id'.
Document the member properly while there.
Signed-off-by: Markus Armbruster
Reviewed-by: Marc-André Lureau
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Michael Roth
---
qga/commands-win32.c | 16
qga/qapi-schema.json | 17 +
2 files changed, 17 insertions(+), 16 deletions(-)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 0c3c05484f..879b02b6c3 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -2390,22 +2390,22 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
}
for (j = 0; hw_ids[j] != NULL; j++) {
GMatchInfo *match_info;
-GuestDeviceAddressPCI *address;
+GuestDeviceIdPCI *id;
if (!g_regex_match(device_pci_re, hw_ids[j], 0, _info)) {
continue;
}
skip = false;
-address = g_new0(GuestDeviceAddressPCI, 1);
+id = g_new0(GuestDeviceIdPCI, 1);
vendor_id = g_match_info_fetch(match_info, 1);
device_id = g_match_info_fetch(match_info, 2);
-address->vendor_id = g_ascii_strtoull(vendor_id, NULL, 16);
-address->device_id = g_ascii_strtoull(device_id, NULL, 16);
+id->vendor_id = g_ascii_strtoull(vendor_id, NULL, 16);
+id->device_id = g_ascii_strtoull(device_id, NULL, 16);
-device->address = g_new0(GuestDeviceAddress, 1);
-device->has_address = true;
-device->address->type = GUEST_DEVICE_ADDRESS_KIND_PCI;
-device->address->u.pci.data = address;
+device->id = g_new0(GuestDeviceId, 1);
+device->has_id = true;
+device->id->type = GUEST_DEVICE_ID_KIND_PCI;
+device->id->u.pci.data = id;
g_match_info_free(match_info);
break;
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index cec98c7e06..f2c81cda2b 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -1257,26 +1257,26 @@
'returns': 'GuestOSInfo' }
##
-# @GuestDeviceAddressPCI:
+# @GuestDeviceIdPCI:
#
# @vendor-id: vendor ID
# @device-id: device ID
#
# Since: 5.2
##
-{ 'struct': 'GuestDeviceAddressPCI',
+{ 'struct': 'GuestDeviceIdPCI',
'data': { 'vendor-id': 'uint16', 'device-id': 'uint16' } }
##
-# @GuestDeviceAddress:
+# @GuestDeviceId:
#
-# Address of the device
-# - @pci: address of PCI device, since: 5.2
+# Id of the device
+# - @pci: PCI ID, since: 5.2
#
# Since: 5.2
##
-{ 'union': 'GuestDeviceAddress',
- 'data': { 'pci': 'GuestDeviceAddressPCI' } }
+{ 'union': 'GuestDeviceId',
+ 'data': { 'pci': 'GuestDeviceIdPCI' } }
##
# @GuestDeviceInfo:
@@ -1284,6 +1284,7 @@
# @driver-name: name of the associated driver
# @driver-date: driver release date in format -MM-DD
# @driver-version: driver version
+# @id: device ID
#
# Since: 5.2
##
@@ -1292,7 +1293,7 @@
'driver-name': 'str',
'*driver-date': 'str',
'*driver-version': 'str',
- '*address': 'GuestDeviceAddress'
+ '*id': 'GuestDeviceId'
} }
##
--
2.25.1
[PULL v3 06/12] qga: add implementation of guest-get-disks for Linux
From: Tomáš Golembiovský
The command lists all disks (real and virtual) as well as disk
partitions. For each disk the list of dependent disks is also listed and
/dev path is used as a handle so it can be matched with "name" field of
other returned disk entries. For disk partitions the "dependents" list
is populated with the the parent device for easier tracking of
hierarchy.
Example output:
{
"return": [
...
{
"name": "/dev/dm-0",
"partition": false,
"dependents": [
"/dev/sda2"
],
"alias": "luks-7062202e-5b9b-433e-81e8-6628c40da9f7"
},
{
"name": "/dev/sda2",
"partition": true,
"dependents": [
"/dev/sda"
]
},
{
"name": "/dev/sda",
"partition": false,
"address": {
"serial": "SAMSUNG_MZ7LN512HCHP-000L1_S1ZKNXAG822493",
"bus-type": "sata",
...
"dev": "/dev/sda",
"target": 0
},
"dependents": []
},
...
]
}
Signed-off-by: Tomáš Golembiovský
Reviewed-by: Marc-André Lureau
*add missing stub for !defined(CONFIG_FSFREEZE)
*remove unused deps_dir variable
Signed-off-by: Michael Roth
---
qga/commands-posix.c | 303 +--
1 file changed, 292 insertions(+), 11 deletions(-)
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 422144bcff..3711080d07 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -1150,13 +1150,27 @@ static void build_guest_fsinfo_for_virtual_device(char
const *syspath,
closedir(dir);
}
+static bool is_disk_virtual(const char *devpath, Error **errp)
+{
+g_autofree char *syspath = realpath(devpath, NULL);
+
+if (!syspath) {
+error_setg_errno(errp, errno, "realpath(\"%s\")", devpath);
+return false;
+}
+return strstr(syspath, "/devices/virtual/block/") != NULL;
+}
+
/* Dispatch to functions for virtual/real device */
static void build_guest_fsinfo_for_device(char const *devpath,
GuestFilesystemInfo *fs,
Error **errp)
{
-char *syspath = realpath(devpath, NULL);
+ERRP_GUARD();
+g_autofree char *syspath = NULL;
+bool is_virtual = false;
+syspath = realpath(devpath, NULL);
if (!syspath) {
error_setg_errno(errp, errno, "realpath(\"%s\")", devpath);
return;
@@ -1167,16 +1181,281 @@ static void build_guest_fsinfo_for_device(char const
*devpath,
}
g_debug(" parse sysfs path '%s'", syspath);
-
-if (strstr(syspath, "/devices/virtual/block/")) {
+is_virtual = is_disk_virtual(syspath, errp);
+if (*errp != NULL) {
+return;
+}
+if (is_virtual) {
build_guest_fsinfo_for_virtual_device(syspath, fs, errp);
} else {
build_guest_fsinfo_for_real_device(syspath, fs, errp);
}
+}
+
+#ifdef CONFIG_LIBUDEV
+
+/*
+ * Wrapper around build_guest_fsinfo_for_device() for getting just
+ * the disk address.
+ */
+static GuestDiskAddress *get_disk_address(const char *syspath, Error **errp)
+{
+g_autoptr(GuestFilesystemInfo) fs = NULL;
-free(syspath);
+fs = g_new0(GuestFilesystemInfo, 1);
+build_guest_fsinfo_for_device(syspath, fs, errp);
+if (fs->disk != NULL) {
+return g_steal_pointer(>disk->value);
+}
+return NULL;
}
+static char *get_alias_for_syspath(const char *syspath)
+{
+struct udev *udev = NULL;
+struct udev_device *udevice = NULL;
+char *ret = NULL;
+
+udev = udev_new();
+if (udev == NULL) {
+g_debug("failed to query udev");
+goto out;
+}
+udevice = udev_device_new_from_syspath(udev, syspath);
+if (udevice == NULL) {
+g_debug("failed to query udev for path: %s", syspath);
+goto out;
+} else {
+const char *alias = udev_device_get_property_value(
+udevice, "DM_NAME");
+/*
+ * NULL means there was an error and empty string means there is no
+ * alias. In case of no alias we return NULL instead of empty string.
+ */
+if (alias == NULL) {
+g_debug("failed to query udev for device alias for: %s",
+syspath);
+} else if (*alias != 0) {
+ret = g_strdup(alias);
+}
+}
+
+out:
+udev_unref(udev);
+udev_device_unref(udevice);
+return ret;
+}
+
+static char *get_device_for_syspath(const char *syspath)
+{
+struct udev *udev = NULL;
+struct udev_device *udevice = NULL;
+char *ret = NULL;
+
+udev = udev_new();
+if (udev == NULL) {
+g_debug("failed to query udev");
+goto out;
+}
+udevice = udev_device_new_from_syspath(udev, syspath);
+if (udevice == NULL) {
+g_debug("failed to query udev for path: %s", syspath);
+goto out;
+} else {
+ret = g_strdup(udev_device_get_devnode(udevice));
+}
+
+out:
+udev_unref(udev);
+udev_device_unref(udevice);
+
[PULL v3 11/12] meson: minor simplification
From: Marc-André Lureau
Signed-off-by: Marc-André Lureau
Signed-off-by: Michael Roth
---
qga/meson.build | 7 +--
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/qga/meson.build b/qga/meson.build
index 635de9af41..4cb3b3f259 100644
--- a/qga/meson.build
+++ b/qga/meson.build
@@ -22,12 +22,7 @@ qga_qapi_files = custom_target('QGA QAPI files',
depend_files: qapi_gen_depends)
qga_ss = ss.source_set()
-i = 0
-foreach output: qga_qapi_outputs
- qga_ss.add(qga_qapi_files[i])
- i = i + 1
-endforeach
-
+qga_ss.add(qga_qapi_files.to_list())
qga_ss.add(files(
'commands.c',
'guest-agent-command-state.c',
--
2.25.1
[PULL v3 09/12] qga: add ssh-{add,remove}-authorized-keys
From: Marc-André Lureau
Add new commands to add and remove SSH public keys from
~/.ssh/authorized_keys.
I took a different approach for testing, including the unit tests right
with the code. I wanted to overwrite the function to get the user
details, I couldn't easily do that over QMP. Furthermore, I prefer
having unit tests very close to the code, and unit files that are domain
specific (commands-posix is too crowded already). FWIW, that
coding/testing style is Rust-style (where tests can or should even be
part of the documentation!).
Fixes:
https://bugzilla.redhat.com/show_bug.cgi?id=1885332
Signed-off-by: Marc-André Lureau
Reviewed-by: Michal Privoznik
Reviewed-by: Daniel P. Berrangé
*squashed in fix-ups for setting file ownership and use of QAPI
conditionals for CONFIG_POSIX instead of stub definitions
*disable qga-ssh-test for now due to G_TEST_OPTION_ISOLATE_DIRS
triggering leak detector in build-oss-fuzz
*fix disallowed g_assert* usage reported by checkpatch
Signed-off-by: Michael Roth
---
qga/commands-posix-ssh.c | 407 +++
qga/meson.build | 25 ++-
qga/qapi-schema.json | 35
3 files changed, 466 insertions(+), 1 deletion(-)
create mode 100644 qga/commands-posix-ssh.c
diff --git a/qga/commands-posix-ssh.c b/qga/commands-posix-ssh.c
new file mode 100644
index 00..f74d89679c
--- /dev/null
+++ b/qga/commands-posix-ssh.c
@@ -0,0 +1,407 @@
+ /*
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#include "qemu/osdep.h"
+
+#include
+#include
+#include
+#include
+
+#include "qapi/error.h"
+#include "qga-qapi-commands.h"
+
+#ifdef QGA_BUILD_UNIT_TEST
+static struct passwd *
+test_get_passwd_entry(const gchar *user_name, GError **error)
+{
+struct passwd *p;
+int ret;
+
+if (!user_name || g_strcmp0(user_name, g_get_user_name())) {
+g_set_error(error, G_UNIX_ERROR, 0, "Invalid user name");
+return NULL;
+}
+
+p = g_new0(struct passwd, 1);
+p->pw_dir = (char *)g_get_home_dir();
+p->pw_uid = geteuid();
+p->pw_gid = getegid();
+
+ret = g_mkdir_with_parents(p->pw_dir, 0700);
+g_assert(ret == 0);
+
+return p;
+}
+
+#define g_unix_get_passwd_entry_qemu(username, err) \
+ test_get_passwd_entry(username, err)
+#endif
+
+static struct passwd *
+get_passwd_entry(const char *username, Error **errp)
+{
+g_autoptr(GError) err = NULL;
+struct passwd *p;
+
+ERRP_GUARD();
+
+p = g_unix_get_passwd_entry_qemu(username, );
+if (p == NULL) {
+error_setg(errp, "failed to lookup user '%s': %s",
+ username, err->message);
+return NULL;
+}
+
+return p;
+}
+
+static bool
+mkdir_for_user(const char *path, const struct passwd *p,
+ mode_t mode, Error **errp)
+{
+ERRP_GUARD();
+
+if (g_mkdir(path, mode) == -1) {
+error_setg(errp, "failed to create directory '%s': %s",
+ path, g_strerror(errno));
+return false;
+}
+
+if (chown(path, p->pw_uid, p->pw_gid) == -1) {
+error_setg(errp, "failed to set ownership of directory '%s': %s",
+ path, g_strerror(errno));
+return false;
+}
+
+if (chmod(path, mode) == -1) {
+error_setg(errp, "failed to set permissions of directory '%s': %s",
+ path, g_strerror(errno));
+return false;
+}
+
+return true;
+}
+
+static bool
+check_openssh_pub_key(const char *key, Error **errp)
+{
+ERRP_GUARD();
+
+/* simple sanity-check, we may want more? */
+if (!key || key[0] == '#' || strchr(key, '\n')) {
+error_setg(errp, "invalid OpenSSH public key: '%s'", key);
+return false;
+}
+
+return true;
+}
+
+static bool
+check_openssh_pub_keys(strList *keys, size_t *nkeys, Error **errp)
+{
+size_t n = 0;
+strList *k;
+
+ERRP_GUARD();
+
+for (k = keys; k != NULL; k = k->next) {
+if (!check_openssh_pub_key(k->value, errp)) {
+return false;
+}
+n++;
+}
+
+if (nkeys) {
+*nkeys = n;
+}
+return true;
+}
+
+static bool
+write_authkeys(const char *path, const GStrv keys,
+ const struct passwd *p, Error **errp)
+{
+g_autofree char *contents = NULL;
+g_autoptr(GError) err = NULL;
+
+ERRP_GUARD();
+
+contents = g_strjoinv("\n", keys);
+if (!g_file_set_contents(path, contents, -1, )) {
+error_setg(errp, "failed to write to '%s': %s", path, err->message);
+return false;
+}
+
+if (chown(path, p->pw_uid, p->pw_gid) == -1) {
+error_setg(errp, "failed to set ownership of directory '%s': %s",
+ path, g_strerror(errno));
+return false;
+}
+
+if (chmod(path, 0600) == -1) {
+error_setg(errp, "failed to set permissions of '%s': %s",
+ path, g_strerror(errno));
+return
[PULL v3 12/12] qga: add ssh-get-authorized-keys
From: Marc-André Lureau
Signed-off-by: Marc-André Lureau
*fix-up merge conflicts due to qga-ssh-test being disabled in earlier
patch due to G_TEST_OPTION_ISOLATE_DIRS triggering build-oss-fuzz
leak detector.
*fix up style and disallowed g_assert* usage reported by checkpatch
Signed-off-by: Michael Roth
---
qga/commands-posix-ssh.c | 66
qga/meson.build | 11 +--
qga/qapi-schema.json | 31 +++
3 files changed, 106 insertions(+), 2 deletions(-)
diff --git a/qga/commands-posix-ssh.c b/qga/commands-posix-ssh.c
index 362c9e8816..749167e82d 100644
--- a/qga/commands-posix-ssh.c
+++ b/qga/commands-posix-ssh.c
@@ -268,6 +268,46 @@ qmp_guest_ssh_remove_authorized_keys(const char *username,
strList *keys,
write_authkeys(authkeys_path, new_keys, p, errp);
}
+GuestAuthorizedKeys *
+qmp_guest_ssh_get_authorized_keys(const char *username, Error **errp)
+{
+g_autofree struct passwd *p = NULL;
+g_autofree char *authkeys_path = NULL;
+g_auto(GStrv) authkeys = NULL;
+g_autoptr(GuestAuthorizedKeys) ret = NULL;
+int i;
+
+ERRP_GUARD();
+
+p = get_passwd_entry(username, errp);
+if (p == NULL) {
+return NULL;
+}
+
+authkeys_path = g_build_filename(p->pw_dir, ".ssh",
+ "authorized_keys", NULL);
+authkeys = read_authkeys(authkeys_path, errp);
+if (authkeys == NULL) {
+return NULL;
+}
+
+ret = g_new0(GuestAuthorizedKeys, 1);
+for (i = 0; authkeys[i] != NULL; i++) {
+strList *new;
+
+g_strstrip(authkeys[i]);
+if (!authkeys[i][0] || authkeys[i][0] == '#') {
+continue;
+}
+
+new = g_new0(strList, 1);
+new->value = g_strdup(authkeys[i]);
+new->next = ret->keys;
+ret->keys = new;
+}
+
+return g_steal_pointer();
+}
#ifdef QGA_BUILD_UNIT_TEST
#if GLIB_CHECK_VERSION(2, 60, 0)
@@ -426,6 +466,31 @@ test_remove_keys(void)
"algo some-key another\n");
}
+static void
+test_get_keys(void)
+{
+Error *err = NULL;
+static const char *authkeys =
+"algo key1 comments\n"
+"# a commented line\n"
+"algo some-key another\n";
+g_autoptr(GuestAuthorizedKeys) ret = NULL;
+strList *k;
+size_t len = 0;
+
+test_authorized_keys_set(authkeys);
+
+ret = qmp_guest_ssh_get_authorized_keys(g_get_user_name(), );
+g_assert(err == NULL);
+
+for (len = 0, k = ret->keys; k != NULL; k = k->next) {
+g_assert(g_str_has_prefix(k->value, "algo "));
+len++;
+}
+
+g_assert(len == 2);
+}
+
int main(int argc, char *argv[])
{
setlocale(LC_ALL, "");
@@ -437,6 +502,7 @@ int main(int argc, char *argv[])
g_test_add_func("/qga/ssh/add_keys", test_add_keys);
g_test_add_func("/qga/ssh/add_reset_keys", test_add_reset_keys);
g_test_add_func("/qga/ssh/remove_keys", test_remove_keys);
+g_test_add_func("/qga/ssh/get_keys", test_get_keys);
return g_test_run();
}
diff --git a/qga/meson.build b/qga/meson.build
index 4cb3b3f259..53ba6de5f8 100644
--- a/qga/meson.build
+++ b/qga/meson.build
@@ -95,8 +95,15 @@ test_env.set('G_TEST_BUILDDIR', meson.current_build_dir())
# issue is identified/fix
#if 'CONFIG_POSIX' in config_host
if false
- qga_ssh_test = executable('qga-ssh-test',
-files('commands-posix-ssh.c'),
+ srcs = [files('commands-posix-ssh.c')]
+ i = 0
+ foreach output: qga_qapi_outputs
+if output.startswith('qga-qapi-types') or
output.startswith('qga-qapi-visit')
+ srcs += qga_qapi_files[i]
+endif
+i = i + 1
+ endforeach
+ qga_ssh_test = executable('qga-ssh-test', srcs,
dependencies: [qemuutil],
c_args: ['-DQGA_BUILD_UNIT_TEST'])
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 4ddea898fa..6ca85f995f 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -1347,6 +1347,37 @@
{ 'command': 'guest-get-devices',
'returns': ['GuestDeviceInfo'] }
+##
+# @GuestAuthorizedKeys:
+#
+# @keys: public keys (in OpenSSH/sshd(8) authorized_keys format)
+#
+# Since: 5.2
+##
+{ 'struct': 'GuestAuthorizedKeys',
+ 'data': {
+ 'keys': ['str']
+ },
+ 'if': 'defined(CONFIG_POSIX)' }
+
+
+##
+# @guest-ssh-get-authorized-keys:
+#
+# @username: the user account to add the authorized keys
+#
+# Return the public keys from user .ssh/authorized_keys on Unix systems (not
+# implemented for other systems).
+#
+# Returns: @GuestAuthorizedKeys
+#
+# Since: 5.2
+##
+{ 'command': 'guest-ssh-get-authorized-keys',
+ 'data': { 'username': 'str' },
+ 'returns': 'GuestAuthorizedKeys',
+ 'if': 'defined(CONFIG_POSIX)' }
+
##
# @guest-ssh-add-authorized-keys:
#
--
2.25.1
Re: [PULL v2 00/12] qemu-ga patch queue for soft-freeze
On Mon, Nov 02, 2020 at 07:11:22PM -0600, Michael Roth wrote:
> Hi Peter,
>
> Sorry to get these out so late, for some inexplicable reason my email
> client decided to flag all responses v1 as spam and I didn't notice
> until I double-checked the archives this morning.
>
> I've fixed the gcc-on-BSD and clang-on-linux issues you pointed out
> (PATCH 6) and added proper test coverage for both.
>
> Also, the qga-ssh-test unit test introduced in this series triggers a
> failure in Gitlab CI build-oss-fuzz test. This seems to be due to a
> memory leak in GLib itself when G_TEST_OPTION_ISOLATE_DIRS option is
> passed to g_test_init(). I verified the unit test doesn't introduce any
> leaks when run without g_test* harness. Since G_TEST_OPTION_ISOLATE_DIRS
> is currently needed to safely run the qga-ssh-test, I've disabled it for
> now (PATCH 9 and 12) until we figure out solution.
>
> Thanks,
>
> Mike
...And I just noticed Markus email noting that checkpatch complaints about
g_assert_* aren't just noise. Re-spinning a v3 to address.
>
> The following changes since commit 2c6605389c1f76973d92b69b85d40d94b8f1092c:
>
> Merge remote-tracking branch 'remotes/awilliam/tags/vfio-update-20201101.0'
> into staging (2020-11-02 09:54:00 +)
>
> are available in the Git repository at:
>
> git://github.com/mdroth/qemu.git tags/qga-pull-2020-10-27-v2-tag
>
> for you to fetch changes up to b457991dfb801bf9227e8823534de5dbb3c276c1:
>
> qga: add ssh-get-authorized-keys (2020-11-02 18:30:42 -0600)
>
>
> qemu-ga patch queue for soft-freeze
>
> * add guest-get-disks for w32/linux
> * add guest-{add,remove,get}-authorized-keys
> * fix API violations and schema documentation inconsistencies with
> recently-added guest-get-devices
>
> v2:
> - fix BSD build error due to missing stub for guest_get_disks
> - fix clang build error on linux due to unused variable
> - disable qga-ssh-test for now due to a memory leak within GLib when
> G_TEST_OPTION_ISOLATE_DIRS is passed to g_test_init() since it
> break Gitlab CI build-oss-fuzz test
> - rebased and re-tested on master
>
>
> Marc-André Lureau (5):
> glib-compat: add g_unix_get_passwd_entry_qemu()
> qga: add ssh-{add,remove}-authorized-keys
> qga: add *reset argument to ssh-add-authorized-keys
> meson: minor simplification
> qga: add ssh-get-authorized-keys
>
> Markus Armbruster (4):
> qga: Rename guest-get-devices return member 'address' to 'id'
> qga: Use common time encoding for guest-get-devices 'driver-date'
> qga-win: Fix guest-get-devices error API violations
> qga: Flatten simple union GuestDeviceId
>
> Tomáš Golembiovský (3):
> qga: add command guest-get-disks
> qga: add implementation of guest-get-disks for Linux
> qga: add implementation of guest-get-disks for Windows
>
> include/glib-compat.h| 26 +++
> qga/commands-posix-ssh.c | 516
> +++
> qga/commands-posix.c | 297 ++-
> qga/commands-win32.c | 140 +++--
> qga/meson.build | 39 +++-
> qga/qapi-schema.json | 127 +++-
> 6 files changed, 1104 insertions(+), 41 deletions(-)
> create mode 100644 qga/commands-posix-ssh.c
>
>
[Bug 1902612] [NEW] assert issue locates in xhci_kick_epctx() in hw/usb/hcd-xhci.c
Public bug reported: Hello, I found an assertion failure through hw/usb/hcd-xhci.c. This was found in latest version 5.1.0. An assertion-failure flaw was found in xhci_kick_epctx() in hw/usb/hcd- xhci.c . XHCI slot's endpoint context is enabled in xhci_configure_slot(), whose ep_ctx structure is controlled by user. With uninitialized endPoint context could trigger assert(ring->dequeue != 0).The guest system could use this flaw to crash the qemu resulting in denial of service. To reproduce the assertion failure, please run the QEMU with following command line. $ qemu-system-x86_64 -enable-kvm -boot c -m 2G -drive format=qcow2,file=./ubuntu.img -nic user,model=rtl8139,hostfwd=tcp:0.0.0.0:-:22 -device nec-usb- xhci,id=xhci -device usb-tablet,bus=xhci.0,port=1,id=usbdev1 The poc is attached. Backtrace is as follows: #0 0x7f6dfd4c4f47 in __GI_raise (sig=sig@entry=0x6) at ../sysdeps/unix/sysv/linux/raise.c:51 #1 0x7f6dfd4c68b1 in __GI_abort () at abort.c:79 #2 0x7f6dfd4b642a in __assert_fail_base (fmt=0x7f6dfd63da38 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", assertion=assertion@entry=0x55e9b9d38a64 "ring->dequeue != 0", file=file@entry=0x55e9b9d388c0 "hw/usb/hcd-xhci.c", line=line@entry=0x7a3, function=function@entry=0x55e9b9d3a5c0 <__PRETTY_FUNCTION__.29754> "xhci_kick_epctx") at assert.c:92 #3 0x7f6dfd4b64a2 in __GI___assert_fail (assertion=assertion@entry=0x55e9b9d38a64 "ring->dequeue != 0", file=file@entry=0x55e9b9d388c0 "hw/usb/hcd-xhci.c", line=line@entry=0x7a3, function=function@entry=0x55e9b9d3a5c0 <__PRETTY_FUNCTION__.29754> "xhci_kick_epctx") at assert.c:101 #4 0x55e9b9a3292f in xhci_kick_epctx (epctx=0x7f6da836b510, streamid=streamid@entry=0x0) at hw/usb/hcd-xhci.c:1955 #5 0x55e9b9a3c64b in xhci_kick_ep (streamid=0x0, epid=0x1, slotid=0x11, xhci=0x7f6df8b38010) at hw/usb/hcd-xhci.c:1861 #6 0x55e9b9a3c64b in xhci_doorbell_write (ptr=0x7f6df8b38010, reg=0x11, val=0x1, size=) at hw/usb/hcd-xhci.c:3162 #7 0x55e9b977d274 in memory_region_write_accessor (mr=0x7f6df8b38d80, addr=0x44, value=, size=0x1, shift=, mask=, attrs=...) at /home/zjusvn/qemu5-hypervisor/qemu-5.0.0/memory.c:483 #8 0x55e9b977ad86 in access_with_adjusted_size (addr=addr@entry=0x44, value=value@entry=0x7f6dfb915f88, size=size@entry=0x1, access_size_min=, access_size_max=, access_fn=0x55e9b977d1f0 , mr=0x7f6df8b38d80, attrs=...) at /home/zjusvn/qemu5-hypervisor/qemu-5.0.0/memory.c:544 #9 0x55e9b977f4c8 in memory_region_dispatch_write (mr=mr@entry=0x7f6df8b38d80, addr=0x44, data=, op=, attrs=attrs@entry=...) at /home/zjusvn/qemu5-hypervisor/qemu-5.0.0/memory.c:1483 #10 0x55e9b972c691 in flatview_write_continue (fv=fv@entry=0x7f6da951f750, addr=addr@entry=0xfebf2044, attrs=..., ptr=ptr@entry=0x7f6dfb9160e0, len=len@entry=0x1, addr1=, l=, mr=0x7f6df8b38d80) at /home/zjusvn/qemu5-hypervisor/qemu-5.0.0/exec.c:3137 #11 0x55e9b972c826 in flatview_write (fv=0x7f6da951f750, addr=0xfebf2044, attrs=..., buf=buf@entry=0x7f6dfb9160e0, len=0x1) at /home/zjusvn/qemu5-hypervisor/qemu-5.0.0/exec.c:3177 #12 0x55e9b972c89a in subpage_write (opaque=, addr=, value=, len=, attrs=...) at /home/zjusvn/qemu5-hypervisor/qemu-5.0.0/exec.c:2789 #13 0x55e9b977b269 in memory_region_write_with_attrs_accessor (mr=0x7f6da9534650, addr=0x44, value=, size=0x1, shift=, mask=, attrs=...) at /home/zjusvn/qemu5-hypervisor/qemu-5.0.0/memory.c:503 #14 0x55e9b977ad86 in access_with_adjusted_size (addr=addr@entry=0x44, value=value@entry=0x7f6dfb9161f8, size=size@entry=0x1, access_size_min=, access_size_max=, access_fn=0x55e9b977b1e0 , mr=0x7f6da9534650, attrs=...) at /home/zjusvn/qemu5-hypervisor/qemu-5.0.0/memory.c:544 #15 0x55e9b977f4c8 in memory_region_dispatch_write (mr=0x7f6da9534650, addr=addr@entry=0x44, data=, data@entry=0x1, op=op@entry=MO_8, attrs=...) at /home/zjusvn/qemu5-hypervisor/qemu-5.0.0/memory.c:1483 #16 0x55e9b979021f in io_writex (env=env@entry=0x55e9baed5b50, iotlbentry=iotlbentry@entry=0x7f6da8b8bc10, mmu_idx=mmu_idx@entry=0x1, val=val@entry=0x1, addr=addr@entry=0x7fbba0601044, retaddr=retaddr@entry=0x7f6db9d90d48, op=MO_8) at /home/zjusvn/qemu5-hypervisor/qemu-5.0.0/accel/tcg/cputlb.c:1084 #17 0x55e9b9794c42 in store_helper (op=MO_8, retaddr=0x7f6db9d90d48, oi=, val=, addr=0x7fbba0601044, env=0x55e9baed5b50) at /home/zjusvn/qemu5-hypervisor/qemu-5.0.0/accel/tcg/cputlb.c:1954 #18 0x55e9b9794c42 in helper_ret_stb_mmu (env=0x55e9baed5b50, addr=0x7fbba0601044, val=0x1, oi=, retaddr=0x7f6db9d90d48) at /home/zjusvn/qemu5-hypervisor/qemu-5.0.0/accel/tcg/cputlb.c:2056 #19 0x7f6db9d90d48 in code_gen_buffer () #20 0x55e9b97a5217 in cpu_tb_exec (itb=, cpu=0x7f6db9d240c0 ) at /home/zjusvn/qemu5-hypervisor/qemu-5.0.0/accel/tcg/cpu-exec.c:172 #21 0x55e9b97a5217 in cpu_loop_exec_tb (tb_exit=, last_tb=, tb=, cpu=0x7f6db9d240c0 ) at /home/zjusvn/qemu5-hypervisor/qemu-5.0.0/accel/tcg/cpu-exec.c:619 #22
RE: [PATCH 5/6] plugins/loader: fix uninitialized variable warning in plugin_reset_uninstall()
> -Original Message-
> From: Philippe Mathieu-Daudé [mailto:phi...@redhat.com]
> Sent: Tuesday, November 3, 2020 10:18 AM
> To: Chenqun (kuhn) ; qemu-devel@nongnu.org;
> qemu-triv...@nongnu.org
> Cc: Alex Bennée ; Zhanghailiang
> ; ganqixin ; Euler
> Robot
> Subject: Re: [PATCH 5/6] plugins/loader: fix uninitialized variable warning in
> plugin_reset_uninstall()
>
> On 11/3/20 2:52 AM, Chen Qun wrote:
> > After the WITH_QEMU_LOCK_GUARD macro is added, the compiler cannot
> > identify that the statements in the macro must be executed. As a
> > result, some variables assignment statements in the macro may be
> considered as unexecuted by the compiler.
> >
> > The compiler showed warning:
> > plugins/loader.c: In function ‘plugin_reset_uninstall’:
> > plugins/loader.c:382:15: warning: ‘ctx’ may be used uninitialized in
> > this function [-Wmaybe-uninitialized]
>
> This shouldn't happen as the function returns before (else there is a NULL
> deref).
>
Yes, in fact, it shouldn't have happened when the program was running.
But after added 'WITH_QEMU_LOCK_GUARD', let the compiler think it might happen.
So, we add a default value, make the compiler happy.
Thanks,
Chen Qun
> > 382 | data->ctx = ctx;
> > | ~~^
> >
> > Add a default value for 'expire_time' to prevented the warning.
> >
> > Reported-by: Euler Robot
> > Signed-off-by: Chen Qun
> > ---
> > Cc: "Alex Bennée"
> > ---
> > plugins/loader.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/plugins/loader.c b/plugins/loader.c index
> > 8ac5dbc20f..88593fe138 100644
> > --- a/plugins/loader.c
> > +++ b/plugins/loader.c
> > @@ -367,7 +367,7 @@ void plugin_reset_uninstall(qemu_plugin_id_t id,
> > bool reset) {
> > struct qemu_plugin_reset_data *data;
> > -struct qemu_plugin_ctx *ctx;
> > +struct qemu_plugin_ctx *ctx = NULL;
> >
> > WITH_QEMU_LOCK_GUARD() {
> > ctx = plugin_id_to_ctx_locked(id);
> >
Re: [RFC PATCH v2 09/13] hw/arm/virt-acpi-build: add PPTT table
On 10/30/2020 12:56 AM, Andrew Jones wrote:
On Tue, Oct 20, 2020 at 09:14:36PM +0800, Ying Fang wrote:
Add the Processor Properties Topology Table (PPTT) to present CPU topology
information to the guest.
Signed-off-by: Andrew Jones
I don't know why I have an s-o-b here. I guess it's because this code
looks nearly identical to what I wrote, except for using the new and,
IMO, unnecessary build_socket_hierarchy and build_smt_hierarchy functions.
IMHO, you should drop the last patch and just take
https://github.com/rhdrjones/qemu/commit/439b38d67ca1f2cbfa5b9892a822b651ebd05c11
as it is, unless it needs to be fixed somehow
Thanks,
drew
This patch is based on your branch however it is slightly modified.
As described in:
[RFC,v2,08/13] hw/acpi/aml-build: add processor hierarchy node structure
The wrapper function build_socket_hierarchy and build_smt_hierarchy are
introduced to make later patch much more readable and make preparations
for cache hierarchy.
Hope it won't make you confused. I will drop your branch patch and
give details in the commit message in the next post.
Thanks,
Ying
Signed-off-by: Ying Fang
---
hw/arm/virt-acpi-build.c | 42
1 file changed, 42 insertions(+)
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index fae5a26741..e1f3ea50ad 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -429,6 +429,42 @@ build_srat(GArray *table_data, BIOSLinker *linker,
VirtMachineState *vms)
"SRAT", table_data->len - srat_start, 3, NULL, NULL);
}
+static void build_pptt(GArray *table_data, BIOSLinker *linker, MachineState *ms)
+{
+int pptt_start = table_data->len;
+int uid = 0, cpus = 0, socket;
+unsigned int smp_cores = ms->smp.cores;
+unsigned int smp_threads = ms->smp.threads;
+
+acpi_data_push(table_data, sizeof(AcpiTableHeader));
+
+for (socket = 0; cpus < ms->possible_cpus->len; socket++) {
+uint32_t socket_offset = table_data->len - pptt_start;
+int core;
+
+build_socket_hierarchy(table_data, 0, socket);
+
+for (core = 0; core < smp_cores; core++) {
+uint32_t core_offset = table_data->len - pptt_start;
+int thread;
+
+if (smp_threads <= 1) {
+build_processor_hierarchy(table_data, 2, socket_offset, uid++);
+ } else {
+build_processor_hierarchy(table_data, 0, socket_offset, core);
+for (thread = 0; thread < smp_threads; thread++) {
+build_smt_hierarchy(table_data, core_offset, uid++);
+}
+ }
+}
+cpus += smp_cores * smp_threads;
+}
+
+build_header(linker, table_data,
+ (void *)(table_data->data + pptt_start), "PPTT",
+ table_data->len - pptt_start, 2, NULL, NULL);
+}
+
/* GTDT */
static void
build_gtdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
@@ -669,6 +705,7 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables
*tables)
unsigned dsdt, xsdt;
GArray *tables_blob = tables->table_data;
MachineState *ms = MACHINE(vms);
+bool cpu_topology_enabled = !vmc->ignore_cpu_topology;
table_offsets = g_array_new(false, true /* clear */,
sizeof(uint32_t));
@@ -688,6 +725,11 @@ void virt_acpi_build(VirtMachineState *vms,
AcpiBuildTables *tables)
acpi_add_table(table_offsets, tables_blob);
build_madt(tables_blob, tables->linker, vms);
+if (cpu_topology_enabled) {
+acpi_add_table(table_offsets, tables_blob);
+build_pptt(tables_blob, tables->linker, ms);
+}
+
acpi_add_table(table_offsets, tables_blob);
build_gtdt(tables_blob, tables->linker, vms);
--
2.23.0
.
Re: [PATCH 6/6] migration: fix uninitialized variable warning in migrate_send_rp_req_pages()
On 11/3/20 2:52 AM, Chen Qun wrote:
> After the WITH_QEMU_LOCK_GUARD macro is added, the compiler cannot identify
> that the statements in the macro must be executed. As a result, some
> variables
> assignment statements in the macro may be considered as unexecuted by the
> compiler.
>
> The compiler showed warning:
> migration/migration.c: In function ‘migrate_send_rp_req_pages’:
> migration/migration.c:384:8: warning: ‘received’ may be used uninitialized in
> this function [-Wmaybe-uninitialized]
> 384 | if (received) {
> |^
>
> Add a default value for 'received' to prevented the warning.
>
> Reported-by: Euler Robot
> Signed-off-by: Chen Qun
> ---
> Cc: Juan Quintela
> Cc: "Dr. David Alan Gilbert"
> ---
> migration/migration.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index 9bb4fee5ac..de90486a61 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -361,7 +361,7 @@ int migrate_send_rp_req_pages(MigrationIncomingState *mis,
>RAMBlock *rb, ram_addr_t start, uint64_t haddr)
> {
> void *aligned = (void *)(uintptr_t)(haddr & (-qemu_ram_pagesize(rb)));
> -bool received;
> +bool received = false;
>
> WITH_QEMU_LOCK_GUARD(>page_request_mutex) {
> received = ramblock_recv_bitmap_test_byte_offset(rb, start);
>
Since this helps static analyzer:
Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 1/6] target/xtensa: fix uninitialized variable warning
On 11/3/20 2:52 AM, Chen Qun wrote:
> The compiler cannot determine whether the return values of the
> xtensa_operand_is_register(isa, opc, opnd)
> and xtensa_operand_is_visible(isa, opc, opnd) functions are the same.
> So,it assumes that 'rf' is not assigned, but it's used.
>
> The compiler showed warning:
> target/xtensa/translate.c: In function ‘disas_xtensa_insn’:
> target/xtensa/translate.c:985:43: warning: ‘rf’ may be used uninitialized in
> this function [-Wmaybe-uninitialized]
> 985 | arg[vopnd].num_bits =
> xtensa_regfile_num_bits(isa, rf);
> |
> ^~~~
>
> Add a default value for 'rf' to prevented the warning.
>
> Reported-by: Euler Robot
> Signed-off-by: Chen Qun
> ---
> Cc: Max Filippov
> ---
> target/xtensa/translate.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/xtensa/translate.c b/target/xtensa/translate.c
> index 944a157747..eea851bbe7 100644
> --- a/target/xtensa/translate.c
> +++ b/target/xtensa/translate.c
> @@ -953,7 +953,7 @@ static void disas_xtensa_insn(CPUXtensaState *env,
> DisasContext *dc)
>
> for (opnd = vopnd = 0; opnd < opnds; ++opnd) {
> void **register_file = NULL;
> -xtensa_regfile rf;
> +xtensa_regfile rf = -1;
NAck (code smells).
Deferring to Max, but possible fix:
-- >8 --
@@ -953,10 +953,9 @@ static void disas_xtensa_insn(CPUXtensaState *env,
DisasContext *dc)
for (opnd = vopnd = 0; opnd < opnds; ++opnd) {
void **register_file = NULL;
-xtensa_regfile rf;
+xtensa_regfile rf = xtensa_operand_regfile(isa, opc, opnd);
if (xtensa_operand_is_register(isa, opc, opnd)) {
-rf = xtensa_operand_regfile(isa, opc, opnd);
register_file = dc->config->regfile[rf];
if (rf == dc->config->a_regfile) {
---
Re: [PATCH 5/6] plugins/loader: fix uninitialized variable warning in plugin_reset_uninstall()
On 11/3/20 2:52 AM, Chen Qun wrote:
> After the WITH_QEMU_LOCK_GUARD macro is added, the compiler cannot identify
> that the statements in the macro must be executed. As a result, some
> variables
> assignment statements in the macro may be considered as unexecuted by the
> compiler.
>
> The compiler showed warning:
> plugins/loader.c: In function ‘plugin_reset_uninstall’:
> plugins/loader.c:382:15: warning: ‘ctx’ may be used uninitialized in this
> function [-Wmaybe-uninitialized]
This shouldn't happen as the function returns before
(else there is a NULL deref).
> 382 | data->ctx = ctx;
> | ~~^
>
> Add a default value for 'expire_time' to prevented the warning.
>
> Reported-by: Euler Robot
> Signed-off-by: Chen Qun
> ---
> Cc: "Alex Bennée"
> ---
> plugins/loader.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/plugins/loader.c b/plugins/loader.c
> index 8ac5dbc20f..88593fe138 100644
> --- a/plugins/loader.c
> +++ b/plugins/loader.c
> @@ -367,7 +367,7 @@ void plugin_reset_uninstall(qemu_plugin_id_t id,
> bool reset)
> {
> struct qemu_plugin_reset_data *data;
> -struct qemu_plugin_ctx *ctx;
> +struct qemu_plugin_ctx *ctx = NULL;
>
> WITH_QEMU_LOCK_GUARD() {
> ctx = plugin_id_to_ctx_locked(id);
>
Re: [RFC PATCH v2 08/13] hw/acpi/aml-build: add processor hierarchy node structure
On 10/30/2020 1:24 AM, Andrew Jones wrote:
On Tue, Oct 20, 2020 at 09:14:35PM +0800, Ying Fang wrote:
Add the processor hierarchy node structures to build ACPI information
for CPU topology. Three helpers are introduced:
(1) build_socket_hierarchy for socket description structure
(2) build_processor_hierarchy for processor description structure
(3) build_smt_hierarchy for thread (logic processor) description structure
I see now the reason to introduce three functions is because the last
patch adds different private resources. You should point that plan out
in this commit message.
Yes, the private resources are used to describe cache hierarchy
and it is variable among different topology level. I will point it
out in the commit message to avoid any confusion.
Thanks,
Ying
Thanks,
drew
Signed-off-by: Ying Fang
Signed-off-by: Henglong Fan
---
hw/acpi/aml-build.c | 37 +
include/hw/acpi/aml-build.h | 7 +++
2 files changed, 44 insertions(+)
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 3792ba96ce..da3b41b514 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -1770,6 +1770,43 @@ void build_slit(GArray *table_data, BIOSLinker *linker,
MachineState *ms)
table_data->len - slit_start, 1, NULL, NULL);
}
+/*
+ * ACPI 6.3: 5.2.29.1 Processor hierarchy node structure (Type 0)
+ */
+void build_socket_hierarchy(GArray *tbl, uint32_t parent, uint32_t id)
+{
+build_append_byte(tbl, 0); /* Type 0 - processor */
+build_append_byte(tbl, 20); /* Length, no private resources */
+build_append_int_noprefix(tbl, 0, 2); /* Reserved */
+build_append_int_noprefix(tbl, 1, 4); /* Flags: Physical package */
+build_append_int_noprefix(tbl, parent, 4); /* Parent */
+build_append_int_noprefix(tbl, id, 4); /* ACPI processor ID */
+build_append_int_noprefix(tbl, 0, 4); /* Number of private resources */
+}
+
+void build_processor_hierarchy(GArray *tbl, uint32_t flags,
+ uint32_t parent, uint32_t id)
+{
+build_append_byte(tbl, 0); /* Type 0 - processor */
+build_append_byte(tbl, 20); /* Length, no private resources */
+build_append_int_noprefix(tbl, 0, 2); /* Reserved */
+build_append_int_noprefix(tbl, flags, 4); /* Flags */
+build_append_int_noprefix(tbl, parent, 4); /* Parent */
+build_append_int_noprefix(tbl, id, 4); /* ACPI processor ID */
+build_append_int_noprefix(tbl, 0, 4); /* Number of private resources */
+}
+
+void build_smt_hierarchy(GArray *tbl, uint32_t parent, uint32_t id)
+{
+build_append_byte(tbl, 0);/* Type 0 - processor */
+build_append_byte(tbl, 20); /* Length, add private resources */
+build_append_int_noprefix(tbl, 0, 2); /* Reserved */
+build_append_int_noprefix(tbl, 0x0e, 4);/* Processor is a thread */
+build_append_int_noprefix(tbl, parent , 4); /* parent */
+build_append_int_noprefix(tbl, id, 4); /* ACPI processor ID */
+build_append_int_noprefix(tbl, 0, 4); /* Num of private resources */
+}
+
/* build rev1/rev3/rev5.1 FADT */
void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f,
const char *oem_id, const char *oem_table_id)
diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index fe0055fffb..56474835a7 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -437,6 +437,13 @@ void build_srat_memory(AcpiSratMemoryAffinity *numamem,
uint64_t base,
void build_slit(GArray *table_data, BIOSLinker *linker, MachineState *ms);
+void build_socket_hierarchy(GArray *tbl, uint32_t parent, uint32_t id);
+
+void build_processor_hierarchy(GArray *tbl, uint32_t flags,
+ uint32_t parent, uint32_t id);
+
+void build_smt_hierarchy(GArray *tbl, uint32_t parent, uint32_t id);
+
void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f,
const char *oem_id, const char *oem_table_id);
--
2.23.0
.
[PATCH-for-5.2 v3 7/7] util/vfio-helpers: Assert offset is aligned to page size
mmap(2) states:
'offset' must be a multiple of the page size as returned
by sysconf(_SC_PAGE_SIZE).
Add an assertion to be sure we don't break this contract.
Signed-off-by: Philippe Mathieu-Daudé
---
util/vfio-helpers.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
index 73f7bfa7540..804768d5c66 100644
--- a/util/vfio-helpers.c
+++ b/util/vfio-helpers.c
@@ -162,6 +162,7 @@ void *qemu_vfio_pci_map_bar(QEMUVFIOState *s, int index,
Error **errp)
{
void *p;
+assert(QEMU_IS_ALIGNED(offset, qemu_real_host_page_size));
assert_bar_index_valid(s, index);
p = mmap(NULL, MIN(size, s->bar_region_info[index].size - offset),
prot, MAP_SHARED,
--
2.26.2
Re: [PATCH 3/6] util/qemu-timer: fix uninitialized variable warning in timer_mod_anticipate_ns()
On 11/3/20 2:52 AM, Chen Qun wrote:
> After the WITH_QEMU_LOCK_GUARD macro is added, the compiler cannot identify
> that the statements in the macro must be executed. As a result, some
> variables
> assignment statements in the macro may be considered as unexecuted by the
> compiler.
>
> The compiler showed warning:
> util/qemu-timer.c: In function ‘timer_mod_anticipate_ns’:
> util/qemu-timer.c:474:8: warning: ‘rearm’ may be used uninitialized in this
> function [-Wmaybe-uninitialized]
> 474 | if (rearm) {
> |^
>
> Change the default value assignment place to prevented the warning.
>
> Reported-by: Euler Robot
> Signed-off-by: Chen Qun
> ---
> Cc: Paolo Bonzini
> ---
> util/qemu-timer.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé
[PATCH-for-5.2 v3 4/7] util/vfio-helpers: Trace where BARs are mapped
For debugging purpose, trace where a BAR is mapped.
Reviewed-by: Fam Zheng
Reviewed-by: Stefan Hajnoczi
Signed-off-by: Philippe Mathieu-Daudé
---
util/vfio-helpers.c | 2 ++
util/trace-events | 1 +
2 files changed, 3 insertions(+)
diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
index cd6287c3a98..278c54902e7 100644
--- a/util/vfio-helpers.c
+++ b/util/vfio-helpers.c
@@ -166,6 +166,8 @@ void *qemu_vfio_pci_map_bar(QEMUVFIOState *s, int index,
p = mmap(NULL, MIN(size, s->bar_region_info[index].size - offset),
prot, MAP_SHARED,
s->device, s->bar_region_info[index].offset + offset);
+trace_qemu_vfio_pci_map_bar(index, s->bar_region_info[index].offset ,
+size, offset, p);
if (p == MAP_FAILED) {
error_setg_errno(errp, errno, "Failed to map BAR region");
p = NULL;
diff --git a/util/trace-events b/util/trace-events
index 0753e4a0ed1..52d43cda3f3 100644
--- a/util/trace-events
+++ b/util/trace-events
@@ -88,3 +88,4 @@ qemu_vfio_dma_unmap(void *s, void *host) "s %p host %p"
qemu_vfio_pci_read_config(void *buf, int ofs, int size, uint64_t region_ofs,
uint64_t region_size) "read cfg ptr %p ofs 0x%x size 0x%x (region addr
0x%"PRIx64" size 0x%"PRIx64")"
qemu_vfio_pci_write_config(void *buf, int ofs, int size, uint64_t region_ofs,
uint64_t region_size) "write cfg ptr %p ofs 0x%x size 0x%x (region addr
0x%"PRIx64" size 0x%"PRIx64")"
qemu_vfio_region_info(const char *desc, uint64_t region_ofs, uint64_t
region_size, uint32_t cap_offset) "region '%s' addr 0x%"PRIx64" size
0x%"PRIx64" cap_ofs 0x%"PRIx32
+qemu_vfio_pci_map_bar(int index, uint64_t region_ofs, uint64_t region_size,
int ofs, void *host) "map region bar#%d addr 0x%"PRIx64" size 0x%"PRIx64" ofs
0x%x host %p"
--
2.26.2
[PATCH-for-5.2 v3 3/7] util/vfio-helpers: Trace PCI BAR region info
For debug purpose, trace BAR regions info.
Reviewed-by: Fam Zheng
Reviewed-by: Stefan Hajnoczi
Signed-off-by: Philippe Mathieu-Daudé
---
util/vfio-helpers.c | 8
util/trace-events | 1 +
2 files changed, 9 insertions(+)
diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
index 1d4efafcaa4..cd6287c3a98 100644
--- a/util/vfio-helpers.c
+++ b/util/vfio-helpers.c
@@ -136,6 +136,7 @@ static inline void assert_bar_index_valid(QEMUVFIOState *s,
int index)
static int qemu_vfio_pci_init_bar(QEMUVFIOState *s, int index, Error **errp)
{
+g_autofree char *barname = NULL;
assert_bar_index_valid(s, index);
s->bar_region_info[index] = (struct vfio_region_info) {
.index = VFIO_PCI_BAR0_REGION_INDEX + index,
@@ -145,6 +146,10 @@ static int qemu_vfio_pci_init_bar(QEMUVFIOState *s, int
index, Error **errp)
error_setg_errno(errp, errno, "Failed to get BAR region info");
return -errno;
}
+barname = g_strdup_printf("bar[%d]", index);
+trace_qemu_vfio_region_info(barname, s->bar_region_info[index].offset,
+s->bar_region_info[index].size,
+s->bar_region_info[index].cap_offset);
return 0;
}
@@ -416,6 +421,9 @@ static int qemu_vfio_init_pci(QEMUVFIOState *s, const char
*device,
ret = -errno;
goto fail;
}
+trace_qemu_vfio_region_info("config", s->config_region_info.offset,
+s->config_region_info.size,
+s->config_region_info.cap_offset);
for (i = 0; i < ARRAY_SIZE(s->bar_region_info); i++) {
ret = qemu_vfio_pci_init_bar(s, i, errp);
diff --git a/util/trace-events b/util/trace-events
index 8d3615e717b..0753e4a0ed1 100644
--- a/util/trace-events
+++ b/util/trace-events
@@ -87,3 +87,4 @@ qemu_vfio_dma_map(void *s, void *host, size_t size, bool
temporary, uint64_t *io
qemu_vfio_dma_unmap(void *s, void *host) "s %p host %p"
qemu_vfio_pci_read_config(void *buf, int ofs, int size, uint64_t region_ofs,
uint64_t region_size) "read cfg ptr %p ofs 0x%x size 0x%x (region addr
0x%"PRIx64" size 0x%"PRIx64")"
qemu_vfio_pci_write_config(void *buf, int ofs, int size, uint64_t region_ofs,
uint64_t region_size) "write cfg ptr %p ofs 0x%x size 0x%x (region addr
0x%"PRIx64" size 0x%"PRIx64")"
+qemu_vfio_region_info(const char *desc, uint64_t region_ofs, uint64_t
region_size, uint32_t cap_offset) "region '%s' addr 0x%"PRIx64" size
0x%"PRIx64" cap_ofs 0x%"PRIx32
--
2.26.2
Re: [PATCH-for-5.2? 3/5] tests/acceptance: Skip incomplete virtio_version tests using '@skip'
On 11/2/20 3:42 PM, Philippe Mathieu-Daudé wrote:
> Prefer skipping incomplete tests with the "@skip" keyword,
> rather than commenting the code.
>
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> tests/acceptance/virtio_version.py | 11 +++
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/tests/acceptance/virtio_version.py
> b/tests/acceptance/virtio_version.py
> index 33593c29dd0..187bbfa1f42 100644
> --- a/tests/acceptance/virtio_version.py
> +++ b/tests/acceptance/virtio_version.py
And I forgot:
-- >8 --
@@ -14,6 +14,7 @@
sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..',
'python'))
from qemu.machine import QEMUMachine
from avocado_qemu import Test
+from avocado import skip
# Virtio Device IDs:
VIRTIO_NET = 1
---
> @@ -140,17 +140,20 @@ def check_all_variants(self, qemu_devtype,
> virtio_devid):
> self.assertIn('conventional-pci-device', trans_ifaces)
> self.assertNotIn('pci-express-device', trans_ifaces)
>
> +@skip("virtio-blk requires 'driver' parameter")
> +def test_conventional_devs_driver(self):
> +self.check_all_variants('virtio-blk-pci', VIRTIO_BLOCK)
> +
> +@skip("virtio-9p requires 'fsdev' parameter")
> +def test_conventional_devs_fsdev(self):
> +self.check_all_variants('virtio-9p-pci', VIRTIO_9P)
>
> def test_conventional_devs(self):
> self.check_all_variants('virtio-net-pci', VIRTIO_NET)
> -# virtio-blk requires 'driver' parameter
> -#self.check_all_variants('virtio-blk-pci', VIRTIO_BLOCK)
> self.check_all_variants('virtio-serial-pci', VIRTIO_CONSOLE)
> self.check_all_variants('virtio-rng-pci', VIRTIO_RNG)
> self.check_all_variants('virtio-balloon-pci', VIRTIO_BALLOON)
> self.check_all_variants('virtio-scsi-pci', VIRTIO_SCSI)
> -# virtio-9p requires 'fsdev' parameter
> -#self.check_all_variants('virtio-9p-pci', VIRTIO_9P)
>
> def check_modern_only(self, qemu_devtype, virtio_devid):
> """Check if a modern-only virtio device type behaves as expected"""
>
[PATCH-for-5.2 v3 2/7] util/vfio-helpers: Trace PCI I/O config accesses
We sometime get kernel panic with some devices on Aarch64
hosts. Alex Williamson suggests it might be broken PCIe
root complex. Add trace event to record the latest I/O
access before crashing. In case, assert our accesses are
aligned.
Reviewed-by: Fam Zheng
Reviewed-by: Stefan Hajnoczi
Signed-off-by: Philippe Mathieu-Daudé
---
util/vfio-helpers.c | 8
util/trace-events | 2 ++
2 files changed, 10 insertions(+)
diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
index 14a549510fe..1d4efafcaa4 100644
--- a/util/vfio-helpers.c
+++ b/util/vfio-helpers.c
@@ -227,6 +227,10 @@ static int qemu_vfio_pci_read_config(QEMUVFIOState *s,
void *buf,
{
int ret;
+trace_qemu_vfio_pci_read_config(buf, ofs, size,
+s->config_region_info.offset,
+s->config_region_info.size);
+assert(QEMU_IS_ALIGNED(s->config_region_info.offset + ofs, size));
do {
ret = pread(s->device, buf, size, s->config_region_info.offset + ofs);
} while (ret == -1 && errno == EINTR);
@@ -237,6 +241,10 @@ static int qemu_vfio_pci_write_config(QEMUVFIOState *s,
void *buf, int size, int
{
int ret;
+trace_qemu_vfio_pci_write_config(buf, ofs, size,
+ s->config_region_info.offset,
+ s->config_region_info.size);
+assert(QEMU_IS_ALIGNED(s->config_region_info.offset + ofs, size));
do {
ret = pwrite(s->device, buf, size, s->config_region_info.offset + ofs);
} while (ret == -1 && errno == EINTR);
diff --git a/util/trace-events b/util/trace-events
index 24c31803b01..8d3615e717b 100644
--- a/util/trace-events
+++ b/util/trace-events
@@ -85,3 +85,5 @@ qemu_vfio_new_mapping(void *s, void *host, size_t size, int
index, uint64_t iova
qemu_vfio_do_mapping(void *s, void *host, size_t size, uint64_t iova) "s %p
host %p size 0x%zx iova 0x%"PRIx64
qemu_vfio_dma_map(void *s, void *host, size_t size, bool temporary, uint64_t
*iova) "s %p host %p size 0x%zx temporary %d iova %p"
qemu_vfio_dma_unmap(void *s, void *host) "s %p host %p"
+qemu_vfio_pci_read_config(void *buf, int ofs, int size, uint64_t region_ofs,
uint64_t region_size) "read cfg ptr %p ofs 0x%x size 0x%x (region addr
0x%"PRIx64" size 0x%"PRIx64")"
+qemu_vfio_pci_write_config(void *buf, int ofs, int size, uint64_t region_ofs,
uint64_t region_size) "write cfg ptr %p ofs 0x%x size 0x%x (region addr
0x%"PRIx64" size 0x%"PRIx64")"
--
2.26.2
Re: [PULL 09/12] qga: add ssh-{add,remove}-authorized-keys
On Mon, Nov 02, 2020 at 04:49:27PM +0100, Markus Armbruster wrote:
> Michael Roth writes:
>
> > From: Marc-André Lureau
> >
> > Add new commands to add and remove SSH public keys from
> > ~/.ssh/authorized_keys.
> >
> > I took a different approach for testing, including the unit tests right
> > with the code. I wanted to overwrite the function to get the user
> > details, I couldn't easily do that over QMP. Furthermore, I prefer
> > having unit tests very close to the code, and unit files that are domain
> > specific (commands-posix is too crowded already). FWIW, that
> > coding/testing style is Rust-style (where tests can or should even be
> > part of the documentation!).
> >
> > Fixes:
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.redhat.com%2Fshow_bug.cgi%3Fid%3D1885332data=04%7C01%7Cmichael.roth%40amd.com%7C7302cfdd51b547a8c3de08d87f46e6cf%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637399289788005891%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=IcYz5b1yBg3Q%2BPg82Z5VMdpf60vYHsLL518ENd0T1A4%3Dreserved=0
> >
> > Signed-off-by: Marc-André Lureau
> > Reviewed-by: Michal Privoznik
> > Reviewed-by: Daniel P. Berrangé
> > *squashed in fix-ups for setting file ownership and use of QAPI
> > conditionals for CONFIG_POSIX instead of stub definitions
> > Signed-off-by: Michael Roth
> > ---
> > qga/commands-posix-ssh.c | 407 +++
> > qga/meson.build | 20 +-
> > qga/qapi-schema.json | 35
> > 3 files changed, 461 insertions(+), 1 deletion(-)
> > create mode 100644 qga/commands-posix-ssh.c
> >
> > diff --git a/qga/commands-posix-ssh.c b/qga/commands-posix-ssh.c
> > new file mode 100644
> > index 00..a7bc9a1c24
> > --- /dev/null
> > +++ b/qga/commands-posix-ssh.c
> > @@ -0,0 +1,407 @@
> > + /*
> > + * This work is licensed under the terms of the GNU GPL, version 2 or
> > later.
> > + * See the COPYING file in the top-level directory.
> > + */
> > +#include "qemu/osdep.h"
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#include "qapi/error.h"
> > +#include "qga-qapi-commands.h"
> > +
> > +#ifdef QGA_BUILD_UNIT_TEST
> > +static struct passwd *
> > +test_get_passwd_entry(const gchar *user_name, GError **error)
> > +{
> > +struct passwd *p;
> > +int ret;
> > +
> > +if (!user_name || g_strcmp0(user_name, g_get_user_name())) {
> > +g_set_error(error, G_UNIX_ERROR, 0, "Invalid user name");
> > +return NULL;
> > +}
> > +
> > +p = g_new0(struct passwd, 1);
> > +p->pw_dir = (char *)g_get_home_dir();
> > +p->pw_uid = geteuid();
> > +p->pw_gid = getegid();
> > +
> > +ret = g_mkdir_with_parents(p->pw_dir, 0700);
> > +g_assert_cmpint(ret, ==, 0);
>
> checkpatch ERROR: Use g_assert or g_assert_not_reached
>
> See commit 6e9389563e "checkpatch: Disallow glib asserts in main code"
> for rationale.
Thanks for the pointer, I couldn't figure out what the issue was and
assumed it was just noise. Wish I noticed this message before I sent out
v2... v3 incoming.
>
> More below, and in PATCH 10+12.
>
> [...]
>
[PATCH-for-5.2 v3 0/7] util/vfio-helpers: Generic code strengthening
v3: - Extract reviewed patches from "util/vfio-helpers: Allow using multiple MSIX IRQs" - Added "Assert offset is aligned to page size" which would have helped debugging: "block/nvme: Fix use of write-only doorbells page on Aarch64 arch" Missing review: 7 Based-on: <20201029093306.1063879-1-phi...@redhat.com> Philippe Mathieu-Daudé (7): util/vfio-helpers: Improve reporting unsupported IOMMU type util/vfio-helpers: Trace PCI I/O config accesses util/vfio-helpers: Trace PCI BAR region info util/vfio-helpers: Trace where BARs are mapped util/vfio-helpers: Improve DMA trace events util/vfio-helpers: Convert vfio_dump_mapping to trace events util/vfio-helpers: Assert offset is aligned to page size util/vfio-helpers.c | 43 ++- util/trace-events | 10 -- 2 files changed, 34 insertions(+), 19 deletions(-) -- 2.26.2
[PATCH-for-5.2 v3 6/7] util/vfio-helpers: Convert vfio_dump_mapping to trace events
The QEMU_VFIO_DEBUG definition is only modifiable at build-time.
Trace events can be enabled at run-time. As we prefer the latter,
convert qemu_vfio_dump_mappings() to use trace events instead
of fprintf().
Reviewed-by: Fam Zheng
Reviewed-by: Stefan Hajnoczi
Signed-off-by: Philippe Mathieu-Daudé
---
util/vfio-helpers.c | 19 ---
util/trace-events | 1 +
2 files changed, 5 insertions(+), 15 deletions(-)
diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
index c24a510df82..73f7bfa7540 100644
--- a/util/vfio-helpers.c
+++ b/util/vfio-helpers.c
@@ -521,23 +521,12 @@ QEMUVFIOState *qemu_vfio_open_pci(const char *device,
Error **errp)
return s;
}
-static void qemu_vfio_dump_mapping(IOVAMapping *m)
-{
-if (QEMU_VFIO_DEBUG) {
-printf(" vfio mapping %p %" PRIx64 " to %" PRIx64 "\n", m->host,
- (uint64_t)m->size, (uint64_t)m->iova);
-}
-}
-
static void qemu_vfio_dump_mappings(QEMUVFIOState *s)
{
-int i;
-
-if (QEMU_VFIO_DEBUG) {
-printf("vfio mappings\n");
-for (i = 0; i < s->nr_mappings; ++i) {
-qemu_vfio_dump_mapping(>mappings[i]);
-}
+for (int i = 0; i < s->nr_mappings; ++i) {
+trace_qemu_vfio_dump_mapping(s->mappings[i].host,
+ s->mappings[i].iova,
+ s->mappings[i].size);
}
}
diff --git a/util/trace-events b/util/trace-events
index 08239941cb2..61e0d4bcdfe 100644
--- a/util/trace-events
+++ b/util/trace-events
@@ -80,6 +80,7 @@ qemu_mutex_unlock(void *mutex, const char *file, const int
line) "released mutex
qemu_vfio_dma_reset_temporary(void *s) "s %p"
qemu_vfio_ram_block_added(void *s, void *p, size_t size) "s %p host %p size
0x%zx"
qemu_vfio_ram_block_removed(void *s, void *p, size_t size) "s %p host %p size
0x%zx"
+qemu_vfio_dump_mapping(void *host, uint64_t iova, size_t size) "vfio mapping
%p to iova 0x%08" PRIx64 " size 0x%zx"
qemu_vfio_find_mapping(void *s, void *p) "s %p host %p"
qemu_vfio_new_mapping(void *s, void *host, size_t size, int index, uint64_t
iova) "s %p host %p size 0x%zx index %d iova 0x%"PRIx64
qemu_vfio_do_mapping(void *s, void *host, uint64_t iova, size_t size) "s %p
host %p <-> iova 0x%"PRIx64 " size 0x%zx"
--
2.26.2
[PATCH-for-5.2 v3 5/7] util/vfio-helpers: Improve DMA trace events
For debugging purpose, trace where DMA regions are mapped.
Reviewed-by: Fam Zheng
Reviewed-by: Stefan Hajnoczi
Signed-off-by: Philippe Mathieu-Daudé
---
util/vfio-helpers.c | 3 ++-
util/trace-events | 5 +++--
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
index 278c54902e7..c24a510df82 100644
--- a/util/vfio-helpers.c
+++ b/util/vfio-helpers.c
@@ -627,7 +627,7 @@ static int qemu_vfio_do_mapping(QEMUVFIOState *s, void
*host, size_t size,
.vaddr = (uintptr_t)host,
.size = size,
};
-trace_qemu_vfio_do_mapping(s, host, size, iova);
+trace_qemu_vfio_do_mapping(s, host, iova, size);
if (ioctl(s->container, VFIO_IOMMU_MAP_DMA, _map)) {
error_report("VFIO_MAP_DMA failed: %s", strerror(errno));
@@ -783,6 +783,7 @@ int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t
size,
}
}
}
+trace_qemu_vfio_dma_mapped(s, host, iova0, size);
if (iova) {
*iova = iova0;
}
diff --git a/util/trace-events b/util/trace-events
index 52d43cda3f3..08239941cb2 100644
--- a/util/trace-events
+++ b/util/trace-events
@@ -82,8 +82,9 @@ qemu_vfio_ram_block_added(void *s, void *p, size_t size) "s
%p host %p size 0x%z
qemu_vfio_ram_block_removed(void *s, void *p, size_t size) "s %p host %p size
0x%zx"
qemu_vfio_find_mapping(void *s, void *p) "s %p host %p"
qemu_vfio_new_mapping(void *s, void *host, size_t size, int index, uint64_t
iova) "s %p host %p size 0x%zx index %d iova 0x%"PRIx64
-qemu_vfio_do_mapping(void *s, void *host, size_t size, uint64_t iova) "s %p
host %p size 0x%zx iova 0x%"PRIx64
-qemu_vfio_dma_map(void *s, void *host, size_t size, bool temporary, uint64_t
*iova) "s %p host %p size 0x%zx temporary %d iova %p"
+qemu_vfio_do_mapping(void *s, void *host, uint64_t iova, size_t size) "s %p
host %p <-> iova 0x%"PRIx64 " size 0x%zx"
+qemu_vfio_dma_map(void *s, void *host, size_t size, bool temporary, uint64_t
*iova) "s %p host %p size 0x%zx temporary %d %p"
+qemu_vfio_dma_mapped(void *s, void *host, uint64_t iova, size_t size) "s %p
host %p <-> iova 0x%"PRIx64" size 0x%zx"
qemu_vfio_dma_unmap(void *s, void *host) "s %p host %p"
qemu_vfio_pci_read_config(void *buf, int ofs, int size, uint64_t region_ofs,
uint64_t region_size) "read cfg ptr %p ofs 0x%x size 0x%x (region addr
0x%"PRIx64" size 0x%"PRIx64")"
qemu_vfio_pci_write_config(void *buf, int ofs, int size, uint64_t region_ofs,
uint64_t region_size) "write cfg ptr %p ofs 0x%x size 0x%x (region addr
0x%"PRIx64" size 0x%"PRIx64")"
--
2.26.2
[PATCH-for-5.2 v3 1/7] util/vfio-helpers: Improve reporting unsupported IOMMU type
Change the confuse "VFIO IOMMU check failed" error message by
the explicit "VFIO IOMMU Type1 is not supported" once.
Example on POWER:
$ qemu-system-ppc64 -drive
if=none,id=nvme0,file=nvme://0001:01:00.0/1,format=raw
qemu-system-ppc64: -drive
if=none,id=nvme0,file=nvme://0001:01:00.0/1,format=raw: VFIO IOMMU Type1 is not
supported
Suggested-by: Alex Williamson
Reviewed-by: Fam Zheng
Reviewed-by: Stefan Hajnoczi
Signed-off-by: Philippe Mathieu-Daudé
---
util/vfio-helpers.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
index c469beb0616..14a549510fe 100644
--- a/util/vfio-helpers.c
+++ b/util/vfio-helpers.c
@@ -300,7 +300,7 @@ static int qemu_vfio_init_pci(QEMUVFIOState *s, const char
*device,
}
if (!ioctl(s->container, VFIO_CHECK_EXTENSION, VFIO_TYPE1_IOMMU)) {
-error_setg_errno(errp, errno, "VFIO IOMMU check failed");
+error_setg_errno(errp, errno, "VFIO IOMMU Type1 is not supported");
ret = -EINVAL;
goto fail_container;
}
--
2.26.2
[PATCH 6/6] migration: fix uninitialized variable warning in migrate_send_rp_req_pages()
After the WITH_QEMU_LOCK_GUARD macro is added, the compiler cannot identify
that the statements in the macro must be executed. As a result, some variables
assignment statements in the macro may be considered as unexecuted by the
compiler.
The compiler showed warning:
migration/migration.c: In function ‘migrate_send_rp_req_pages’:
migration/migration.c:384:8: warning: ‘received’ may be used uninitialized in
this function [-Wmaybe-uninitialized]
384 | if (received) {
|^
Add a default value for 'received' to prevented the warning.
Reported-by: Euler Robot
Signed-off-by: Chen Qun
---
Cc: Juan Quintela
Cc: "Dr. David Alan Gilbert"
---
migration/migration.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/migration/migration.c b/migration/migration.c
index 9bb4fee5ac..de90486a61 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -361,7 +361,7 @@ int migrate_send_rp_req_pages(MigrationIncomingState *mis,
RAMBlock *rb, ram_addr_t start, uint64_t haddr)
{
void *aligned = (void *)(uintptr_t)(haddr & (-qemu_ram_pagesize(rb)));
-bool received;
+bool received = false;
WITH_QEMU_LOCK_GUARD(>page_request_mutex) {
received = ramblock_recv_bitmap_test_byte_offset(rb, start);
--
2.27.0
[Bug 1901981] Re: assert issue locates in hw/usb/dev-storage.c:248: usb_msd_send_status
** Changed in: qemu Assignee: (unassigned) => Gaoning Pan (hades0506) -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1901981 Title: assert issue locates in hw/usb/dev-storage.c:248: usb_msd_send_status Status in QEMU: New Bug description: Hello, I found an assertion failure through hw/usb/dev-storage.c. This was found in latest version 5.1.0. qemu-system-x86_64: hw/usb/dev-storage.c:248: usb_msd_send_status: Assertion `s->csw.sig == cpu_to_le32(0x53425355)' failed. [1]29544 abort sudo -enable-kvm -boot c -m 2G -drive format=qcow2,file=./ubuntu.img -nic To reproduce the assertion failure, please run the QEMU with following command line. $ qemu-system-x86_64 -enable-kvm -boot c -m 2G -drive format=qcow2,file=./ubuntu.img -nic user,model=rtl8139,hostfwd=tcp:0.0.0.0:-:22 -device piix4-usb-uhci,id=uhci -device usb-storage,drive=mydrive -drive id=mydrive,file=null-co://,size=2M,format=raw,if=none The poc is attached. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1901981/+subscriptions
[PATCH 0/6] fix uninitialized variable warning
Hi all, There are some variables initialized warnings reported by the compiler, even if the default CFLAG for the compiler parameters are uesed. This serial has added some default values or changed the assignment places for the variablies to fix them. Thanks, Chen Qun Chen Qun (6): target/xtensa: fix uninitialized variable warning hw/rdma/rdma_backend: fix uninitialized variable warning in rdma_poll_cq() util/qemu-timer: fix uninitialized variable warning in timer_mod_anticipate_ns() util/qemu-timer: fix uninitialized variable warning for expire_time plugins/loader: fix uninitialized variable warning in plugin_reset_uninstall() migration: fix uninitialized variable warning in migrate_send_rp_req_pages() hw/rdma/rdma_backend.c| 2 +- migration/migration.c | 2 +- plugins/loader.c | 2 +- target/xtensa/translate.c | 2 +- util/qemu-timer.c | 8 +++- 5 files changed, 7 insertions(+), 9 deletions(-) -- 2.27.0
[PATCH 3/6] util/qemu-timer: fix uninitialized variable warning in timer_mod_anticipate_ns()
After the WITH_QEMU_LOCK_GUARD macro is added, the compiler cannot identify
that the statements in the macro must be executed. As a result, some variables
assignment statements in the macro may be considered as unexecuted by the
compiler.
The compiler showed warning:
util/qemu-timer.c: In function ‘timer_mod_anticipate_ns’:
util/qemu-timer.c:474:8: warning: ‘rearm’ may be used uninitialized in this
function [-Wmaybe-uninitialized]
474 | if (rearm) {
|^
Change the default value assignment place to prevented the warning.
Reported-by: Euler Robot
Signed-off-by: Chen Qun
---
Cc: Paolo Bonzini
---
util/qemu-timer.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/util/qemu-timer.c b/util/qemu-timer.c
index 81c28af517..8b73882fbb 100644
--- a/util/qemu-timer.c
+++ b/util/qemu-timer.c
@@ -459,7 +459,7 @@ void timer_mod_ns(QEMUTimer *ts, int64_t expire_time)
void timer_mod_anticipate_ns(QEMUTimer *ts, int64_t expire_time)
{
QEMUTimerList *timer_list = ts->timer_list;
-bool rearm;
+bool rearm = false;
WITH_QEMU_LOCK_GUARD(_list->active_timers_lock) {
if (ts->expire_time == -1 || ts->expire_time > expire_time) {
@@ -467,8 +467,6 @@ void timer_mod_anticipate_ns(QEMUTimer *ts, int64_t
expire_time)
timer_del_locked(timer_list, ts);
}
rearm = timer_mod_ns_locked(timer_list, ts, expire_time);
-} else {
-rearm = false;
}
}
if (rearm) {
--
2.27.0
[PATCH 1/6] target/xtensa: fix uninitialized variable warning
The compiler cannot determine whether the return values of the
xtensa_operand_is_register(isa, opc, opnd)
and xtensa_operand_is_visible(isa, opc, opnd) functions are the same.
So,it assumes that 'rf' is not assigned, but it's used.
The compiler showed warning:
target/xtensa/translate.c: In function ‘disas_xtensa_insn’:
target/xtensa/translate.c:985:43: warning: ‘rf’ may be used uninitialized in
this function [-Wmaybe-uninitialized]
985 | arg[vopnd].num_bits = xtensa_regfile_num_bits(isa,
rf);
|
^~~~
Add a default value for 'rf' to prevented the warning.
Reported-by: Euler Robot
Signed-off-by: Chen Qun
---
Cc: Max Filippov
---
target/xtensa/translate.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/xtensa/translate.c b/target/xtensa/translate.c
index 944a157747..eea851bbe7 100644
--- a/target/xtensa/translate.c
+++ b/target/xtensa/translate.c
@@ -953,7 +953,7 @@ static void disas_xtensa_insn(CPUXtensaState *env,
DisasContext *dc)
for (opnd = vopnd = 0; opnd < opnds; ++opnd) {
void **register_file = NULL;
-xtensa_regfile rf;
+xtensa_regfile rf = -1;
if (xtensa_operand_is_register(isa, opc, opnd)) {
rf = xtensa_operand_regfile(isa, opc, opnd);
--
2.27.0
[PATCH 5/6] plugins/loader: fix uninitialized variable warning in plugin_reset_uninstall()
After the WITH_QEMU_LOCK_GUARD macro is added, the compiler cannot identify
that the statements in the macro must be executed. As a result, some variables
assignment statements in the macro may be considered as unexecuted by the
compiler.
The compiler showed warning:
plugins/loader.c: In function ‘plugin_reset_uninstall’:
plugins/loader.c:382:15: warning: ‘ctx’ may be used uninitialized in this
function [-Wmaybe-uninitialized]
382 | data->ctx = ctx;
| ~~^
Add a default value for 'expire_time' to prevented the warning.
Reported-by: Euler Robot
Signed-off-by: Chen Qun
---
Cc: "Alex Bennée"
---
plugins/loader.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/plugins/loader.c b/plugins/loader.c
index 8ac5dbc20f..88593fe138 100644
--- a/plugins/loader.c
+++ b/plugins/loader.c
@@ -367,7 +367,7 @@ void plugin_reset_uninstall(qemu_plugin_id_t id,
bool reset)
{
struct qemu_plugin_reset_data *data;
-struct qemu_plugin_ctx *ctx;
+struct qemu_plugin_ctx *ctx = NULL;
WITH_QEMU_LOCK_GUARD() {
ctx = plugin_id_to_ctx_locked(id);
--
2.27.0
[PATCH 4/6] util/qemu-timer: fix uninitialized variable warning for expire_time
After the WITH_QEMU_LOCK_GUARD macro is added, the compiler cannot identify
that the statements in the macro must be executed. As a result, some variables
assignment statements in the macro may be considered as unexecuted by the
compiler.
The compiler showed warning:
util/qemu-timer.c: In function ‘timerlist_expired’:
util/qemu-timer.c:199:24: warning: ‘expire_time’ may be used uninitialized in
this function [-Wmaybe-uninitialized]
199 | return expire_time <= qemu_clock_get_ns(timer_list->clock->type);
|^
util/qemu-timer.c: In function ‘timerlist_deadline_ns’:
util/qemu-timer.c:237:11: warning: ‘expire_time’ may be used uninitialized in
this function [-Wmaybe-uninitialized]
237 | delta = expire_time - qemu_clock_get_ns(timer_list->clock->type);
| ~~^~
Add a default value for 'expire_time' to prevented the warning.
Reported-by: Euler Robot
Signed-off-by: Chen Qun
---
Cc: Paolo Bonzini
---
util/qemu-timer.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/util/qemu-timer.c b/util/qemu-timer.c
index 8b73882fbb..3910003e86 100644
--- a/util/qemu-timer.c
+++ b/util/qemu-timer.c
@@ -183,7 +183,7 @@ bool qemu_clock_has_timers(QEMUClockType type)
bool timerlist_expired(QEMUTimerList *timer_list)
{
-int64_t expire_time;
+int64_t expire_time = -1;
if (!qatomic_read(_list->active_timers)) {
return false;
@@ -213,7 +213,7 @@ bool qemu_clock_expired(QEMUClockType type)
int64_t timerlist_deadline_ns(QEMUTimerList *timer_list)
{
int64_t delta;
-int64_t expire_time;
+int64_t expire_time = -1;
if (!qatomic_read(_list->active_timers)) {
return -1;
--
2.27.0
[PATCH 2/6] hw/rdma/rdma_backend: fix uninitialized variable warning in rdma_poll_cq()
After the WITH_QEMU_LOCK_GUARD macro is added, the compiler cannot identify
that the statements in the macro must be executed. As a result, some variables
assignment statements in the macro may be considered as unexecuted by the
compiler.
The compiler showed warning:
hw/rdma/rdma_backend.c: In function ‘rdma_poll_cq’:
hw/rdma/rdma_utils.h:25:5: warning: ‘ne’ may be used uninitialized in this
function [-Wmaybe-uninitialized]
25 | error_report("%s: " fmt, "rdma", ## __VA_ARGS__)
| ^~~~
hw/rdma/rdma_backend.c:93:12: note: ‘ne’ was declared here
93 | int i, ne, total_ne = 0;
|^~
Add a default value for 'ne' to prevented the warning.
Reported-by: Euler Robot
Signed-off-by: Chen Qun
---
Cc: Yuval Shaia
Cc: Marcel Apfelbaum
---
hw/rdma/rdma_backend.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/rdma/rdma_backend.c b/hw/rdma/rdma_backend.c
index 5de010b1fa..2fe4a3501c 100644
--- a/hw/rdma/rdma_backend.c
+++ b/hw/rdma/rdma_backend.c
@@ -90,7 +90,7 @@ static void clean_recv_mads(RdmaBackendDev *backend_dev)
static int rdma_poll_cq(RdmaDeviceResources *rdma_dev_res, struct ibv_cq *ibcq)
{
-int i, ne, total_ne = 0;
+int i, ne = 0, total_ne = 0;
BackendCtx *bctx;
struct ibv_wc wc[2];
RdmaProtectedGSList *cqe_ctx_list;
--
2.27.0
Re: [PATCH 0/3] tests/qtest: npcm7xx test fixes
Cc'ing Daniel (patches 1-3) & Marc-André (2). On 11/3/20 2:14 AM, Havard Skinnemoen via wrote: > This series contains a fix for the randomness calculation in npcm7xx_rng-test. > It also makes test failures fatal. The last patch would have dumped the random > data to stderr if the randomness test fails, except now that failures are > fatal, it never actually gets a chance to do that. > > It may not make sense to apply all three, but I'd definitely take (1), and > I'll > leave it up to you whether to apply (2), (3) or both. > > Havard Skinnemoen (3): > tests/qtest/npcm7xx_rng-test: count runs properly > tests/qtest/npcm7xx: Don't call g_test_set_nonfatal_assertions > tests/qtest/npcm7xx_rng-test: dump random data on failure > > tests/qtest/npcm7xx_gpio-test.c | 1 - > tests/qtest/npcm7xx_rng-test.c| 15 +-- > tests/qtest/npcm7xx_timer-test.c | 1 - > tests/qtest/npcm7xx_watchdog_timer-test.c | 1 - > 4 files changed, 13 insertions(+), 5 deletions(-) >
Re: [RFC PATCH v2 05/13] hw: add compat machines for 5.3
On 10/30/2020 1:08 AM, Andrew Jones wrote:
On Tue, Oct 20, 2020 at 09:14:32PM +0800, Ying Fang wrote:
Add 5.2 machine types for arm/i440fx/q35/s390x/spapr.
^ 5.3
Thanks. Will fix, careless spelling mistake.
Thanks,
drew
Signed-off-by: Ying Fang
---
hw/arm/virt.c | 9 -
hw/core/machine.c | 3 +++
hw/i386/pc.c | 3 +++
hw/i386/pc_piix.c | 15 ++-
hw/i386/pc_q35.c | 14 +-
hw/ppc/spapr.c | 15 +--
hw/s390x/s390-virtio-ccw.c | 14 +-
include/hw/boards.h| 3 +++
include/hw/i386/pc.h | 3 +++
9 files changed, 73 insertions(+), 6 deletions(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index ba902b53ba..ff8a14439e 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2665,10 +2665,17 @@ static void machvirt_machine_init(void)
}
type_init(machvirt_machine_init);
+static void virt_machine_5_3_options(MachineClass *mc)
+{
+}
+DEFINE_VIRT_MACHINE_AS_LATEST(5, 3)
+
static void virt_machine_5_2_options(MachineClass *mc)
{
+virt_machine_5_3_options(mc);
+compat_props_add(mc->compat_props, hw_compat_5_2, hw_compat_5_2_len);
}
-DEFINE_VIRT_MACHINE_AS_LATEST(5, 2)
+DEFINE_VIRT_MACHINE(5, 2)
static void virt_machine_5_1_options(MachineClass *mc)
{
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 7e2f4ec08e..6dc77699a9 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -28,6 +28,9 @@
#include "hw/mem/nvdimm.h"
#include "migration/vmstate.h"
+GlobalProperty hw_compat_5_2[] = { };
+const size_t hw_compat_5_2_len = G_N_ELEMENTS(hw_compat_5_2);
+
GlobalProperty hw_compat_5_1[] = {
{ "vhost-scsi", "num_queues", "1"},
{ "vhost-user-blk", "num-queues", "1"},
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index e87be5d29a..eaa046ff5d 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -97,6 +97,9 @@
#include "trace.h"
#include CONFIG_DEVICES
+GlobalProperty pc_compat_5_2[] = { };
+const size_t pc_compat_5_2_len = G_N_ELEMENTS(pc_compat_5_2);
+
GlobalProperty pc_compat_5_1[] = {
{ "ICH9-LPC", "x-smi-cpu-hotplug", "off" },
};
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 3c2ae0612b..01254090ce 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -426,7 +426,7 @@ static void pc_i440fx_machine_options(MachineClass *m)
machine_class_allow_dynamic_sysbus_dev(m, TYPE_VMBUS_BRIDGE);
}
-static void pc_i440fx_5_2_machine_options(MachineClass *m)
+static void pc_i440fx_5_3_machine_options(MachineClass *m)
{
PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
pc_i440fx_machine_options(m);
@@ -435,6 +435,19 @@ static void pc_i440fx_5_2_machine_options(MachineClass *m)
pcmc->default_cpu_version = 1;
}
+DEFINE_I440FX_MACHINE(v5_3, "pc-i440fx-5.3", NULL,
+ pc_i440fx_5_3_machine_options);
+
+static void pc_i440fx_5_2_machine_options(MachineClass *m)
+{
+PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
+pc_i440fx_machine_options(m);
+m->alias = NULL;
+m->is_default = false;
+compat_props_add(m->compat_props, hw_compat_5_2, hw_compat_5_2_len);
+compat_props_add(m->compat_props, pc_compat_5_2, pc_compat_5_2_len);
+}
+
DEFINE_I440FX_MACHINE(v5_2, "pc-i440fx-5.2", NULL,
pc_i440fx_5_2_machine_options);
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index a3f4959c43..dd14803edb 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -344,7 +344,7 @@ static void pc_q35_machine_options(MachineClass *m)
m->max_cpus = 288;
}
-static void pc_q35_5_2_machine_options(MachineClass *m)
+static void pc_q35_5_3_machine_options(MachineClass *m)
{
PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
pc_q35_machine_options(m);
@@ -352,6 +352,18 @@ static void pc_q35_5_2_machine_options(MachineClass *m)
pcmc->default_cpu_version = 1;
}
+DEFINE_Q35_MACHINE(v5_3, "pc-q35-5.3", NULL,
+ pc_q35_5_3_machine_options);
+
+static void pc_q35_5_2_machine_options(MachineClass *m)
+{
+PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
+pc_q35_machine_options(m);
+m->alias = NULL;
+compat_props_add(m->compat_props, hw_compat_5_2, hw_compat_5_2_len);
+compat_props_add(m->compat_props, pc_compat_5_2, pc_compat_5_2_len);
+}
+
DEFINE_Q35_MACHINE(v5_2, "pc-q35-5.2", NULL,
pc_q35_5_2_machine_options);
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 2db810f73a..c292a3edd9 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -4511,15 +4511,26 @@ static void
spapr_machine_latest_class_options(MachineClass *mc)
}\
type_init(spapr_machine_register_##suffix)
+/*
+ * pseries-5.3
+ */
+static void spapr_machine_5_3_class_options(MachineClass *mc)
+{
+/* Defaults for the latest behaviour inherited from the base class */
+}
+
+DEFINE_SPAPR_MACHINE(5_3, "5.3", true);
+
Re: [PULL v2 00/12] qemu-ga patch queue for soft-freeze
Patchew URL:
https://patchew.org/QEMU/20201103011134.887744-1-michael.r...@amd.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20201103011134.887744-1-michael.r...@amd.com
Subject: [PULL v2 00/12] qemu-ga patch queue for soft-freeze
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
- [tag update] patchew/20201102202710.2224691-1-f4...@amsat.org ->
patchew/20201102202710.2224691-1-f4...@amsat.org
- [tag update] patchew/20201102203609.2228309-1-phi...@redhat.com ->
patchew/20201102203609.2228309-1-phi...@redhat.com
* [new tag] patchew/20201103011134.887744-1-michael.r...@amd.com ->
patchew/20201103011134.887744-1-michael.r...@amd.com
Switched to a new branch 'test'
134f664 qga: add ssh-get-authorized-keys
9f5e642 meson: minor simplification
445e4c4 qga: add *reset argument to ssh-add-authorized-keys
7ab2def qga: add ssh-{add,remove}-authorized-keys
b137836 glib-compat: add g_unix_get_passwd_entry_qemu()
bf1041a qga: add implementation of guest-get-disks for Windows
78bc690 qga: add implementation of guest-get-disks for Linux
de60426 qga: add command guest-get-disks
e8dc031 qga: Flatten simple union GuestDeviceId
a78863f qga-win: Fix guest-get-devices error API violations
3d3253f qga: Use common time encoding for guest-get-devices 'driver-date'
1965fac qga: Rename guest-get-devices return member 'address' to 'id'
=== OUTPUT BEGIN ===
1/12 Checking commit 1965fac569d1 (qga: Rename guest-get-devices return member
'address' to 'id')
2/12 Checking commit 3d3253f3695c (qga: Use common time encoding for
guest-get-devices 'driver-date')
3/12 Checking commit a78863f8a833 (qga-win: Fix guest-get-devices error API
violations)
4/12 Checking commit e8dc03167d53 (qga: Flatten simple union GuestDeviceId)
5/12 Checking commit de60426a744f (qga: add command guest-get-disks)
6/12 Checking commit 78bc69001515 (qga: add implementation of guest-get-disks
for Linux)
7/12 Checking commit bf1041a868af (qga: add implementation of guest-get-disks
for Windows)
8/12 Checking commit b1378362971f (glib-compat: add
g_unix_get_passwd_entry_qemu())
WARNING: Block comments use a leading /* on a separate line
#42: FILE: include/glib-compat.h:81:
+/* Note: The fallback implementation is not MT-safe, and it returns a copy of
WARNING: Block comments use a trailing */ on a separate line
#45: FILE: include/glib-compat.h:84:
+ * GLib API substitution. */
total: 0 errors, 2 warnings, 38 lines checked
Patch 8/12 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
9/12 Checking commit 7ab2def768e8 (qga: add ssh-{add,remove}-authorized-keys)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#32:
new file mode 100644
ERROR: Use g_assert or g_assert_not_reached
#69: FILE: qga/commands-posix-ssh.c:33:
+g_assert_cmpint(ret, ==, 0);
ERROR: Use g_assert or g_assert_not_reached
#330: FILE: qga/commands-posix-ssh.c:294:
+g_assert_cmpint(ret, ==, 0);
ERROR: Use g_assert or g_assert_not_reached
#335: FILE: qga/commands-posix-ssh.c:299:
+g_assert_no_error(err);
ERROR: Use g_assert or g_assert_not_reached
#347: FILE: qga/commands-posix-ssh.c:311:
+g_assert_no_error(err);
ERROR: Use g_assert or g_assert_not_reached
#349: FILE: qga/commands-posix-ssh.c:313:
+g_assert_cmpstr(contents, ==, expected);
ERROR: Use g_assert or g_assert_not_reached
#386: FILE: qga/commands-posix-ssh.c:350:
+g_assert_null(err);
ERROR: Use g_assert or g_assert_not_reached
#392: FILE: qga/commands-posix-ssh.c:356:
+g_assert_null(err);
ERROR: Use g_assert or g_assert_not_reached
#413: FILE: qga/commands-posix-ssh.c:377:
+g_assert_null(err);
ERROR: Use g_assert or g_assert_not_reached
#418: FILE: qga/commands-posix-ssh.c:382:
+g_assert_null(err);
total: 9 errors, 1 warnings, 479 lines checked
Patch 9/12 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
10/12 Checking commit 445e4c4ff87d (qga: add *reset argument to
ssh-add-authorized-keys)
ERROR: Use g_assert or g_assert_not_reached
#96: FILE: qga/commands-posix-ssh.c:381:
+g_assert_null(err);
ERROR: Use g_assert or g_assert_not_reached
#106: FILE: qga/commands-posix-ssh.c:391:
+g_assert_null(err);
ERROR: Use g_assert or g_assert_not_reached
#115: FILE: qga/commands-posix-ssh.c:400:
+g_assert_null(err);
total: 3 errors, 0 warnings, 121 lines checked
Patch 10/12 has style problems, please review. If any of these errors
are false positives report them to the
[PATCH 3/3] tests/qtest/npcm7xx_rng-test: dump random data on failure
Dump the collected random data after a randomness test failure.
Note that you won't actually see this unless you add
g_test_set_nonfatal_assertions() back in.
Signed-off-by: Havard Skinnemoen
---
tests/qtest/npcm7xx_rng-test.c | 12
1 file changed, 12 insertions(+)
diff --git a/tests/qtest/npcm7xx_rng-test.c b/tests/qtest/npcm7xx_rng-test.c
index d7e42cf062..09111d640c 100644
--- a/tests/qtest/npcm7xx_rng-test.c
+++ b/tests/qtest/npcm7xx_rng-test.c
@@ -20,6 +20,7 @@
#include "libqtest-single.h"
#include "qemu/bitops.h"
+#include "qemu-common.h"
#define RNG_BASE_ADDR 0xf000b000
@@ -36,6 +37,13 @@
/* Number of bits to collect for randomness tests. */
#define TEST_INPUT_BITS (128)
+static void dump_buf_if_failed(const uint8_t *buf, size_t size)
+{
+if (g_test_failed()) {
+qemu_hexdump(stderr, "", buf, size);
+}
+}
+
static void rng_writeb(unsigned int offset, uint8_t value)
{
writeb(RNG_BASE_ADDR + offset, value);
@@ -188,6 +196,7 @@ static void test_continuous_monobit(void)
}
g_assert_cmpfloat(calc_monobit_p(buf, sizeof(buf)), >, 0.01);
+dump_buf_if_failed(buf, sizeof(buf));
}
/*
@@ -209,6 +218,7 @@ static void test_continuous_runs(void)
}
g_assert_cmpfloat(calc_runs_p(buf.l, sizeof(buf) * BITS_PER_BYTE), >,
0.01);
+dump_buf_if_failed(buf.c, sizeof(buf));
}
/*
@@ -230,6 +240,7 @@ static void test_first_byte_monobit(void)
}
g_assert_cmpfloat(calc_monobit_p(buf, sizeof(buf)), >, 0.01);
+dump_buf_if_failed(buf, sizeof(buf));
}
/*
@@ -254,6 +265,7 @@ static void test_first_byte_runs(void)
}
g_assert_cmpfloat(calc_runs_p(buf.l, sizeof(buf) * BITS_PER_BYTE), >,
0.01);
+dump_buf_if_failed(buf.c, sizeof(buf));
}
int main(int argc, char **argv)
--
2.29.1.341.ge80a0c044ae-goog
[PATCH 2/3] tests/qtest/npcm7xx: Don't call g_test_set_nonfatal_assertions
Even though g_test_set_nonfatal_assertions() makes test failure
reporting a lot better, no other tests currently do this so we'll turn
it off as well.
Signed-off-by: Havard Skinnemoen
---
tests/qtest/npcm7xx_gpio-test.c | 1 -
tests/qtest/npcm7xx_rng-test.c| 1 -
tests/qtest/npcm7xx_timer-test.c | 1 -
tests/qtest/npcm7xx_watchdog_timer-test.c | 1 -
4 files changed, 4 deletions(-)
diff --git a/tests/qtest/npcm7xx_gpio-test.c b/tests/qtest/npcm7xx_gpio-test.c
index 1004cef812..3af49173a7 100644
--- a/tests/qtest/npcm7xx_gpio-test.c
+++ b/tests/qtest/npcm7xx_gpio-test.c
@@ -357,7 +357,6 @@ int main(int argc, char **argv)
int i;
g_test_init(, , NULL);
-g_test_set_nonfatal_assertions();
qtest_add_func("/npcm7xx_gpio/dout_to_din", test_dout_to_din);
qtest_add_func("/npcm7xx_gpio/pullup_pulldown", test_pullup_pulldown);
diff --git a/tests/qtest/npcm7xx_rng-test.c b/tests/qtest/npcm7xx_rng-test.c
index 57787c5ffc..d7e42cf062 100644
--- a/tests/qtest/npcm7xx_rng-test.c
+++ b/tests/qtest/npcm7xx_rng-test.c
@@ -261,7 +261,6 @@ int main(int argc, char **argv)
int ret;
g_test_init(, , NULL);
-g_test_set_nonfatal_assertions();
qtest_add_func("npcm7xx_rng/enable_disable", test_enable_disable);
qtest_add_func("npcm7xx_rng/rosel", test_rosel);
diff --git a/tests/qtest/npcm7xx_timer-test.c b/tests/qtest/npcm7xx_timer-test.c
index f08b0cd62a..77e6e0d472 100644
--- a/tests/qtest/npcm7xx_timer-test.c
+++ b/tests/qtest/npcm7xx_timer-test.c
@@ -530,7 +530,6 @@ int main(int argc, char **argv)
int i, j;
g_test_init(, , NULL);
-g_test_set_nonfatal_assertions();
for (i = 0; i < ARRAY_SIZE(timer_block); i++) {
for (j = 0; j < ARRAY_SIZE(timer); j++) {
diff --git a/tests/qtest/npcm7xx_watchdog_timer-test.c
b/tests/qtest/npcm7xx_watchdog_timer-test.c
index 54d5d6d8f2..426303ecfa 100644
--- a/tests/qtest/npcm7xx_watchdog_timer-test.c
+++ b/tests/qtest/npcm7xx_watchdog_timer-test.c
@@ -303,7 +303,6 @@ static void watchdog_add_test(const char *name, const
Watchdog* wd,
int main(int argc, char **argv)
{
g_test_init(, , NULL);
-g_test_set_nonfatal_assertions();
for (int i = 0; i < ARRAY_SIZE(watchdog_list); ++i) {
const Watchdog *wd = _list[i];
--
2.29.1.341.ge80a0c044ae-goog
[PULL v2 05/12] qga: add command guest-get-disks
From: Tomáš Golembiovský
Add API and stubs for new guest-get-disks command.
The command guest-get-fsinfo can be used to list information about disks
and partitions but it is limited only to mounted disks with filesystem.
This new command should allow listing information about disks of the VM
regardles whether they are mounted or not. This can be usefull for
management applications for mapping virtualized devices or pass-through
devices to device names in the guest OS.
Signed-off-by: Tomáš Golembiovský
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Marc-André Lureau
Signed-off-by: Michael Roth
---
qga/commands-posix.c | 6 ++
qga/commands-win32.c | 6 ++
qga/qapi-schema.json | 31 +++
3 files changed, 43 insertions(+)
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 3bffee99d4..422144bcff 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -3051,3 +3051,9 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
return NULL;
}
+
+GuestDiskInfoList *qmp_guest_get_disks(Error **errp)
+{
+error_setg(errp, QERR_UNSUPPORTED);
+return NULL;
+}
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 0c33d48aaa..f7bdd5a8b5 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -2458,3 +2458,9 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
}
return head;
}
+
+GuestDiskInfoList *qmp_guest_get_disks(Error **errp)
+{
+error_setg(errp, QERR_UNSUPPORTED);
+return NULL;
+}
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index fe10631e4c..e123a000be 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -865,6 +865,37 @@
'bus': 'int', 'target': 'int', 'unit': 'int',
'*serial': 'str', '*dev': 'str'} }
+##
+# @GuestDiskInfo:
+#
+# @name: device node (Linux) or device UNC (Windows)
+# @partition: whether this is a partition or disk
+# @dependents: list of dependent devices; e.g. for LVs of the LVM this will
+# hold the list of PVs, for LUKS encrypted volume this will
+# contain the disk where the volume is placed. (Linux)
+# @address: disk address information (only for non-virtual devices)
+# @alias: optional alias assigned to the disk, on Linux this is a name assigned
+# by device mapper
+#
+# Since 5.2
+##
+{ 'struct': 'GuestDiskInfo',
+ 'data': {'name': 'str', 'partition': 'bool', 'dependents': ['str'],
+ '*address': 'GuestDiskAddress', '*alias': 'str'} }
+
+##
+# @guest-get-disks:
+#
+# Returns: The list of disks in the guest. For Windows these are only the
+# physical disks. On Linux these are all root block devices of
+# non-zero size including e.g. removable devices, loop devices,
+# NBD, etc.
+#
+# Since: 5.2
+##
+{ 'command': 'guest-get-disks',
+ 'returns': ['GuestDiskInfo'] }
+
##
# @GuestFilesystemInfo:
#
--
2.25.1
[PULL v2 09/12] qga: add ssh-{add,remove}-authorized-keys
From: Marc-André Lureau
Add new commands to add and remove SSH public keys from
~/.ssh/authorized_keys.
I took a different approach for testing, including the unit tests right
with the code. I wanted to overwrite the function to get the user
details, I couldn't easily do that over QMP. Furthermore, I prefer
having unit tests very close to the code, and unit files that are domain
specific (commands-posix is too crowded already). FWIW, that
coding/testing style is Rust-style (where tests can or should even be
part of the documentation!).
Fixes:
https://bugzilla.redhat.com/show_bug.cgi?id=1885332
Signed-off-by: Marc-André Lureau
Reviewed-by: Michal Privoznik
Reviewed-by: Daniel P. Berrangé
*squashed in fix-ups for setting file ownership and use of QAPI
conditionals for CONFIG_POSIX instead of stub definitions
*disable qga-ssh-test for now due to G_TEST_OPTION_ISOLATE_DIRS
triggering leak detector in build-oss-fuzz
Signed-off-by: Michael Roth
---
qga/commands-posix-ssh.c | 407 +++
qga/meson.build | 25 ++-
qga/qapi-schema.json | 35
3 files changed, 466 insertions(+), 1 deletion(-)
create mode 100644 qga/commands-posix-ssh.c
diff --git a/qga/commands-posix-ssh.c b/qga/commands-posix-ssh.c
new file mode 100644
index 00..a7bc9a1c24
--- /dev/null
+++ b/qga/commands-posix-ssh.c
@@ -0,0 +1,407 @@
+ /*
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#include "qemu/osdep.h"
+
+#include
+#include
+#include
+#include
+
+#include "qapi/error.h"
+#include "qga-qapi-commands.h"
+
+#ifdef QGA_BUILD_UNIT_TEST
+static struct passwd *
+test_get_passwd_entry(const gchar *user_name, GError **error)
+{
+struct passwd *p;
+int ret;
+
+if (!user_name || g_strcmp0(user_name, g_get_user_name())) {
+g_set_error(error, G_UNIX_ERROR, 0, "Invalid user name");
+return NULL;
+}
+
+p = g_new0(struct passwd, 1);
+p->pw_dir = (char *)g_get_home_dir();
+p->pw_uid = geteuid();
+p->pw_gid = getegid();
+
+ret = g_mkdir_with_parents(p->pw_dir, 0700);
+g_assert_cmpint(ret, ==, 0);
+
+return p;
+}
+
+#define g_unix_get_passwd_entry_qemu(username, err) \
+ test_get_passwd_entry(username, err)
+#endif
+
+static struct passwd *
+get_passwd_entry(const char *username, Error **errp)
+{
+g_autoptr(GError) err = NULL;
+struct passwd *p;
+
+ERRP_GUARD();
+
+p = g_unix_get_passwd_entry_qemu(username, );
+if (p == NULL) {
+error_setg(errp, "failed to lookup user '%s': %s",
+ username, err->message);
+return NULL;
+}
+
+return p;
+}
+
+static bool
+mkdir_for_user(const char *path, const struct passwd *p,
+ mode_t mode, Error **errp)
+{
+ERRP_GUARD();
+
+if (g_mkdir(path, mode) == -1) {
+error_setg(errp, "failed to create directory '%s': %s",
+ path, g_strerror(errno));
+return false;
+}
+
+if (chown(path, p->pw_uid, p->pw_gid) == -1) {
+error_setg(errp, "failed to set ownership of directory '%s': %s",
+ path, g_strerror(errno));
+return false;
+}
+
+if (chmod(path, mode) == -1) {
+error_setg(errp, "failed to set permissions of directory '%s': %s",
+ path, g_strerror(errno));
+return false;
+}
+
+return true;
+}
+
+static bool
+check_openssh_pub_key(const char *key, Error **errp)
+{
+ERRP_GUARD();
+
+/* simple sanity-check, we may want more? */
+if (!key || key[0] == '#' || strchr(key, '\n')) {
+error_setg(errp, "invalid OpenSSH public key: '%s'", key);
+return false;
+}
+
+return true;
+}
+
+static bool
+check_openssh_pub_keys(strList *keys, size_t *nkeys, Error **errp)
+{
+size_t n = 0;
+strList *k;
+
+ERRP_GUARD();
+
+for (k = keys; k != NULL; k = k->next) {
+if (!check_openssh_pub_key(k->value, errp)) {
+return false;
+}
+n++;
+}
+
+if (nkeys) {
+*nkeys = n;
+}
+return true;
+}
+
+static bool
+write_authkeys(const char *path, const GStrv keys,
+ const struct passwd *p, Error **errp)
+{
+g_autofree char *contents = NULL;
+g_autoptr(GError) err = NULL;
+
+ERRP_GUARD();
+
+contents = g_strjoinv("\n", keys);
+if (!g_file_set_contents(path, contents, -1, )) {
+error_setg(errp, "failed to write to '%s': %s", path, err->message);
+return false;
+}
+
+if (chown(path, p->pw_uid, p->pw_gid) == -1) {
+error_setg(errp, "failed to set ownership of directory '%s': %s",
+ path, g_strerror(errno));
+return false;
+}
+
+if (chmod(path, 0600) == -1) {
+error_setg(errp, "failed to set permissions of '%s': %s",
+ path, g_strerror(errno));
+return false;
+}
+
+return true;
+}
+
+static
[PULL v2 01/12] qga: Rename guest-get-devices return member 'address' to 'id'
From: Markus Armbruster
Member 'address' is union GuestDeviceAddress with a single branch
GuestDeviceAddressPCI, containing PCI vendor ID and device ID. This
is not a PCI address. Type GuestPCIAddress is. Messed up in recent
commit 2e4211cee4 "qga: add command guest-get-devices for reporting
VirtIO devices".
Rename type GuestDeviceAddressPCI to GuestDeviceIdPCI, type
GuestDeviceAddress to GuestDeviceId, and member 'address' to 'id'.
Document the member properly while there.
Signed-off-by: Markus Armbruster
Reviewed-by: Marc-André Lureau
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Michael Roth
---
qga/commands-win32.c | 16
qga/qapi-schema.json | 17 +
2 files changed, 17 insertions(+), 16 deletions(-)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 0c3c05484f..879b02b6c3 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -2390,22 +2390,22 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
}
for (j = 0; hw_ids[j] != NULL; j++) {
GMatchInfo *match_info;
-GuestDeviceAddressPCI *address;
+GuestDeviceIdPCI *id;
if (!g_regex_match(device_pci_re, hw_ids[j], 0, _info)) {
continue;
}
skip = false;
-address = g_new0(GuestDeviceAddressPCI, 1);
+id = g_new0(GuestDeviceIdPCI, 1);
vendor_id = g_match_info_fetch(match_info, 1);
device_id = g_match_info_fetch(match_info, 2);
-address->vendor_id = g_ascii_strtoull(vendor_id, NULL, 16);
-address->device_id = g_ascii_strtoull(device_id, NULL, 16);
+id->vendor_id = g_ascii_strtoull(vendor_id, NULL, 16);
+id->device_id = g_ascii_strtoull(device_id, NULL, 16);
-device->address = g_new0(GuestDeviceAddress, 1);
-device->has_address = true;
-device->address->type = GUEST_DEVICE_ADDRESS_KIND_PCI;
-device->address->u.pci.data = address;
+device->id = g_new0(GuestDeviceId, 1);
+device->has_id = true;
+device->id->type = GUEST_DEVICE_ID_KIND_PCI;
+device->id->u.pci.data = id;
g_match_info_free(match_info);
break;
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index cec98c7e06..f2c81cda2b 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -1257,26 +1257,26 @@
'returns': 'GuestOSInfo' }
##
-# @GuestDeviceAddressPCI:
+# @GuestDeviceIdPCI:
#
# @vendor-id: vendor ID
# @device-id: device ID
#
# Since: 5.2
##
-{ 'struct': 'GuestDeviceAddressPCI',
+{ 'struct': 'GuestDeviceIdPCI',
'data': { 'vendor-id': 'uint16', 'device-id': 'uint16' } }
##
-# @GuestDeviceAddress:
+# @GuestDeviceId:
#
-# Address of the device
-# - @pci: address of PCI device, since: 5.2
+# Id of the device
+# - @pci: PCI ID, since: 5.2
#
# Since: 5.2
##
-{ 'union': 'GuestDeviceAddress',
- 'data': { 'pci': 'GuestDeviceAddressPCI' } }
+{ 'union': 'GuestDeviceId',
+ 'data': { 'pci': 'GuestDeviceIdPCI' } }
##
# @GuestDeviceInfo:
@@ -1284,6 +1284,7 @@
# @driver-name: name of the associated driver
# @driver-date: driver release date in format -MM-DD
# @driver-version: driver version
+# @id: device ID
#
# Since: 5.2
##
@@ -1292,7 +1293,7 @@
'driver-name': 'str',
'*driver-date': 'str',
'*driver-version': 'str',
- '*address': 'GuestDeviceAddress'
+ '*id': 'GuestDeviceId'
} }
##
--
2.25.1
[PULL v2 06/12] qga: add implementation of guest-get-disks for Linux
From: Tomáš Golembiovský
The command lists all disks (real and virtual) as well as disk
partitions. For each disk the list of dependent disks is also listed and
/dev path is used as a handle so it can be matched with "name" field of
other returned disk entries. For disk partitions the "dependents" list
is populated with the the parent device for easier tracking of
hierarchy.
Example output:
{
"return": [
...
{
"name": "/dev/dm-0",
"partition": false,
"dependents": [
"/dev/sda2"
],
"alias": "luks-7062202e-5b9b-433e-81e8-6628c40da9f7"
},
{
"name": "/dev/sda2",
"partition": true,
"dependents": [
"/dev/sda"
]
},
{
"name": "/dev/sda",
"partition": false,
"address": {
"serial": "SAMSUNG_MZ7LN512HCHP-000L1_S1ZKNXAG822493",
"bus-type": "sata",
...
"dev": "/dev/sda",
"target": 0
},
"dependents": []
},
...
]
}
Signed-off-by: Tomáš Golembiovský
Reviewed-by: Marc-André Lureau
*add missing stub for !defined(CONFIG_FSFREEZE)
*remove unused deps_dir variable
Signed-off-by: Michael Roth
---
qga/commands-posix.c | 303 +--
1 file changed, 292 insertions(+), 11 deletions(-)
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 422144bcff..3711080d07 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -1150,13 +1150,27 @@ static void build_guest_fsinfo_for_virtual_device(char
const *syspath,
closedir(dir);
}
+static bool is_disk_virtual(const char *devpath, Error **errp)
+{
+g_autofree char *syspath = realpath(devpath, NULL);
+
+if (!syspath) {
+error_setg_errno(errp, errno, "realpath(\"%s\")", devpath);
+return false;
+}
+return strstr(syspath, "/devices/virtual/block/") != NULL;
+}
+
/* Dispatch to functions for virtual/real device */
static void build_guest_fsinfo_for_device(char const *devpath,
GuestFilesystemInfo *fs,
Error **errp)
{
-char *syspath = realpath(devpath, NULL);
+ERRP_GUARD();
+g_autofree char *syspath = NULL;
+bool is_virtual = false;
+syspath = realpath(devpath, NULL);
if (!syspath) {
error_setg_errno(errp, errno, "realpath(\"%s\")", devpath);
return;
@@ -1167,16 +1181,281 @@ static void build_guest_fsinfo_for_device(char const
*devpath,
}
g_debug(" parse sysfs path '%s'", syspath);
-
-if (strstr(syspath, "/devices/virtual/block/")) {
+is_virtual = is_disk_virtual(syspath, errp);
+if (*errp != NULL) {
+return;
+}
+if (is_virtual) {
build_guest_fsinfo_for_virtual_device(syspath, fs, errp);
} else {
build_guest_fsinfo_for_real_device(syspath, fs, errp);
}
+}
+
+#ifdef CONFIG_LIBUDEV
+
+/*
+ * Wrapper around build_guest_fsinfo_for_device() for getting just
+ * the disk address.
+ */
+static GuestDiskAddress *get_disk_address(const char *syspath, Error **errp)
+{
+g_autoptr(GuestFilesystemInfo) fs = NULL;
-free(syspath);
+fs = g_new0(GuestFilesystemInfo, 1);
+build_guest_fsinfo_for_device(syspath, fs, errp);
+if (fs->disk != NULL) {
+return g_steal_pointer(>disk->value);
+}
+return NULL;
}
+static char *get_alias_for_syspath(const char *syspath)
+{
+struct udev *udev = NULL;
+struct udev_device *udevice = NULL;
+char *ret = NULL;
+
+udev = udev_new();
+if (udev == NULL) {
+g_debug("failed to query udev");
+goto out;
+}
+udevice = udev_device_new_from_syspath(udev, syspath);
+if (udevice == NULL) {
+g_debug("failed to query udev for path: %s", syspath);
+goto out;
+} else {
+const char *alias = udev_device_get_property_value(
+udevice, "DM_NAME");
+/*
+ * NULL means there was an error and empty string means there is no
+ * alias. In case of no alias we return NULL instead of empty string.
+ */
+if (alias == NULL) {
+g_debug("failed to query udev for device alias for: %s",
+syspath);
+} else if (*alias != 0) {
+ret = g_strdup(alias);
+}
+}
+
+out:
+udev_unref(udev);
+udev_device_unref(udevice);
+return ret;
+}
+
+static char *get_device_for_syspath(const char *syspath)
+{
+struct udev *udev = NULL;
+struct udev_device *udevice = NULL;
+char *ret = NULL;
+
+udev = udev_new();
+if (udev == NULL) {
+g_debug("failed to query udev");
+goto out;
+}
+udevice = udev_device_new_from_syspath(udev, syspath);
+if (udevice == NULL) {
+g_debug("failed to query udev for path: %s", syspath);
+goto out;
+} else {
+ret = g_strdup(udev_device_get_devnode(udevice));
+}
+
+out:
+udev_unref(udev);
+udev_device_unref(udevice);
+
[PATCH 0/3] tests/qtest: npcm7xx test fixes
This series contains a fix for the randomness calculation in npcm7xx_rng-test. It also makes test failures fatal. The last patch would have dumped the random data to stderr if the randomness test fails, except now that failures are fatal, it never actually gets a chance to do that. It may not make sense to apply all three, but I'd definitely take (1), and I'll leave it up to you whether to apply (2), (3) or both. Havard Skinnemoen (3): tests/qtest/npcm7xx_rng-test: count runs properly tests/qtest/npcm7xx: Don't call g_test_set_nonfatal_assertions tests/qtest/npcm7xx_rng-test: dump random data on failure tests/qtest/npcm7xx_gpio-test.c | 1 - tests/qtest/npcm7xx_rng-test.c| 15 +-- tests/qtest/npcm7xx_timer-test.c | 1 - tests/qtest/npcm7xx_watchdog_timer-test.c | 1 - 4 files changed, 13 insertions(+), 5 deletions(-) -- 2.29.1.341.ge80a0c044ae-goog
[PULL v2 12/12] qga: add ssh-get-authorized-keys
From: Marc-André Lureau
Signed-off-by: Marc-André Lureau
*fix-up merge conflicts due to qga-ssh-test being disabled in earlier
patch due to G_TEST_OPTION_ISOLATE_DIRS triggering build-oss-fuzz
leak detector.
Signed-off-by: Michael Roth
---
qga/commands-posix-ssh.c | 66
qga/meson.build | 11 +--
qga/qapi-schema.json | 31 +++
3 files changed, 106 insertions(+), 2 deletions(-)
diff --git a/qga/commands-posix-ssh.c b/qga/commands-posix-ssh.c
index f974bc4b64..4d75cb0113 100644
--- a/qga/commands-posix-ssh.c
+++ b/qga/commands-posix-ssh.c
@@ -268,6 +268,46 @@ qmp_guest_ssh_remove_authorized_keys(const char *username,
strList *keys,
write_authkeys(authkeys_path, new_keys, p, errp);
}
+GuestAuthorizedKeys *
+qmp_guest_ssh_get_authorized_keys(const char *username, Error **errp)
+{
+g_autofree struct passwd *p = NULL;
+g_autofree char *authkeys_path = NULL;
+g_auto(GStrv) authkeys = NULL;
+g_autoptr(GuestAuthorizedKeys) ret = NULL;
+int i;
+
+ERRP_GUARD();
+
+p = get_passwd_entry(username, errp);
+if (p == NULL) {
+return NULL;
+}
+
+authkeys_path = g_build_filename(p->pw_dir, ".ssh",
+ "authorized_keys", NULL);
+authkeys = read_authkeys(authkeys_path, errp);
+if (authkeys == NULL) {
+return NULL;
+}
+
+ret = g_new0(GuestAuthorizedKeys, 1);
+for (i = 0; authkeys[i] != NULL; i++) {
+strList *new;
+
+g_strstrip(authkeys[i]);
+if (!authkeys[i][0] || authkeys[i][0] == '#') {
+continue;
+}
+
+new = g_new0(strList, 1);
+new->value = g_strdup(authkeys[i]);
+new->next = ret->keys;
+ret->keys = new;
+}
+
+return g_steal_pointer ();
+}
#ifdef QGA_BUILD_UNIT_TEST
#if GLIB_CHECK_VERSION(2, 60, 0)
@@ -426,6 +466,31 @@ test_remove_keys(void)
"algo some-key another\n");
}
+static void
+test_get_keys(void)
+{
+Error *err = NULL;
+static const char *authkeys =
+"algo key1 comments\n"
+"# a commented line\n"
+"algo some-key another\n";
+g_autoptr(GuestAuthorizedKeys) ret = NULL;
+strList *k;
+size_t len = 0;
+
+test_authorized_keys_set(authkeys);
+
+ret = qmp_guest_ssh_get_authorized_keys(g_get_user_name(), );
+g_assert_null(err);
+
+for (len = 0, k = ret->keys; k != NULL; k = k->next) {
+g_assert(g_str_has_prefix(k->value, "algo "));
+len++;
+}
+
+g_assert_cmpint(len, ==, 2);
+}
+
int main(int argc, char *argv[])
{
setlocale(LC_ALL, "");
@@ -437,6 +502,7 @@ int main(int argc, char *argv[])
g_test_add_func("/qga/ssh/add_keys", test_add_keys);
g_test_add_func("/qga/ssh/add_reset_keys", test_add_reset_keys);
g_test_add_func("/qga/ssh/remove_keys", test_remove_keys);
+g_test_add_func("/qga/ssh/get_keys", test_get_keys);
return g_test_run();
}
diff --git a/qga/meson.build b/qga/meson.build
index 4cb3b3f259..53ba6de5f8 100644
--- a/qga/meson.build
+++ b/qga/meson.build
@@ -95,8 +95,15 @@ test_env.set('G_TEST_BUILDDIR', meson.current_build_dir())
# issue is identified/fix
#if 'CONFIG_POSIX' in config_host
if false
- qga_ssh_test = executable('qga-ssh-test',
-files('commands-posix-ssh.c'),
+ srcs = [files('commands-posix-ssh.c')]
+ i = 0
+ foreach output: qga_qapi_outputs
+if output.startswith('qga-qapi-types') or
output.startswith('qga-qapi-visit')
+ srcs += qga_qapi_files[i]
+endif
+i = i + 1
+ endforeach
+ qga_ssh_test = executable('qga-ssh-test', srcs,
dependencies: [qemuutil],
c_args: ['-DQGA_BUILD_UNIT_TEST'])
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 4ddea898fa..6ca85f995f 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -1347,6 +1347,37 @@
{ 'command': 'guest-get-devices',
'returns': ['GuestDeviceInfo'] }
+##
+# @GuestAuthorizedKeys:
+#
+# @keys: public keys (in OpenSSH/sshd(8) authorized_keys format)
+#
+# Since: 5.2
+##
+{ 'struct': 'GuestAuthorizedKeys',
+ 'data': {
+ 'keys': ['str']
+ },
+ 'if': 'defined(CONFIG_POSIX)' }
+
+
+##
+# @guest-ssh-get-authorized-keys:
+#
+# @username: the user account to add the authorized keys
+#
+# Return the public keys from user .ssh/authorized_keys on Unix systems (not
+# implemented for other systems).
+#
+# Returns: @GuestAuthorizedKeys
+#
+# Since: 5.2
+##
+{ 'command': 'guest-ssh-get-authorized-keys',
+ 'data': { 'username': 'str' },
+ 'returns': 'GuestAuthorizedKeys',
+ 'if': 'defined(CONFIG_POSIX)' }
+
##
# @guest-ssh-add-authorized-keys:
#
--
2.25.1
[PATCH 1/3] tests/qtest/npcm7xx_rng-test: count runs properly
The number of runs is equal to the number of 0-1 and 1-0 transitions,
plus one. Currently, it's counting the number of times these transitions
do _not_ happen, plus one.
Source:
https://nvlpubs.nist.gov/nistpubs/Legacy/SP/nistspecialpublication800-22r1a.pdf
section 2.3.4 point (3).
Signed-off-by: Havard Skinnemoen
---
tests/qtest/npcm7xx_rng-test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/qtest/npcm7xx_rng-test.c b/tests/qtest/npcm7xx_rng-test.c
index da6e639bf6..57787c5ffc 100644
--- a/tests/qtest/npcm7xx_rng-test.c
+++ b/tests/qtest/npcm7xx_rng-test.c
@@ -126,7 +126,7 @@ static double calc_runs_p(const unsigned long *buf,
unsigned int nr_bits)
pi = (double)nr_ones / nr_bits;
for (k = 0; k < nr_bits - 1; k++) {
-vn_obs += !(test_bit(k, buf) ^ test_bit(k + 1, buf));
+vn_obs += (test_bit(k, buf) ^ test_bit(k + 1, buf));
}
vn_obs += 1;
--
2.29.1.341.ge80a0c044ae-goog
[PULL v2 08/12] glib-compat: add g_unix_get_passwd_entry_qemu()
From: Marc-André Lureau
The glib function was introduced in 2.64. It's a safer version of
getpwnam, and also simpler to use than getpwnam_r.
Currently, it's only use by the next patch in qemu-ga, which doesn't
(well well...) need the thread safety guarantees. Since the fallback
version is still unsafe, I would rather keep the _qemu postfix, to make
sure it's not being misused by mistake. When/if necessary, we can
implement a safer fallback and drop the _qemu suffix.
Signed-off-by: Marc-André Lureau
Reviewed-by: Michal Privoznik
Signed-off-by: Michael Roth
---
include/glib-compat.h | 26 ++
1 file changed, 26 insertions(+)
diff --git a/include/glib-compat.h b/include/glib-compat.h
index 0b0ec76299..64e68aa730 100644
--- a/include/glib-compat.h
+++ b/include/glib-compat.h
@@ -30,6 +30,11 @@
#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
#include
+#if defined(G_OS_UNIX)
+#include
+#include
+#include
+#endif
/*
* Note that because of the GLIB_VERSION_MAX_ALLOWED constant above, allowing
@@ -72,6 +77,27 @@
gint g_poll_fixed(GPollFD *fds, guint nfds, gint timeout);
#endif
+#if defined(G_OS_UNIX)
+/* Note: The fallback implementation is not MT-safe, and it returns a copy of
+ * the libc passwd (must be g_free() after use) but not the content. Because of
+ * these important differences the caller must be aware of, it's not #define
for
+ * GLib API substitution. */
+static inline struct passwd *
+g_unix_get_passwd_entry_qemu(const gchar *user_name, GError **error)
+{
+#if GLIB_CHECK_VERSION(2, 64, 0)
+return g_unix_get_passwd_entry(user_name, error);
+#else
+struct passwd *p = getpwnam(user_name);
+if (!p) {
+g_set_error_literal(error, G_UNIX_ERROR, 0, g_strerror(errno));
+return NULL;
+}
+return (struct passwd *)g_memdup(p, sizeof(*p));
+#endif
+}
+#endif /* G_OS_UNIX */
+
#pragma GCC diagnostic pop
#endif
--
2.25.1
[PULL v2 03/12] qga-win: Fix guest-get-devices error API violations
From: Markus Armbruster
The Error ** argument must be NULL, _abort, _fatal, or a
pointer to a variable containing NULL. Passing an argument of the
latter kind twice without clearing it in between is wrong: if the
first call sets an error, it no longer points to NULL for the second
call.
qmp_guest_get_devices() is wrong that way: it calls error_setg() in a
loop.
If no iteration fails, the function returns a value and sets no error.
Okay.
If exactly one iteration fails, the function returns a value and sets
an error. Wrong.
If multiple iterations fail, the function trips error_setv()'s
assertion.
Fix it to return immediately on error.
Perhaps the failure to convert the driver version to UTF-8 should not
be an error. We could simply not report the botched version string
instead.
Drop a superfluous continue while there.
Signed-off-by: Markus Armbruster
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Marc-André Lureau
Signed-off-by: Michael Roth
---
qga/commands-win32.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index b01616a992..1efe3ba076 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -2385,7 +2385,7 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
device->driver_name = g_utf16_to_utf8(name, -1, NULL, NULL, NULL);
if (device->driver_name == NULL) {
error_setg(errp, "conversion to utf8 failed (driver name)");
-continue;
+return NULL;
}
slog("querying device: %s", device->driver_name);
hw_ids = ga_get_hardware_ids(dev_info_data.DevInst);
@@ -2428,7 +2428,7 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
NULL, NULL);
if (device->driver_version == NULL) {
error_setg(errp, "conversion to utf8 failed (driver version)");
-continue;
+return NULL;
}
device->has_driver_version = true;
@@ -2452,7 +2452,6 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
cur_item->next = item;
cur_item = item;
}
-continue;
}
if (dev_info != INVALID_HANDLE_VALUE) {
--
2.25.1
[PULL v2 11/12] meson: minor simplification
From: Marc-André Lureau
Signed-off-by: Marc-André Lureau
Signed-off-by: Michael Roth
---
qga/meson.build | 7 +--
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/qga/meson.build b/qga/meson.build
index 635de9af41..4cb3b3f259 100644
--- a/qga/meson.build
+++ b/qga/meson.build
@@ -22,12 +22,7 @@ qga_qapi_files = custom_target('QGA QAPI files',
depend_files: qapi_gen_depends)
qga_ss = ss.source_set()
-i = 0
-foreach output: qga_qapi_outputs
- qga_ss.add(qga_qapi_files[i])
- i = i + 1
-endforeach
-
+qga_ss.add(qga_qapi_files.to_list())
qga_ss.add(files(
'commands.c',
'guest-agent-command-state.c',
--
2.25.1
[PULL v2 04/12] qga: Flatten simple union GuestDeviceId
From: Markus Armbruster
Simple unions are simpler than flat unions in the schema, but more
complicated in C and on the QMP wire: there's extra indirection in C
and extra nesting on the wire, both pointless. They should be avoided
in new code.
GuestDeviceId was recently added for guest-get-devices. Convert it to
a flat union.
Signed-off-by: Markus Armbruster
Reviewed-by: Marc-André Lureau
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Michael Roth
---
qga/commands-win32.c | 9 -
qga/qapi-schema.json | 8
2 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 1efe3ba076..0c33d48aaa 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -2400,16 +2400,15 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
}
skip = false;
-id = g_new0(GuestDeviceIdPCI, 1);
vendor_id = g_match_info_fetch(match_info, 1);
device_id = g_match_info_fetch(match_info, 2);
-id->vendor_id = g_ascii_strtoull(vendor_id, NULL, 16);
-id->device_id = g_ascii_strtoull(device_id, NULL, 16);
device->id = g_new0(GuestDeviceId, 1);
device->has_id = true;
-device->id->type = GUEST_DEVICE_ID_KIND_PCI;
-device->id->u.pci.data = id;
+device->id->type = GUEST_DEVICE_TYPE_PCI;
+id = >id->u.pci;
+id->vendor_id = g_ascii_strtoull(vendor_id, NULL, 16);
+id->device_id = g_ascii_strtoull(device_id, NULL, 16);
g_match_info_free(match_info);
break;
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index c7bfb8bf6a..fe10631e4c 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -1256,6 +1256,12 @@
{ 'command': 'guest-get-osinfo',
'returns': 'GuestOSInfo' }
+##
+# @GuestDeviceType:
+##
+{ 'enum': 'GuestDeviceType',
+ 'data': [ 'pci' ] }
+
##
# @GuestDeviceIdPCI:
#
@@ -1276,6 +1282,8 @@
# Since: 5.2
##
{ 'union': 'GuestDeviceId',
+ 'base': { 'type': 'GuestDeviceType' },
+ 'discriminator': 'type',
'data': { 'pci': 'GuestDeviceIdPCI' } }
##
--
2.25.1
[PULL v2 00/12] qemu-ga patch queue for soft-freeze
Hi Peter,
Sorry to get these out so late, for some inexplicable reason my email
client decided to flag all responses v1 as spam and I didn't notice
until I double-checked the archives this morning.
I've fixed the gcc-on-BSD and clang-on-linux issues you pointed out
(PATCH 6) and added proper test coverage for both.
Also, the qga-ssh-test unit test introduced in this series triggers a
failure in Gitlab CI build-oss-fuzz test. This seems to be due to a
memory leak in GLib itself when G_TEST_OPTION_ISOLATE_DIRS option is
passed to g_test_init(). I verified the unit test doesn't introduce any
leaks when run without g_test* harness. Since G_TEST_OPTION_ISOLATE_DIRS
is currently needed to safely run the qga-ssh-test, I've disabled it for
now (PATCH 9 and 12) until we figure out solution.
Thanks,
Mike
The following changes since commit 2c6605389c1f76973d92b69b85d40d94b8f1092c:
Merge remote-tracking branch 'remotes/awilliam/tags/vfio-update-20201101.0'
into staging (2020-11-02 09:54:00 +)
are available in the Git repository at:
git://github.com/mdroth/qemu.git tags/qga-pull-2020-10-27-v2-tag
for you to fetch changes up to b457991dfb801bf9227e8823534de5dbb3c276c1:
qga: add ssh-get-authorized-keys (2020-11-02 18:30:42 -0600)
qemu-ga patch queue for soft-freeze
* add guest-get-disks for w32/linux
* add guest-{add,remove,get}-authorized-keys
* fix API violations and schema documentation inconsistencies with
recently-added guest-get-devices
v2:
- fix BSD build error due to missing stub for guest_get_disks
- fix clang build error on linux due to unused variable
- disable qga-ssh-test for now due to a memory leak within GLib when
G_TEST_OPTION_ISOLATE_DIRS is passed to g_test_init() since it
break Gitlab CI build-oss-fuzz test
- rebased and re-tested on master
Marc-André Lureau (5):
glib-compat: add g_unix_get_passwd_entry_qemu()
qga: add ssh-{add,remove}-authorized-keys
qga: add *reset argument to ssh-add-authorized-keys
meson: minor simplification
qga: add ssh-get-authorized-keys
Markus Armbruster (4):
qga: Rename guest-get-devices return member 'address' to 'id'
qga: Use common time encoding for guest-get-devices 'driver-date'
qga-win: Fix guest-get-devices error API violations
qga: Flatten simple union GuestDeviceId
Tomáš Golembiovský (3):
qga: add command guest-get-disks
qga: add implementation of guest-get-disks for Linux
qga: add implementation of guest-get-disks for Windows
include/glib-compat.h| 26 +++
qga/commands-posix-ssh.c | 516 +++
qga/commands-posix.c | 297 ++-
qga/commands-win32.c | 140 +++--
qga/meson.build | 39 +++-
qga/qapi-schema.json | 127 +++-
6 files changed, 1104 insertions(+), 41 deletions(-)
create mode 100644 qga/commands-posix-ssh.c
[PULL v2 07/12] qga: add implementation of guest-get-disks for Windows
From: Tomáš Golembiovský
The command lists all the physical disk drives. Unlike for Linux
partitions and virtual volumes are not listed.
Example output:
{
"return": [
{
"name": ".\\PhysicalDrive0",
"partition": false,
"address": {
"serial": "QM1",
"bus-type": "sata",
...
},
"dependents": []
}
]
}
Signed-off-by: Tomáš Golembiovský
Signed-off-by: Michael Roth
---
qga/commands-win32.c | 107 ---
1 file changed, 101 insertions(+), 6 deletions(-)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index f7bdd5a8b5..300b87c859 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -979,6 +979,101 @@ out:
return list;
}
+GuestDiskInfoList *qmp_guest_get_disks(Error **errp)
+{
+ERRP_GUARD();
+GuestDiskInfoList *new = NULL, *ret = NULL;
+HDEVINFO dev_info;
+SP_DEVICE_INTERFACE_DATA dev_iface_data;
+int i;
+
+dev_info = SetupDiGetClassDevs(_DEVINTERFACE_DISK, 0, 0,
+DIGCF_PRESENT | DIGCF_DEVICEINTERFACE);
+if (dev_info == INVALID_HANDLE_VALUE) {
+error_setg_win32(errp, GetLastError(), "failed to get device tree");
+return NULL;
+}
+
+g_debug("enumerating devices");
+dev_iface_data.cbSize = sizeof(SP_DEVICE_INTERFACE_DATA);
+for (i = 0;
+SetupDiEnumDeviceInterfaces(dev_info, NULL, _DEVINTERFACE_DISK,
+i, _iface_data);
+i++) {
+GuestDiskAddress *address = NULL;
+GuestDiskInfo *disk = NULL;
+Error *local_err = NULL;
+g_autofree PSP_DEVICE_INTERFACE_DETAIL_DATA
+pdev_iface_detail_data = NULL;
+STORAGE_DEVICE_NUMBER sdn;
+HANDLE dev_file;
+DWORD size = 0;
+BOOL result;
+int attempt;
+
+g_debug(" getting device path");
+for (attempt = 0, result = FALSE; attempt < 2 && !result; attempt++) {
+result = SetupDiGetDeviceInterfaceDetail(dev_info,
+_iface_data, pdev_iface_detail_data, size, , NULL);
+if (result) {
+break;
+}
+if (GetLastError() == ERROR_INSUFFICIENT_BUFFER) {
+pdev_iface_detail_data = g_realloc(pdev_iface_detail_data,
+size);
+pdev_iface_detail_data->cbSize =
+sizeof(*pdev_iface_detail_data);
+} else {
+g_debug("failed to get device interface details");
+break;
+}
+}
+if (!result) {
+g_debug("skipping device");
+continue;
+}
+
+g_debug(" device: %s", pdev_iface_detail_data->DevicePath);
+dev_file = CreateFile(pdev_iface_detail_data->DevicePath, 0,
+FILE_SHARE_READ, NULL, OPEN_EXISTING, 0, NULL);
+if (!DeviceIoControl(dev_file, IOCTL_STORAGE_GET_DEVICE_NUMBER,
+NULL, 0, , sizeof(sdn), , NULL)) {
+CloseHandle(dev_file);
+debug_error("failed to get storage device number");
+continue;
+}
+CloseHandle(dev_file);
+
+disk = g_new0(GuestDiskInfo, 1);
+disk->name = g_strdup_printf(".\\PhysicalDrive%lu",
+sdn.DeviceNumber);
+
+g_debug(" number: %lu", sdn.DeviceNumber);
+address = g_malloc0(sizeof(GuestDiskAddress));
+address->has_dev = true;
+address->dev = g_strdup(disk->name);
+get_single_disk_info(sdn.DeviceNumber, address, _err);
+if (local_err) {
+g_debug("failed to get disk info: %s",
+error_get_pretty(local_err));
+error_free(local_err);
+qapi_free_GuestDiskAddress(address);
+address = NULL;
+} else {
+disk->address = address;
+disk->has_address = true;
+}
+
+new = g_malloc0(sizeof(GuestDiskInfoList));
+new->value = disk;
+new->next = ret;
+ret = new;
+}
+
+SetupDiDestroyDeviceInfoList(dev_info);
+return ret;
+}
+
#else
static GuestDiskAddressList *build_guest_disk_info(char *guid, Error **errp)
@@ -986,6 +1081,12 @@ static GuestDiskAddressList *build_guest_disk_info(char
*guid, Error **errp)
return NULL;
}
+GuestDiskInfoList *qmp_guest_get_disks(Error **errp)
+{
+error_setg(errp, QERR_UNSUPPORTED);
+return NULL;
+}
+
#endif /* CONFIG_QGA_NTDDSCSI */
static GuestFilesystemInfo *build_guest_fsinfo(char *guid, Error **errp)
@@ -2458,9 +2559,3 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
}
return head;
}
-
-GuestDiskInfoList *qmp_guest_get_disks(Error **errp)
-{
-error_setg(errp, QERR_UNSUPPORTED);
-return NULL;
-}
--
2.25.1
[PULL v2 02/12] qga: Use common time encoding for guest-get-devices 'driver-date'
From: Markus Armbruster
guest-get-devices returns 'driver-date' as string in the format
-MM-DD. Goes back to recent commit 2e4211cee4 "qga: add command
guest-get-devices for reporting VirtIO devices".
We should avoid use of multiple encodings for the same kind of data.
Especially string encodings. Change it to return nanoseconds since
the epoch, like guest-get-time does.
Signed-off-by: Markus Armbruster
Reviewed-by: Marc-André Lureau
Signed-off-by: Michael Roth
---
qga/commands-win32.c | 19 +++
qga/qapi-schema.json | 4 ++--
2 files changed, 13 insertions(+), 10 deletions(-)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 879b02b6c3..b01616a992 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -1641,6 +1641,12 @@ out:
return head;
}
+static int64_t filetime_to_ns(const FILETIME *tf)
+{
+return int64_t)tf->dwHighDateTime << 32) | tf->dwLowDateTime)
+- W32_FT_OFFSET) * 100;
+}
+
int64_t qmp_guest_get_time(Error **errp)
{
SYSTEMTIME ts = {0};
@@ -1657,8 +1663,7 @@ int64_t qmp_guest_get_time(Error **errp)
return -1;
}
-return int64_t)tf.dwHighDateTime << 32) | tf.dwLowDateTime)
-- W32_FT_OFFSET) * 100;
+return filetime_to_ns();
}
void qmp_guest_set_time(bool has_time, int64_t time_ns, Error **errp)
@@ -2363,7 +2368,6 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
slog("enumerating devices");
for (i = 0; SetupDiEnumDeviceInfo(dev_info, i, _info_data); i++) {
bool skip = true;
-SYSTEMTIME utc_date;
g_autofree LPWSTR name = NULL;
g_autofree LPFILETIME date = NULL;
g_autofree LPWSTR version = NULL;
@@ -2434,13 +2438,12 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
slog("failed to get driver date");
continue;
}
-FileTimeToSystemTime(date, _date);
-device->driver_date = g_strdup_printf("%04d-%02d-%02d",
-utc_date.wYear, utc_date.wMonth, utc_date.wDay);
+device->driver_date = filetime_to_ns(date);
device->has_driver_date = true;
-slog("driver: %s\ndriver version: %s,%s\n", device->driver_name,
-device->driver_date, device->driver_version);
+slog("driver: %s\ndriver version: %" PRId64 ",%s\n",
+ device->driver_name, device->driver_date,
+ device->driver_version);
item = g_new0(GuestDeviceInfoList, 1);
item->value = g_steal_pointer();
if (!cur_item) {
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index f2c81cda2b..c7bfb8bf6a 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -1282,7 +1282,7 @@
# @GuestDeviceInfo:
#
# @driver-name: name of the associated driver
-# @driver-date: driver release date in format -MM-DD
+# @driver-date: driver release date, in nanoseconds since the epoch
# @driver-version: driver version
# @id: device ID
#
@@ -1291,7 +1291,7 @@
{ 'struct': 'GuestDeviceInfo',
'data': {
'driver-name': 'str',
- '*driver-date': 'str',
+ '*driver-date': 'int',
'*driver-version': 'str',
'*id': 'GuestDeviceId'
} }
--
2.25.1
[PULL v2 10/12] qga: add *reset argument to ssh-add-authorized-keys
From: Marc-André Lureau
I prefer 'reset' over 'clear', since 'clear' and keys may have some
other relations or meaning.
Signed-off-by: Marc-André Lureau
Signed-off-by: Michael Roth
---
qga/commands-posix-ssh.c | 53
qga/qapi-schema.json | 3 ++-
2 files changed, 50 insertions(+), 6 deletions(-)
diff --git a/qga/commands-posix-ssh.c b/qga/commands-posix-ssh.c
index a7bc9a1c24..f974bc4b64 100644
--- a/qga/commands-posix-ssh.c
+++ b/qga/commands-posix-ssh.c
@@ -168,6 +168,7 @@ read_authkeys(const char *path, Error **errp)
void
qmp_guest_ssh_add_authorized_keys(const char *username, strList *keys,
+ bool has_reset, bool reset,
Error **errp)
{
g_autofree struct passwd *p = NULL;
@@ -178,6 +179,7 @@ qmp_guest_ssh_add_authorized_keys(const char *username,
strList *keys,
size_t nkeys, nauthkeys;
ERRP_GUARD();
+reset = has_reset && reset;
if (!check_openssh_pub_keys(keys, , errp)) {
return;
@@ -191,7 +193,9 @@ qmp_guest_ssh_add_authorized_keys(const char *username,
strList *keys,
ssh_path = g_build_filename(p->pw_dir, ".ssh", NULL);
authkeys_path = g_build_filename(ssh_path, "authorized_keys", NULL);
-authkeys = read_authkeys(authkeys_path, NULL);
+if (!reset) {
+authkeys = read_authkeys(authkeys_path, NULL);
+}
if (authkeys == NULL) {
if (!g_file_test(ssh_path, G_FILE_TEST_IS_DIR) &&
!mkdir_for_user(ssh_path, p, 0700, errp)) {
@@ -318,7 +322,7 @@ test_invalid_user(void)
{
Error *err = NULL;
-qmp_guest_ssh_add_authorized_keys("", NULL, );
+qmp_guest_ssh_add_authorized_keys("", NULL, FALSE, FALSE, );
error_free_or_abort();
qmp_guest_ssh_remove_authorized_keys("", NULL, );
@@ -333,7 +337,8 @@ test_invalid_key(void)
};
Error *err = NULL;
-qmp_guest_ssh_add_authorized_keys(g_get_user_name(), , );
+qmp_guest_ssh_add_authorized_keys(g_get_user_name(), ,
+ FALSE, FALSE, );
error_free_or_abort();
qmp_guest_ssh_remove_authorized_keys(g_get_user_name(), , );
@@ -346,13 +351,17 @@ test_add_keys(void)
Error *err = NULL;
qmp_guest_ssh_add_authorized_keys(g_get_user_name(),
- (strList *)_key2, );
+ (strList *)_key2,
+ FALSE, FALSE,
+ );
g_assert_null(err);
test_authorized_keys_equal("algo key2 comments");
qmp_guest_ssh_add_authorized_keys(g_get_user_name(),
- (strList *)_key1_2, );
+ (strList *)_key1_2,
+ FALSE, FALSE,
+ );
g_assert_null(err);
/* key2 came first, and should'nt be duplicated */
@@ -360,6 +369,39 @@ test_add_keys(void)
"algo key1 comments");
}
+static void
+test_add_reset_keys(void)
+{
+Error *err = NULL;
+
+qmp_guest_ssh_add_authorized_keys(g_get_user_name(),
+ (strList *)_key1_2,
+ FALSE, FALSE,
+ );
+g_assert_null(err);
+
+/* reset with key2 only */
+test_authorized_keys_equal("algo key1 comments\n"
+ "algo key2 comments");
+
+qmp_guest_ssh_add_authorized_keys(g_get_user_name(),
+ (strList *)_key2,
+ TRUE, TRUE,
+ );
+g_assert_null(err);
+
+test_authorized_keys_equal("algo key2 comments");
+
+/* empty should clear file */
+qmp_guest_ssh_add_authorized_keys(g_get_user_name(),
+ (strList *)NULL,
+ TRUE, TRUE,
+ );
+g_assert_null(err);
+
+test_authorized_keys_equal("");
+}
+
static void
test_remove_keys(void)
{
@@ -393,6 +435,7 @@ int main(int argc, char *argv[])
g_test_add_func("/qga/ssh/invalid_user", test_invalid_user);
g_test_add_func("/qga/ssh/invalid_key", test_invalid_key);
g_test_add_func("/qga/ssh/add_keys", test_add_keys);
+g_test_add_func("/qga/ssh/add_reset_keys", test_add_reset_keys);
g_test_add_func("/qga/ssh/remove_keys", test_remove_keys);
return g_test_run();
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index a2727ed86b..4ddea898fa 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -1352,6 +1352,7 @@
#
# @username: the user account to add the authorized keys
# @keys: the public keys to add (in OpenSSH/sshd(8) authorized_keys format)
+# @reset: ignore the existing content, set it with the given keys only
#
# Append public keys to user
Re: [PATCH-for-5.2] target/mips: Deprecate nanoMIPS ISA
On 11/2/20 12:27 PM, Philippe Mathieu-Daudé wrote: > The nanoMIPS ISA has been announced in 2018 for various projects: > > GCC: https://gcc.gnu.org/legacy-ml/gcc/2018-05/msg00012.html > Linux: https://lwn.net/Articles/753605/ > QEMU: https://www.mail-archive.com/qemu-devel@nongnu.org/msg530721.html > > Unfortunately the links referenced doesn't work anymore (www.mips.com). > > From this Wayback machine link [1] we can get to a working place to > download a toolchain (a more recent release than the one referenced > in the announcement mails): > http://codescape.mips.com/components/toolchain/nanomips/2018.04-02/downloads.html > > The toolchain page mention LLVM but simply links http://llvm.org/ > where there is no reference on nanoMIPS. > > The only reference in the GCC mailing list, is the nanoMIPS > announcement: https://gcc.gnu.org/pipermail/gcc/2018-May.txt > > The developer who authored the announcements have been emailed [2] > to ask for more information but all their emails are now bouncing: > > - Your message to stefan.marko...@mips.com couldn't be delivered. > > - Your message to smarko...@wavecomp.com couldn't be delivered. > > - Couldn't deliver the message to the following recipients: > robert.sucha...@mips.com, matthew.fort...@mips.com, > marcin.nowakow...@mips.com > > Our deprecation policy do not allow feature removal before 2 release, > therefore declare the nanoMIPS ISA code deprecated as of QEMU 5.2. > This gives time to developers to update the QEMU community, or > interested parties to step in to maintain this code. > > [1] > https://web.archive.org/web/20180904044530/https://www.mips.com/develop/tools/compilers/ > [2] https://www.mail-archive.com/qemu-devel@nongnu.org/msg756392.html > > Signed-off-by: Philippe Mathieu-Daudé > --- Acked-by: Richard Henderson r~
Re: [PATCH v11 09/10] virtio-iommu: Set supported page size mask
On Fri, Oct 30, 2020 at 07:05:09PM +0100, Jean-Philippe Brucker wrote:
> From: Bharat Bhushan
>
> The virtio-iommu device can deal with arbitrary page sizes for virtual
> endpoints, but for endpoints assigned with VFIO it must follow the page
> granule used by the host IOMMU driver.
>
> Implement the interface to set the vIOMMU page size mask, called by VFIO
> for each endpoint. We assume that all host IOMMU drivers use the same
> page granule (the host page granule). Override the page_size_mask field
> in the virtio config space.
(Nit: Seems slightly mismatched with the code below)
[...]
> +/*
> + * After the machine is finalized, we can't change the mask anymore. If
> by
> + * chance the hotplugged device supports the same granule, we can still
> + * accept it. Having a different masks is possible but the guest will use
> + * sub-optimal block sizes, so warn about it.
> + */
> +if (qdev_hotplug) {
> +int new_granule = ctz64(new_mask);
> +int cur_granule = ctz64(cur_mask);
> +
> +if (new_granule != cur_granule) {
> +error_setg(errp, "virtio-iommu page mask 0x%"PRIx64
> + " is incompatible with mask 0x%"PRIx64, cur_mask,
> + new_mask);
> +return -1;
> +} else if (new_mask != cur_mask) {
> +warn_report("virtio-iommu page mask 0x%"PRIx64
> +" does not match 0x%"PRIx64, cur_mask, new_mask);
IMHO, new_mask!=cur_mask is ok, as long as it's a superset of reported
cur_mask, then it'll still guarantee to work.
Meanwhile, checking against granule seems not safe enough if the guest driver
started to use huge pages in iommu pgtables...
In summary:
if (qdev_hotplug) {
if ((new_mask & cur_mask) == cur_mask) {
/* Superset of old mask; we're good. Keep the old mask since same
*/
return 0;
} else {
/* Guest driver can use any psize in cur_mask, not safe to continue
*/
error_setg(...);
return -1;
}
}
Maybe we can also work on top too (if this is the only reason to repost,
especially if Michael would like to pick this up sooner), so I just raise this
up.
Thanks,
--
Peter Xu
Re: Does QEMU's coverity-scan run need to track coverity issues in dtb or slirp ?
On Mon, 2 Nov 2020 at 20:37, Samuel Thibault wrote: > > Hello, > > Peter Maydell, le lun. 02 nov. 2020 19:54:14 +, a ecrit: > > Do dtc and slirp as upstream projects already track Coverity issues > > We started tracking them yes. OK, so I can just dismiss anything that comes up in slirp/ in QEMU's scan; that will help to cut down on the recent buildup. thanks -- PMM
Re: [PATCH-for-5.2] hw/virtio/vhost-backend: Fix Coverity CID 1432871
On Mon, 2 Nov 2020 at 20:36, Philippe Mathieu-Daudé wrote:
>
> Fix uninitialized value issues reported by Coverity:
>
> Field 'msg.reserved' is uninitialized when calling write().
>
> Reported-by: Coverity (CID 1432864: UNINIT)
> Fixes: c471ad0e9bd ("vhost_net: device IOTLB support")
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> hw/virtio/vhost-backend.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c
> index 88c8ecc9e03..222bbcc62de 100644
> --- a/hw/virtio/vhost-backend.c
> +++ b/hw/virtio/vhost-backend.c
> @@ -257,7 +257,7 @@ static int vhost_kernel_send_device_iotlb_msg(struct
> vhost_dev *dev,
>struct vhost_iotlb_msg *imsg)
> {
> if (dev->backend_cap & (1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2)) {
> -struct vhost_msg_v2 msg;
> +struct vhost_msg_v2 msg = {};
>
> msg.type = VHOST_IOTLB_MSG_V2;
> msg.iotlb = *imsg;
> @@ -267,7 +267,7 @@ static int vhost_kernel_send_device_iotlb_msg(struct
> vhost_dev *dev,
> return -EFAULT;
> }
> } else {
> -struct vhost_msg msg;
> +struct vhost_msg msg = {};
>
> msg.type = VHOST_IOTLB_MSG;
> msg.iotlb = *imsg;
Coverity didn't mind about the struct vhost_msg because
it doesn't have a 'reserved' field like vhost_msg_v2; but it
doesn't hurt to initialize it and it makes the two parts of
the function consistent, which is good.
Reviewed-by: Peter Maydell
thanks
-- PMM
Re: Does QEMU's coverity-scan run need to track coverity issues in dtb or slirp ?
Hello, Peter Maydell, le lun. 02 nov. 2020 19:54:14 +, a ecrit: > Do dtc and slirp as upstream projects already track Coverity issues We started tracking them yes. Samuel
[PATCH-for-5.2] hw/virtio/vhost-backend: Fix Coverity CID 1432871
Fix uninitialized value issues reported by Coverity:
Field 'msg.reserved' is uninitialized when calling write().
Reported-by: Coverity (CID 1432864: UNINIT)
Fixes: c471ad0e9bd ("vhost_net: device IOTLB support")
Signed-off-by: Philippe Mathieu-Daudé
---
hw/virtio/vhost-backend.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c
index 88c8ecc9e03..222bbcc62de 100644
--- a/hw/virtio/vhost-backend.c
+++ b/hw/virtio/vhost-backend.c
@@ -257,7 +257,7 @@ static int vhost_kernel_send_device_iotlb_msg(struct
vhost_dev *dev,
struct vhost_iotlb_msg *imsg)
{
if (dev->backend_cap & (1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2)) {
-struct vhost_msg_v2 msg;
+struct vhost_msg_v2 msg = {};
msg.type = VHOST_IOTLB_MSG_V2;
msg.iotlb = *imsg;
@@ -267,7 +267,7 @@ static int vhost_kernel_send_device_iotlb_msg(struct
vhost_dev *dev,
return -EFAULT;
}
} else {
-struct vhost_msg msg;
+struct vhost_msg msg = {};
msg.type = VHOST_IOTLB_MSG;
msg.iotlb = *imsg;
--
2.26.2
Re: [PULL] nvme emulation patches for 5.2
On Mon, 2 Nov 2020 at 15:27, Keith Busch wrote: > > Hi Peter, > > We are sorting out Klaus' signature authentication this week, but in the > interest of timing, I've signed our pull request for this round. > > The following changes since commit 1dc887329a10903940501b43e8c0cc67af7c06d5: > > Merge remote-tracking branch 'remotes/philmd-gitlab/tags/sd-next-20201026' > into staging (2020-10-26 17:19:26 +) > > are available in the Git repository at: > > git://git.infradead.org/qemu-nvme.git tags/pull-nvme-20201102 > > for you to fetch changes up to 843c8f91a7ad63f8f3e4e564d3f41f3d030ab8a9: > > hw/block/nvme: fix queue identifer validation (2020-10-27 11:29:25 +0100) > > nvme emulation patches for 5.2 > > - lots of cleanups > - add support for scatter/gather lists > - add support for multiple namespaces (adds new nvme-ns device) > - change default pci vendor/device id > - add support for per-namespace smart log > > > nvme pull 2 Nov 2020 > > Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/5.2 for any user-visible changes. -- PMM
[PATCH-for-5.2] target/mips: Deprecate nanoMIPS ISA
The nanoMIPS ISA has been announced in 2018 for various projects: GCC: https://gcc.gnu.org/legacy-ml/gcc/2018-05/msg00012.html Linux: https://lwn.net/Articles/753605/ QEMU: https://www.mail-archive.com/qemu-devel@nongnu.org/msg530721.html Unfortunately the links referenced doesn't work anymore (www.mips.com). >From this Wayback machine link [1] we can get to a working place to download a toolchain (a more recent release than the one referenced in the announcement mails): http://codescape.mips.com/components/toolchain/nanomips/2018.04-02/downloads.html The toolchain page mention LLVM but simply links http://llvm.org/ where there is no reference on nanoMIPS. The only reference in the GCC mailing list, is the nanoMIPS announcement: https://gcc.gnu.org/pipermail/gcc/2018-May.txt The developer who authored the announcements have been emailed [2] to ask for more information but all their emails are now bouncing: - Your message to stefan.marko...@mips.com couldn't be delivered. - Your message to smarko...@wavecomp.com couldn't be delivered. - Couldn't deliver the message to the following recipients: robert.sucha...@mips.com, matthew.fort...@mips.com, marcin.nowakow...@mips.com Our deprecation policy do not allow feature removal before 2 release, therefore declare the nanoMIPS ISA code deprecated as of QEMU 5.2. This gives time to developers to update the QEMU community, or interested parties to step in to maintain this code. [1] https://web.archive.org/web/20180904044530/https://www.mips.com/develop/tools/compilers/ [2] https://www.mail-archive.com/qemu-devel@nongnu.org/msg756392.html Signed-off-by: Philippe Mathieu-Daudé --- docs/system/deprecated.rst | 23 +++ MAINTAINERS| 6 +- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst index 32a0e620dbb..a26af200c73 100644 --- a/docs/system/deprecated.rst +++ b/docs/system/deprecated.rst @@ -310,6 +310,13 @@ to build binaries for it. ``Icelake-Client`` CPU Models are deprecated. Use ``Icelake-Server`` CPU Models instead. +MIPS ``I7200`` CPU Model (since 5.2) + + +The ``I7200`` guest CPU relies on the nanoMIPS ISA, which is deprecated +(the ISA has never been upstreamed to a compiler toolchain). Therefore +this CPU is also deprecated. + System emulator devices --- @@ -413,6 +420,13 @@ The ``ppc64abi32`` architecture has a number of issues which regularly trip up our CI testing and is suspected to be quite broken. For that reason the maintainers strongly suspect no one actually uses it. +MIPS ``I7200`` CPU (since 5.2) +'' + +The ``I7200`` guest CPU relies on the nanoMIPS ISA, which is deprecated +(the ISA has never been upstreamed to a compiler toolchain). Therefore +this CPU is also deprecated. + Related binaries @@ -477,6 +491,15 @@ versions, aliases will point to newer CPU model versions depending on the machine type, so management software must resolve CPU model aliases before starting a virtual machine. +Guest Emulator ISAs +--- + +nanoMIPS ISA + + +The ``nanoMIPS`` ISA has never been upstreamed to any compiler toolchain. +As it is hard to generate binaries for it, declare it deprecated. + Recently removed features = diff --git a/MAINTAINERS b/MAINTAINERS index 2c22bbca5ac..4f701012eea 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -227,7 +227,7 @@ R: Aleksandar Rikalo S: Odd Fixes F: target/mips/ F: default-configs/*mips* -F: disas/*mips* +F: disas/mips.c F: docs/system/cpu-models-mips.rst.inc F: hw/intc/mips_gic.c F: hw/mips/ @@ -240,6 +240,10 @@ F: include/hw/timer/mips_gictimer.h F: tests/tcg/mips/ K: ^Subject:.*(?i)mips +MIPS TCG CPUs (nanoMIPS ISA) +S: Orphan +F: disas/nanomips.* + Moxie TCG CPUs M: Anthony Green S: Maintained -- 2.26.2
Re: [Bug 1902470] Re: migration with TLS-MultiFD is stuck when the dst-libvirtd service restarts
* zhengchuan (zhengch...@huawei.com) wrote: > Anyone who could help this would be appreciated since we have stuck for three > days:( > > IIUC, the client (Src) has sent first hello message to sever(Dst), however > due to something happened while restarted libvirtd, > The messages is lost, and both of them are waiting which leading to hang > forever, but I could find out how for now. If you need to un-break things, I suggest killing the destination might free it; but I'm not sure. An interesting question is if we can make migration-cancel work in this case. Dave > -Original Message- > From: Qemu-devel [mailto:qemu-devel-bounces+zhengchuan=huawei@nongnu.org] > On Behalf Of Yan Jin > Sent: 2020年11月2日 11:12 > To: qemu-devel@nongnu.org > Subject: [Bug 1902470] Re: migration with TLS-MultiFD is stuck when the > dst-libvirtd service restarts > > ** Description changed: > > hi, > > I found that the multi-channel TLS-handshake will be stuck when the dst- > libvirtd restarts, both the src and dst sockets are blocked in recvmsg. > In the meantime, live_migration thread is blocked in > multifd_send_sync_main, so migration cannot be cancelled though src- > libvirt has delivered the QMP command. > > Is there any way to exit migration when the multi-channel TLS-handshake > - is stuck? Does setting TLS handshake timeout function take effect? > + is stuck? Does setting TLS-handshake timeout function take effect? > > The stack trace are as follows: > > =src qemu-system-aar stack=: > #0 0x87d6f28c in recvmsg () from target:/usr/lib64/libpthread.so.0 > #1 0xe3817424 in qio_channel_socket_readv (ioc=0xe9e30a30, > iov=0xdb58e8a8, niov=1, fds=0x0, nfds=0x0, errp=0x0) at > ../io/channel-socket.c:502 > #2 0xe380f468 in qio_channel_readv_full (ioc=0xe9e30a30, > iov=0xdb58e8a8, niov=1, fds=0x0, nfds=0x0, errp=0x0) at ../io/channel.c:66 > #3 0xe380f9e8 in qio_channel_read (ioc=0xe9e30a30, > buf=0xea204e9b "\026\003\001\001L\001", buflen=5, errp=0x0) at > ../io/channel.c:217 > #4 0xe380e7d4 in qio_channel_tls_read_handler (buf=0xea204e9b > "\026\003\001\001L\001", len=5, opaque=0xfffd38001190) at > ../io/channel-tls.c:53 > #5 0xe3801114 in qcrypto_tls_session_pull (opaque=0xe99d5700, > buf=0xea204e9b, len=5) at ../crypto/tlssession.c:89 > #6 0x8822ed30 in _gnutls_stream_read (ms=0xdb58eaac, > pull_func=0xfffd38001870, size=5, bufel=, > session=0xe983cd60) at buffers.c:346 > #7 _gnutls_read (ms=0xdb58eaac, pull_func=0xfffd38001870, size=5, > bufel=, session=0xe983cd60) at buffers.c:426 > #8 _gnutls_io_read_buffered (session=session@entry=0xe983cd60, > total=5, recv_type=recv_type@entry=4294967295, ms=0xdb58eaac) at > buffers.c:581 > #9 0x88224954 in recv_headers (ms=, > record=0x883cd000 , > htype=65535, type=2284006288, record_params=0xe9e22a60, > session=0xe983cd60) at record.c:1163 > #10 _gnutls_recv_in_buffers (session=session@entry=0xe983cd60, > type=2284006288, type@entry=GNUTLS_HANDSHAKE, htype=65535, > htype@entry=GNUTLS_HANDSHAKE_HELLO_RETRY_REQUEST, ms=, > ms@entry=0) at record.c:1302 > #11 0x88230568 in _gnutls_handshake_io_recv_int > (session=session@entry=0xe983cd60, > htype=htype@entry=GNUTLS_HANDSHAKE_HELLO_RETRY_REQUEST, > hsk=hsk@entry=0xdb58ec38, optional=optional@entry=1) at buffers.c:1445 > #12 0x88232b90 in _gnutls_recv_handshake > (session=session@entry=0xe983cd60, > type=type@entry=GNUTLS_HANDSHAKE_HELLO_RETRY_REQUEST, > optional=optional@entry=1, buf=buf@entry=0x0) at handshake.c:1534 > #13 0x88235b40 in handshake_client > (session=session@entry=0xe983cd60) at handshake.c:2925 > #14 0x88237824 in gnutls_handshake (session=0xe983cd60) at > handshake.c:2739 > #15 0xe380213c in qcrypto_tls_session_handshake > (session=0xe99d5700, errp=0xdb58ee58) at ../crypto/tlssession.c:493 > #16 0xe380ea40 in qio_channel_tls_handshake_task > (ioc=0xfffd38001190, task=0xea61d4e0, context=0x0) at > ../io/channel-tls.c:161 > #17 0xe380ec60 in qio_channel_tls_handshake (ioc=0xfffd38001190, > func=0xe3394d20 , opaque=0xea189c30, > destroy=0x0, context=0x0) at ../io/channel-tls.c:239 > #18 0xe3394e78 in multifd_tls_channel_connect (p=0xea189c30, > ioc=0xe9e30a30, errp=0xdb58ef28) at ../migration/multifd.c:782 > #19 0xe3394f30 in multifd_channel_connect (p=0xea189c30, > ioc=0xe9e30a30, error=0x0) at ../migration/multifd.c:804 > #20 0xe33950b8 in multifd_new_send_channel_async > (task=0xea6855a0, opaque=0xea189c30) at ../migration/multifd.c:858 > #21 0xe3810cf8 in qio_task_complete (task=0xea6855a0) at > ../io/task.c:197 > #22 0xe381096c in qio_task_thread_result
Re: [PATCH-for-5.2 v2] hw/mips: Remove the 'r4k' machine
On 11/2/20 7:50 PM, Philippe Mathieu-Daudé wrote: > We deprecated the support for the 'r4k' machine for the 5.0 release > (commit d32dc61421), which means that our deprecation policy allows > us to drop it in release 5.2. Remove the code. > > To repeat the rationale from the deprecation note: > - this virtual machine has no specification > - the Linux kernel dropped support for it 10 years ago > > Users are recommended to use the Malta board instead. > > Acked-by: Richard Henderson > Signed-off-by: Philippe Mathieu-Daudé > --- > v2: Fixed Header underline length (Richard) > --- > docs/system/deprecated.rst| 4 +- > .../devices/mips-softmmu-common.mak | 1 - > hw/mips/r4k.c | 318 -- > MAINTAINERS | 6 - > hw/mips/Kconfig | 13 - > hw/mips/meson.build | 1 - > 6 files changed, 2 insertions(+), 341 deletions(-) > delete mode 100644 hw/mips/r4k.c > > diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst > index 0ebce37a191..2a16078a09b 100644 > --- a/docs/system/deprecated.rst > +++ b/docs/system/deprecated.rst > @@ -327,8 +327,8 @@ The 'scsi-disk' device is deprecated. Users should use > 'scsi-hd' or > System emulator machines > > > -mips ``r4k`` platform (since 5.0) > -' > +mips ``r4k`` platform (removed in 5.2) > +'' > > This machine type is very old and unmaintained. Users should use the > ``malta`` > machine type instead. This is incorrect, I have to move it from "Deprecated features" to "Recently removed features" section.
