Re: [RFC v3 0/6] virtio,vhost: Add VIRTIO_F_IN_ORDER support
QE tested this series with packed=on/off, in_order=true and vhost=off under regression tests, everything are works fine. Tested-by: Lei Yang On Mon, Apr 8, 2024 at 11:34 PM Jonah Palmer wrote: > > The goal of these patches is to add support to a variety of virtio and > vhost devices for the VIRTIO_F_IN_ORDER transport feature. This feature > indicates that all buffers are used by the device in the same order in > which they were made available by the driver. > > These patches attempt to implement a generalized, non-device-specific > solution to support this feature. > > The core feature behind this solution is a buffer mechanism in the form > of a VirtQueue's used_elems VirtQueueElement array. This allows devices > who always use buffers in-order by default to have a minimal overhead > impact. Devices that may not always use buffers in-order likely will > experience a performance hit. How large that performance hit is will > depend on how frequent elements are completed out-of-order. > > A VirtQueue whose device who uses this feature will use its used_elems > VirtQueueElement array to hold used VirtQueueElements. The index that > used elements are placed in used_elems is the same index on the > used/descriptor ring that would satisfy the in-order requirement. In > other words, used elements are placed in their in-order locations on > used_elems and are only written to the used/descriptor ring once the > elements on used_elems are able to continue their expected order. > > To differentiate between a "used" and "unused" element on the used_elems > array (a "used" element being an element that has returned from > processing and an "unused" element being an element that has not yet > been processed), we added a boolean 'filled' member to the > VirtQueueElement struct. This flag is set to true when the element comes > back from processing (virtqueue_ordered_fill) and then set back to false > once it's been written to the used/descriptor ring > (virtqueue_ordered_flush). > > --- > v3: Add elements to used_elems during virtqueue_split/packed_pop > Replace current_seq_idx usage with vq->last_avail_idx > Remove used_seq_idx, leverage used_idx and last_avail_idx for > searching used_elems > Remove seq_idx in VirtQueueElement > Add boolean to VirtQueueElement to signal element status > Add virtqueue_ordered_fill/flush functions for ordering > > v2: Use a VirtQueue's used_elems array as a buffer mechanism > > v1: Implement custom GLib GHashTable as a buffer mechanism > > Jonah Palmer (6): > virtio: Add bool to VirtQueueElement > virtio: virtqueue_pop - VIRTIO_F_IN_ORDER support > virtio: virtqueue_ordered_fill - VIRTIO_F_IN_ORDER support > virtio: virtqueue_ordered_flush - VIRTIO_F_IN_ORDER support > vhost,vhost-user: Add VIRTIO_F_IN_ORDER to vhost feature bits > virtio: Add VIRTIO_F_IN_ORDER property definition > > hw/block/vhost-user-blk.c| 1 + > hw/net/vhost_net.c | 2 + > hw/scsi/vhost-scsi.c | 1 + > hw/scsi/vhost-user-scsi.c| 1 + > hw/virtio/vhost-user-fs.c| 1 + > hw/virtio/vhost-user-vsock.c | 1 + > hw/virtio/virtio.c | 118 ++- > include/hw/virtio/virtio.h | 5 +- > net/vhost-vdpa.c | 1 + > 9 files changed, 127 insertions(+), 4 deletions(-) > > -- > 2.39.3 >
[RFC 1/1] hw/nvme: add atomic write support
Forces writes to be atomic based on new atomic write parameters.
New NVMe QEMU Parameters (See NVMe Specification for details):
atomic.dn (default off) - Set the value of Disable Normal.
atomic.awun=UINT16 (default: 0)
atomic.awupf=UINT16 (default: 0)
atomic.acwu=UINT16 (default: 0)
Signed-off-by: Alan Adamson
---
hw/nvme/ctrl.c | 147 -
hw/nvme/nvme.h | 17 ++
2 files changed, 163 insertions(+), 1 deletion(-)
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 127c3d238346..5d19965122d0 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -40,6 +40,9 @@
* sriov_vi_flexible= \
* sriov_max_vi_per_vf= \
* sriov_max_vq_per_vf= \
+ * atomic.awun, \
+ * atomic.awupf, \
+ * atomic.acwu, \
* subsys=
* -device nvme-ns,drive=,bus=,nsid=,\
* zoned=, \
@@ -254,6 +257,7 @@ static const uint32_t nvme_feature_cap[NVME_FID_MAX] = {
[NVME_ERROR_RECOVERY] = NVME_FEAT_CAP_CHANGE | NVME_FEAT_CAP_NS,
[NVME_VOLATILE_WRITE_CACHE] = NVME_FEAT_CAP_CHANGE,
[NVME_NUMBER_OF_QUEUES] = NVME_FEAT_CAP_CHANGE,
+[NVME_WRITE_ATOMICITY] = NVME_FEAT_CAP_CHANGE,
[NVME_ASYNCHRONOUS_EVENT_CONF] = NVME_FEAT_CAP_CHANGE,
[NVME_TIMESTAMP]= NVME_FEAT_CAP_CHANGE,
[NVME_HOST_BEHAVIOR_SUPPORT]= NVME_FEAT_CAP_CHANGE,
@@ -6071,6 +6075,9 @@ defaults:
}
goto out;
+break;
+case NVME_WRITE_ATOMICITY:
+result = n->dn;
break;
default:
result = nvme_feature_default[fid];
@@ -6154,6 +6161,8 @@ static uint16_t nvme_set_feature(NvmeCtrl *n, NvmeRequest
*req)
uint8_t save = NVME_SETFEAT_SAVE(dw10);
uint16_t status;
int i;
+NvmeIdCtrl *id = >id_ctrl;
+NvmeAtomic *atomic = >atomic;
trace_pci_nvme_setfeat(nvme_cid(req), nsid, fid, save, dw11);
@@ -6306,6 +6315,21 @@ static uint16_t nvme_set_feature(NvmeCtrl *n,
NvmeRequest *req)
return NVME_CMD_SEQ_ERROR | NVME_DNR;
case NVME_FDP_EVENTS:
return nvme_set_feature_fdp_events(n, ns, req);
+case NVME_WRITE_ATOMICITY:
+
+/*
+ * Since NAWUN and NAWUPF are not currently supported, this
+ * has no affect.
+ */
+n->dn = 0x1 & dw11;
+
+if (n->dn) {
+atomic->atomic_max_write_size = id->awupf + 1;
+} else {
+atomic->atomic_max_write_size = id->awun + 1;
+}
+
+break;
default:
return NVME_FEAT_NOT_CHANGEABLE | NVME_DNR;
}
@@ -7003,16 +7027,76 @@ static void nvme_update_sq_tail(NvmeSQueue *sq)
trace_pci_nvme_update_sq_tail(sq->sqid, sq->tail);
}
+#define NVME_ATOMIC_NO_START0
+#define NVME_ATOMIC_START_ATOMIC1
+#define NVME_ATOMIC_START_NONATOMIC 2
+
+static int nvme_atomic_write_check(NvmeCtrl *n, NvmeCmd *cmd, NvmeAtomic
*atomic)
+{
+NvmeRwCmd *rw = (NvmeRwCmd *)cmd;
+uint64_t slba = le64_to_cpu(rw->slba);
+uint32_t nlb = (uint32_t)le16_to_cpu(rw->nlb);
+uint64_t elba = slba + nlb;
+bool cmd_atomic_wr = 1;
+int i;
+
+if ((cmd->opcode == NVME_CMD_READ) || ((cmd->opcode == NVME_CMD_WRITE) &&
+((rw->nlb + 1) > atomic->atomic_max_write_size))) {
+cmd_atomic_wr = 0;
+}
+
+for (i = 1; i < n->params.max_ioqpairs + 1; i++) {
+NvmeSQueue *sq;
+NvmeRequest *r;
+NvmeRwCmd *req_rw;
+uint64_t req_slba;
+uint32_t req_nlb;
+uint64_t req_elba;
+
+sq = n->sq[i];
+if (!sq) {
+break;
+}
+
+QTAILQ_FOREACH(r, >out_req_list, entry) {
+req_rw = (NvmeRwCmd *)>cmd;
+
+if (cmd->nsid == r->ns->params.nsid) {
+req_slba = le64_to_cpu(req_rw->slba);
+req_nlb = (uint32_t)le16_to_cpu(req_rw->nlb);
+req_elba = req_slba + req_nlb;
+
+if (cmd_atomic_wr) {
+if ((elba >= req_slba) && (slba <= req_elba)) {
+return NVME_ATOMIC_NO_START;
+}
+} else {
+if (r->atomic_write && ((elba >= req_slba) &&
+(slba <= req_elba))) {
+return NVME_ATOMIC_NO_START;
+}
+}
+}
+}
+}
+if (cmd_atomic_wr) {
+return NVME_ATOMIC_START_ATOMIC;
+}
+return NVME_ATOMIC_START_NONATOMIC;
+}
+
static void nvme_process_sq(void *opaque)
{
NvmeSQueue *sq = opaque;
NvmeCtrl *n = sq->ctrl;
NvmeCQueue *cq = n->cq[sq->cqid];
-
+NvmeAtomic *atomic = >atomic;
uint16_t status;
hwaddr addr;
NvmeCmd cmd;
NvmeRequest *req;
+int ret;
+bool set_atomic = 0;
if (n->dbbuf_enabled) {
nvme_update_sq_tail(sq);
@@ -7026,6 +7110,23 @@ static void
[RFC 0/1] hw/nvme: add atomic write support
Since there is discussion in the Linux NVMe Driver community to add NVMe Atomic Write support, it would be desirable to test it with qemu nvme emulation. Initially, this RFC will focus on supporting NVMe controller atomic write parameters(AWUN, AWUPF, and ACWU) but will be extended to support Namespace parameters (NAWUN, NAWUPF and NACWU). Atomic Write Parameters for NVMe QEMU - New NVMe QEMU Parameters (See NVMe Specification for details): atomic.dn (default off) - Set the value of Disable Normal. atomic.awun=UINT16 (default: 0) atomic.awupf=UINT16 (default: 0) atomic.acwu=UINT16 (default: 0) qemu command line example: qemu-system-x86_64 -cpu host --enable-kvm -smp cpus=4 -no-reboot -m 8192M -drive file=./disk.img,if=ide \ -boot c -device e1000,netdev=net0,mac=DE:CC:CC:EF:99:88 -netdev tap,id=net0 \ -device nvme,id=nvme-ctrl-0,serial=nvme-1,atomic.dn=off,atomic.awun=63,atomic.awupf=63,atomic.acwu=0 \ -drive file=./nvme.img,if=none,id=nvm-1 -device nvme-ns,drive=nvm-1,bus=nvme-ctrl-0 nvme-ns,drive=nvm-1,bus=nvme-ctrl-0 Making Writes Atomic: - - Prior to a command being pulled off the SQ and executed, a check is made to see if it conflicts "atomically" with a currently executing command. - All currently executing commands on the same namespace, across all SQs need to be checked. - If an atomic conflict is detected, the command is not started and remains on the queue. Testing --- NVMe QEMU Parameters used: atomic.dn=off,atomic.awun=63,atomic.awupf=63,atomic.acwu=0 # nvme id-ctrl /dev/nvme0 | grep awun awun : 63 # nvme id-ctrl /dev/nvme0 | grep awupf awupf : 63 # nvme id-ctrl /dev/nvme0 | grep acwu acwu : 0< Since qemu-nvme doesn't support Compare and Write, this is always zero # nvme get-feature /dev/nvme0 -f 0xa get-feature:0x0a (Write Atomicity Normal), Current value: # # fio --filename=/dev/nvme0n1 --direct=1 --rw=randwrite --bs=32k --iodepth=256 --name=iops --numjobs=50 --verify=crc64 --verify_fatal=1 --ioengine=libaio When executed without atomic write support, eventually the following error will be observed: crc64: verify failed at file /dev/nvme0n1 offset 857669632, length 32768 (requested block: offset=857669632, length=32768, flags=88) Expected CRC: 9c87d3539dafdca0 Received CRC: d521f7ea3b69d2ee When executed with atomic write support, this error no longer happens. Questions - AWUN vs AWUPF - Does the nvme emulation need to do treat these differently? Currently the larger of the two will be used as the max atomic write size. Future Work --- - Namespace support (NAWUN, NAWUPF and NACWU) - Namespace Boundary support (NABSN, NABO, and NABSPF) - Atomic Compare and Write Unit (ACWU) Alan Adamson (1): nvme: add atomic write support hw/nvme/ctrl.c | 147 - hw/nvme/nvme.h | 17 ++ 2 files changed, 163 insertions(+), 1 deletion(-) -- 2.39.3
Re: [PATCH v2 09/13] block/gluster: Use URI parsing code from glib
On Fri, Apr 12, 2024 at 09:40:11AM -0500, Eric Blake wrote:
> On Fri, Apr 12, 2024 at 03:24:11PM +0200, Thomas Huth wrote:
> > Since version 2.66, glib has useful URI parsing functions, too.
> > Use those instead of the QEMU-internal ones to be finally able
> > to get rid of the latter.
> >
> > Signed-off-by: Thomas Huth
> > ---
> > block/gluster.c | 71 -
> > 1 file changed, 35 insertions(+), 36 deletions(-)
> >
> > diff --git a/block/gluster.c b/block/gluster.c
> > index cc74af06dc..1c9505f8bb 100644
> > --- a/block/gluster.c
> > +++ b/block/gluster.c
> > @@ -17,7 +17,6 @@
> > #include "qapi/error.h"
> > #include "qapi/qmp/qdict.h"
> > #include "qapi/qmp/qerror.h"
> > -#include "qemu/uri.h"
> > #include "qemu/error-report.h"
> > #include "qemu/module.h"
> > #include "qemu/option.h"
> > @@ -289,9 +288,9 @@ static void glfs_clear_preopened(glfs_t *fs)
> > }
> > }
> >
> > -static int parse_volume_options(BlockdevOptionsGluster *gconf, char *path)
> > +static int parse_volume_options(BlockdevOptionsGluster *gconf, const char
> > *path)
>
> Is it worth mentioning in the commit message that this includes a
> const-correctness tweak?
>
> > @@ -364,57 +363,57 @@ static int
> > qemu_gluster_parse_uri(BlockdevOptionsGluster *gconf,
> > QAPI_LIST_PREPEND(gconf->server, gsconf);
> >
> > /* transport */
> > -if (!uri->scheme || !strcmp(uri->scheme, "gluster")) {
> > +uri_scheme = g_uri_get_scheme(uri);
> > +if (!uri_scheme || !strcmp(uri_scheme, "gluster")) {
>
> Pre-existing, but per RFC 3986, we should probably be using strcasecmp
> for scheme comparisons (I'm not sure if g_uri_parse guarantees a
> lower-case return, even when the user passed in upper case). That can
> be a separate patch.
Docs say it is lowercase:
https://developer-old.gnome.org/glib/stable/glib-URI-Functions.html
"on return, contains the scheme (converted to lowercase), or NULL."
With regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH v2 09/13] block/gluster: Use URI parsing code from glib
On Fri, Apr 12, 2024 at 03:24:11PM +0200, Thomas Huth wrote: > Since version 2.66, glib has useful URI parsing functions, too. > Use those instead of the QEMU-internal ones to be finally able > to get rid of the latter. > > Signed-off-by: Thomas Huth > --- > block/gluster.c | 71 - > 1 file changed, 35 insertions(+), 36 deletions(-) Reviewed-by: Daniel P. Berrangé With regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH v2 08/13] Remove glib compatibility code that is not required anymore
On Fri, Apr 12, 2024 at 03:24:10PM +0200, Thomas Huth wrote: > Now that we bumped the minumum glib version to 2.66, we can drop > the old code. > > Suggested-by: Paolo Bonzini > Signed-off-by: Thomas Huth > --- > qga/commands-posix-ssh.c | 8 > util/error-report.c | 10 -- > 2 files changed, 18 deletions(-) Reviewed-by: Daniel P. Berrangé With regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH v2 07/13] Bump minimum glib version to v2.66
On Fri, Apr 12, 2024 at 03:24:09PM +0200, Thomas Huth wrote: > Now that we dropped support for CentOS 8 and Ubuntu 20.04, we can > look into bumping the glib version to a new minimum for further > clean-ups. According to repology.org, available versions are: > > CentOS Stream 9: 2.66.7 > Debian 11: 2.66.8 > Fedora 38: 2.74.1 > Freebsd: 2.78.4 > Homebrew: 2.80.0 > Openbsd: 2.78.4 > OpenSuse leap 15.5:2.70.5 > pkgsrc_current:2.78.4 > Ubuntu 22.04: 2.72.1 > > Thus it should be safe to bump the minimum glib version to 2.66 now. > Version 2.66 comes with new functions for URI parsing which will > allow further clean-ups in the following patches. > > Signed-off-by: Thomas Huth > --- > meson.build | 16 +--- > include/glib-compat.h| 27 ++- > qga/commands-posix-ssh.c | 4 ++-- > 3 files changed, 5 insertions(+), 42 deletions(-) Reviewed-by: Daniel P. Berrangé With regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH v2 06/13] ci: move external build environment setups to CentOS Stream 9
On Fri, Apr 12, 2024 at 03:24:08PM +0200, Thomas Huth wrote:
> From: Paolo Bonzini
>
> RHEL 9 (and thus also the derivatives) are available since two years
> now, so according to QEMU's support policy, we can drop the active
> support for the previous major version 8 now.
>
> Thus upgrade our CentOS Stream build environment playbooks to major
> version 9 now.
>
> Signed-off-by: Paolo Bonzini
> Reviewed-by: Thomas Huth
> Message-ID: <20240412103708.27650-1-pbonz...@redhat.com>
> Signed-off-by: Thomas Huth
> ---
> .../stream/{8 => 9}/build-environment.yml | 31 ++---
> .../stream/{8 => 9}/x86_64/configure | 4 +-
> .../stream/{8 => 9}/x86_64/test-avocado | 0
> scripts/ci/setup/build-environment.yml| 44 +++
> 4 files changed, 34 insertions(+), 45 deletions(-)
> rename scripts/ci/org.centos/stream/{8 => 9}/build-environment.yml (75%)
> rename scripts/ci/org.centos/stream/{8 => 9}/x86_64/configure (98%)
> rename scripts/ci/org.centos/stream/{8 => 9}/x86_64/test-avocado (100%)
Reviewed-by: Daniel P. Berrangé
With regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH v2 05/13] .travis.yml: Update the jobs to Ubuntu 22.04
On Fri, Apr 12, 2024 at 03:24:07PM +0200, Thomas Huth wrote:
> According to our support policy, we'll soon drop our official support
> for Ubuntu 20.04 ("Focal Fossa") in QEMU. Thus we should update the
> Travis jobs now to a newer release (Ubuntu 22.04 - "Jammy Jellyfish")
> for future testing. Since all jobs are using this release now, we
> can drop the entries from the individual jobs and use the global
> setting again.
>
> Signed-off-by: Thomas Huth
> ---
> .travis.yml | 13 +++--
> 1 file changed, 3 insertions(+), 10 deletions(-)
Reviewed-by: Daniel P. Berrangé
With regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH v2 04/13] tests: Update our CI to use CentOS Stream 9 instead of 8
On Fri, Apr 12, 2024 at 03:24:06PM +0200, Thomas Huth wrote:
> RHEL 9 (and thus also the derivatives) are available since two years
> now, so according to QEMU's support policy, we can drop the active
> support for the previous major version 8 now.
> Thus upgrade our CentOS Stream container to major version 9 now.
>
> Signed-off-by: Thomas Huth
> ---
> .gitlab-ci.d/buildtest.yml| 16 -
> .gitlab-ci.d/container-core.yml | 4 +--
> .../{centos8.docker => centos9.docker}| 34 +++
> tests/lcitool/mappings.yml| 20 ---
> tests/lcitool/refresh | 2 +-
> tests/vm/centos | 4 +--
> 6 files changed, 26 insertions(+), 54 deletions(-)
> rename tests/docker/dockerfiles/{centos8.docker => centos9.docker} (82%)
Reviewed-by: Daniel P. Berrangé
With regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH v2 03/13] tests/docker/dockerfiles: Run lcitool-refresh after the lcitool update
On Fri, Apr 12, 2024 at 03:24:05PM +0200, Thomas Huth wrote: > This update adds the removing of the EXTERNALLY-MANAGED marker files > that has been added to the lcitool recently. For those who don't know, python now commonly blocks the ability to run 'pip install' outside of a venv. This generally makes sense for a precious installation environment. Our containers are disposable though, so a venv has no benefit. Removing the 'EXTERNALLY-MANAGED' allows the historical arbitrary use of 'pip' outside a venv. lcitool just does this unconditionally given the containers are not precious. > > Signed-off-by: Thomas Huth > --- > tests/docker/dockerfiles/alpine.docker| 3 ++- > tests/docker/dockerfiles/centos8.docker | 1 + > tests/docker/dockerfiles/debian-amd64-cross.docker| 3 ++- > tests/docker/dockerfiles/debian-arm64-cross.docker| 3 ++- > tests/docker/dockerfiles/debian-armel-cross.docker| 3 ++- > tests/docker/dockerfiles/debian-armhf-cross.docker| 3 ++- > tests/docker/dockerfiles/debian-i686-cross.docker | 3 ++- > tests/docker/dockerfiles/debian-mips64el-cross.docker | 3 ++- > tests/docker/dockerfiles/debian-mipsel-cross.docker | 3 ++- > tests/docker/dockerfiles/debian-ppc64el-cross.docker | 3 ++- > tests/docker/dockerfiles/debian-riscv64-cross.docker | 3 ++- > tests/docker/dockerfiles/debian-s390x-cross.docker| 3 ++- > tests/docker/dockerfiles/debian.docker| 1 + > tests/docker/dockerfiles/fedora-win64-cross.docker| 3 ++- > tests/docker/dockerfiles/fedora.docker| 1 + > tests/docker/dockerfiles/opensuse-leap.docker | 1 + > tests/docker/dockerfiles/ubuntu2204.docker| 1 + > 17 files changed, 29 insertions(+), 12 deletions(-) Reviewed-by: Daniel P. Berrangé With regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH v2 02/13] tests/lcitool/libvirt-ci: Update to the latest master branch
On Fri, Apr 12, 2024 at 03:24:04PM +0200, Thomas Huth wrote: > We need the latest fixes for the lcitool to be able to properly > update our CentOS docker file to CentOS Stream 9. > > Signed-off-by: Thomas Huth > --- > tests/lcitool/libvirt-ci | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Daniel P. Berrangé With regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
MAINTAINERS tweak [was: [PATCH for-9.1 0/9] Switch to glib URI parsing code]
[Trying Peter Lieven's alternate address...] On Thu, Mar 28, 2024 at 03:05:57PM +0100, Thomas Huth wrote: > In the QEMU 9.1 development cycle, we can drop the support for > Ubuntu 20.04 and CentOS 8 since the following major versions of > these distributions are available since 2 years already. Every time I've replied to any message in this thread, I've gotten a response: | +Your message to p...@kamp.de couldn't be delivered. | kamp.de couldn't confirm that your message was sent from a trusted location. | eblake Office 365 pl | Action Required Recipient | SPF validation error | | How to Fix It | Your organization's email admin will have to diagnose and fix your domain's email settings. Please forward this message to your | +email admin. | | | | More Info for Email Admins | Status code: 550 5.7.23 | | This error occurs when Sender Policy Framework (SPF) validation for the sender's domain fails. If you're the sender's email | +admin, make sure the SPF records for your domain at your domain registrar are set up correctly. Office 365 supports only one | +SPF record (a TXT record that defines SPF) for your domain. Include the following domain name: spf.protection.outlook.com. If | +you have a hybrid configuration (some mailboxes in the cloud, and some mailboxes on premises) or if you're an Exchange Online | +Protection standalone customer, add the outbound IP address of your on-premises servers to the TXT record. Red Hat IT says that it is unlikely to be Red Hat's SPF settings, and suspects that it is instead something caused by whatever Peter is using to bounce mail from his alias Peter Lieven to his Office 365 account. As I appear to be unable to contact Peter (even my use of direct email, bypassing the list, and using a personal account instead of my Red Hat email) about this issue, I'm wondering if Peter is still an active contributor to the project. But while typing this email, to see if RBD, iSCSI, and NFS need a new entry in MAINTAINERS, I did a search through the list archives, where the last email I found from Peter was https://lists.gnu.org/archive/html/qemu-devel/2023-01/msg00574.html, which asked to update MAINTAINERS to his new address, and that has not made it in so far... -- Eric Blake, Principal Software Engineer Red Hat, Inc. Virtualization: qemu.org | libguestfs.org
Re: [PATCH 02/12] hw/vfio/pci: Replace sprintf() by g_strdup_printf()
On 4/12/24 17:25, Alex Williamson wrote: On Wed, 10 Apr 2024 18:06:03 +0200 Philippe Mathieu-Daudé wrote: sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1, resulting in painful developper experience. Use g_strdup_printf() instead. Isn't this code only compiled for Linux hosts? It is not. Maybe still a valid change, but the rationale seems irrelevant. I agree the commit log should be rephrased. There is also a v2 doing a different change : https://lore.kernel.org/qemu-devel/20240411101550.99392-1-phi...@linaro.org/ This is a bit confusing. Thanks, C.
Re: [PATCH v2 11/15] hw/southbridge/ich9: Add a AHCI function
Am 26. Februar 2024 11:14:10 UTC schrieb "Philippe Mathieu-Daudé"
:
>Instantiate TYPE_ICH9_AHCI in TYPE_ICH9_SOUTHBRIDGE.
>
>Since the PC machines can disable SATA (see the
>PC_MACHINE_SATA dynamic property), add the 'sata-enabled'
>property to disable it.
>
>Signed-off-by: Philippe Mathieu-Daudé
>---
> MAINTAINERS | 2 ++
> include/hw/southbridge/ich9.h | 4
> hw/i386/pc_q35.c | 21 +++--
> hw/southbridge/ich9.c | 35 +++
> hw/i386/Kconfig | 1 -
> hw/southbridge/Kconfig| 1 +
> 6 files changed, 41 insertions(+), 23 deletions(-)
>
>diff --git a/MAINTAINERS b/MAINTAINERS
>index 4576339053..7d1b3e0d99 100644
>--- a/MAINTAINERS
>+++ b/MAINTAINERS
>@@ -2615,10 +2615,12 @@ M: Marcel Apfelbaum
> S: Supported
> F: hw/acpi/ich9*.c
> F: hw/i2c/smbus_ich9.c
>+F: hw/ide/ich9_ahci.c
> F: hw/isa/lpc_ich9.c
> F: hw/southbridge/ich9.c
> F: include/hw/acpi/ich9*.h
> F: include/hw/i2c/ich9_smbus.h
>+F: include/hw/ide/ahci-pci.h
> F: include/hw/pci-bridge/ich9_dmi.h
> F: include/hw/southbridge/ich9.h
>
>diff --git a/include/hw/southbridge/ich9.h b/include/hw/southbridge/ich9.h
>index b9122d299d..ac7f9f4ff5 100644
>--- a/include/hw/southbridge/ich9.h
>+++ b/include/hw/southbridge/ich9.h
>@@ -166,10 +166,6 @@ struct ICH9LPCState {
>
> #define ICH9_GPIO_GSI "gsi"
>
>-/* D31:F2 SATA Controller #1 */
>-#define ICH9_SATA1_DEV 31
>-#define ICH9_SATA1_FUNC 2
>-
> /* D31:F0 power management I/O registers
>offset from the address ICH9_LPC_PMBASE */
>
>diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
>index f951cf1e3a..6903719b97 100644
>--- a/hw/i386/pc_q35.c
>+++ b/hw/i386/pc_q35.c
>@@ -60,9 +60,6 @@
> #include "hw/i386/acpi-build.h"
> #include "target/i386/cpu.h"
>
>-/* ICH9 AHCI has 6 ports */
>-#define MAX_SATA_PORTS 6
>-
> struct ehci_companions {
> const char *name;
> int func;
>@@ -134,7 +131,6 @@ static void pc_q35_init(MachineState *machine)
> ISABus *isa_bus;
> int i;
> ram_addr_t lowmem;
>-DriveInfo *hd[MAX_SATA_PORTS];
> MachineClass *mc = MACHINE_GET_CLASS(machine);
> bool acpi_pcihp;
> bool keep_pci_slot_hpc;
>@@ -227,6 +223,7 @@ static void pc_q35_init(MachineState *machine)
> object_property_set_link(OBJECT(ich9), "mch-pcie-bus",
> OBJECT(pcms->pcibus), _abort);
> qdev_prop_set_bit(ich9, "d2p-enabled", false);
>+qdev_prop_set_bit(ich9, "sata-enabled", pcms->sata_enabled);
> qdev_realize_and_unref(ich9, NULL, _fatal);
>
> /* create ISA bus */
>@@ -287,20 +284,8 @@ static void pc_q35_init(MachineState *machine)
> 0xff0104);
>
> if (pcms->sata_enabled) {
>-PCIDevice *pdev;
>-AHCIPCIState *ich9;
The ahci include and perhaps all ide includes can be removed here.
Any plans for a v3?
Best regards,
Bernhard
>-
>-/* ahci and SATA device, for q35 1 ahci controller is built-in */
>-pdev = pci_create_simple_multifunction(pcms->pcibus,
>- PCI_DEVFN(ICH9_SATA1_DEV,
>- ICH9_SATA1_FUNC),
>- "ich9-ahci");
>-ich9 = ICH9_AHCI(pdev);
>-pcms->idebus[0] = qdev_get_child_bus(DEVICE(pdev), "ide.0");
>-pcms->idebus[1] = qdev_get_child_bus(DEVICE(pdev), "ide.1");
>-g_assert(MAX_SATA_PORTS == ich9->ahci.ports);
>-ide_drive_get(hd, ich9->ahci.ports);
>-ahci_ide_create_devs(>ahci, hd);
>+pcms->idebus[0] = qdev_get_child_bus(ich9, "ide.0");
>+pcms->idebus[1] = qdev_get_child_bus(ich9, "ide.1");
> }
>
> if (machine_usb(machine)) {
>diff --git a/hw/southbridge/ich9.c b/hw/southbridge/ich9.c
>index 8c4356ff74..37255bb941 100644
>--- a/hw/southbridge/ich9.c
>+++ b/hw/southbridge/ich9.c
>@@ -13,22 +13,30 @@
> #include "hw/southbridge/ich9.h"
> #include "hw/pci/pci.h"
> #include "hw/pci-bridge/ich9_dmi.h"
>+#include "hw/ide/ahci-pci.h"
>+#include "hw/ide/ide-dev.h"
>
> #define ICH9_D2P_DEVFN PCI_DEVFN(30, 0)
>+#define ICH9_SATA1_DEVFNPCI_DEVFN(31, 2)
>+
>+#define SATA_PORTS 6
>
> struct ICH9State {
> DeviceState parent_obj;
>
> I82801b11Bridge d2p;
>+AHCIPCIState sata0;
>
> PCIBus *pci_bus;
> bool d2p_enabled;
>+bool sata_enabled;
> };
>
> static Property ich9_props[] = {
> DEFINE_PROP_LINK("mch-pcie-bus", ICH9State, pci_bus,
> TYPE_PCIE_BUS, PCIBus *),
> DEFINE_PROP_BOOL("d2p-enabled", ICH9State, d2p_enabled, true),
>+DEFINE_PROP_BOOL("sata-enabled", ICH9State, sata_enabled, true),
> DEFINE_PROP_END_OF_LIST(),
> };
>
>@@ -52,6 +60,29 @@ static bool ich9_realize_d2p(ICH9State *s, Error **errp)
> return true;
> }
>
>+static bool ich9_realize_sata(ICH9State *s, Error **errp)
>+{
>+DriveInfo *hd[SATA_PORTS];
Re: [PATCH v2] tests/qtest : Use `g_assert_cmphex` instead of `g_assert_cmpuint`
On 14/4/24 19:28, Inès Varhol wrote: The messages for assertions using hexadecimal numbers will be easier to understand with `g_assert_cmphex`. Cases changed : "cmpuint.*0x", "cmpuint.*<<" Signed-off-by: Inès Varhol --- tests/qtest/aspeed_fsi-test.c | 20 ++-- tests/qtest/cmsdk-apb-dualtimer-test.c | 2 +- tests/qtest/cmsdk-apb-watchdog-test.c | 2 +- tests/qtest/erst-test.c| 2 +- tests/qtest/ivshmem-test.c | 10 +- tests/qtest/libqos/ahci.c | 4 +- tests/qtest/microbit-test.c| 46 - tests/qtest/sse-timer-test.c | 4 +- tests/qtest/stm32l4x5_exti-test.c | 138 - tests/qtest/stm32l4x5_syscfg-test.c| 74 ++--- 10 files changed, 151 insertions(+), 151 deletions(-) Thanks for the generic cleanup! Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v2 03/13] hw/ppc/spapr: Replace sprintf() by snprintf()
On 4/11/24 15:45, Philippe Mathieu-Daudé wrote: sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1, resulting in painful developper experience. s/developper/developer ? Replace sprintf() by snprintf() in order to avoid: hw/ppc/spapr.c:385:5: warning: 'sprintf' is deprecated: This function is provided for compatibility reasons only. Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use snprintf(3) instead. [-Wdeprecated-declarations] sprintf(mem_name, "memory@%" HWADDR_PRIx, start); ^ 1 warning generated. Signed-off-by: Philippe Mathieu-Daudé With the typo fixed, Reviewed-by: Harsh Prateek Bora --- hw/ppc/spapr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index e9bc97fee0..9e97992c79 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -382,7 +382,7 @@ static int spapr_dt_memory_node(SpaprMachineState *spapr, void *fdt, int nodeid, mem_reg_property[0] = cpu_to_be64(start); mem_reg_property[1] = cpu_to_be64(size); -sprintf(mem_name, "memory@%" HWADDR_PRIx, start); +snprintf(mem_name, sizeof(mem_name), "memory@%" HWADDR_PRIx, start); off = fdt_add_subnode(fdt, 0, mem_name); _FDT(off); _FDT((fdt_setprop_string(fdt, off, "device_type", "memory")));
