[libvirt PATCH v3 2/3] qemu: add vhost-vdpa capability
Recent versions of qemu added the -netdev vhost-vdpa device. This
capability allows libvirt to know whether this is supported.
Signed-off-by: Jonathon Jongsma
---
src/qemu/qemu_capabilities.c | 4
src/qemu/qemu_capabilities.h | 3 +++
tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml | 1 +
4 files changed, 9 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 2cc0c61588..b19928f68d 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -597,6 +597,9 @@ VIR_ENUM_IMPL(virQEMUCaps,
"spapr-tpm-proxy",
"numa.hmat",
"blockdev-hostdev-scsi",
+
+ /* 380 */
+ "netdev.vhost-vdpa",
);
@@ -1526,6 +1529,7 @@ static struct virQEMUCapsStringFlags
virQEMUCapsQMPSchemaQueries[] = {
{ "migrate-set-parameters/arg-type/downtime-limit",
QEMU_CAPS_MIGRATION_PARAM_DOWNTIME },
{ "migrate-set-parameters/arg-type/xbzrle-cache-size",
QEMU_CAPS_MIGRATION_PARAM_XBZRLE_CACHE_SIZE },
{ "set-numa-node/arg-type/+hmat-lb", QEMU_CAPS_NUMA_HMAT },
+{ "netdev_add/arg-type/+vhost-vdpa", QEMU_CAPS_NETDEV_VHOST_VDPA },
};
typedef struct _virQEMUCapsObjectTypeProps virQEMUCapsObjectTypeProps;
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 5d08941538..b6110f1c34 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -578,6 +578,9 @@ typedef enum { /* virQEMUCapsFlags grouping marker for
syntax-check */
QEMU_CAPS_NUMA_HMAT, /* -numa hmat */
QEMU_CAPS_BLOCKDEV_HOSTDEV_SCSI, /* -blockdev used for (i)SCSI hostdevs */
+/* 380 */
+QEMU_CAPS_NETDEV_VHOST_VDPA, /* -netdev vhost-vdpa*/
+
QEMU_CAPS_LAST /* this must always be the last item */
} virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml
b/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml
index 7496ff1379..0fd2f3b816 100644
--- a/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml
@@ -242,6 +242,7 @@
+
5001000
0
43100242
diff --git a/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml
b/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml
index 151bd18137..e4686000a9 100644
--- a/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml
@@ -242,6 +242,7 @@
+
5001050
0
43100243
--
2.26.2
[libvirt PATCH v3 3/3] qemu: add vdpa support
Enable for qemu domains. This provides basic
support and does not support hotplug or migration.
Signed-off-by: Jonathon Jongsma
---
src/qemu/qemu_command.c | 30 +--
src/qemu/qemu_command.h | 3 +-
src/qemu/qemu_domain.c| 6 ++-
src/qemu/qemu_hotplug.c | 12 +++---
src/qemu/qemu_interface.c | 23
src/qemu/qemu_interface.h | 2 +
src/qemu/qemu_migration.c | 10 -
src/qemu/qemu_validate.c | 14 +++
.../net-vdpa.x86_64-latest.args | 37 +++
tests/qemuxml2argvdata/net-vdpa.xml | 28 ++
tests/qemuxml2argvmock.c | 11 +-
tests/qemuxml2argvtest.c | 1 +
tests/qemuxml2xmloutdata/net-vdpa.xml | 34 +
tests/qemuxml2xmltest.c | 1 +
14 files changed, 199 insertions(+), 13 deletions(-)
create mode 100644 tests/qemuxml2argvdata/net-vdpa.x86_64-latest.args
create mode 100644 tests/qemuxml2argvdata/net-vdpa.xml
create mode 100644 tests/qemuxml2xmloutdata/net-vdpa.xml
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 7b7176eb72..1c8c723d58 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3553,7 +3553,8 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
size_t tapfdSize,
char **vhostfd,
size_t vhostfdSize,
-const char *slirpfd)
+const char *slirpfd,
+const char *vdpadev)
{
bool is_tap = false;
virDomainNetType netType = virDomainNetGetActualType(net);
@@ -3692,6 +3693,12 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
break;
case VIR_DOMAIN_NET_TYPE_VDPA:
+/* Caller will pass the fd to qemu with add-fd */
+if (virJSONValueObjectCreate(&netprops, "s:type", "vhost-vdpa", NULL)
< 0 ||
+virJSONValueObjectAppendString(netprops, "vhostdev", vdpadev) < 0)
+return NULL;
+break;
+
case VIR_DOMAIN_NET_TYPE_HOSTDEV:
/* Should have been handled earlier via PCI/USB hotplug code. */
case VIR_DOMAIN_NET_TYPE_LAST:
@@ -8017,6 +8024,8 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
char **tapfdName = NULL;
char **vhostfdName = NULL;
g_autofree char *slirpfdName = NULL;
+g_autofree char *vdpafdName = NULL;
+int vdpafd = -1;
virDomainNetType actualType = virDomainNetGetActualType(net);
const virNetDevBandwidth *actualBandwidth;
bool requireNicdev = false;
@@ -8102,13 +8111,17 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
break;
+case VIR_DOMAIN_NET_TYPE_VDPA:
+if ((vdpafd = qemuInterfaceVDPAConnect(net)) < 0)
+goto cleanup;
+break;
+
case VIR_DOMAIN_NET_TYPE_USER:
case VIR_DOMAIN_NET_TYPE_SERVER:
case VIR_DOMAIN_NET_TYPE_CLIENT:
case VIR_DOMAIN_NET_TYPE_MCAST:
case VIR_DOMAIN_NET_TYPE_INTERNAL:
case VIR_DOMAIN_NET_TYPE_UDP:
-case VIR_DOMAIN_NET_TYPE_VDPA:
case VIR_DOMAIN_NET_TYPE_LAST:
/* nada */
break;
@@ -8225,13 +8238,24 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
vhostfd[i] = -1;
}
+if (vdpafd > 0) {
+g_autofree char *fdset = NULL;
+
+virCommandPassFD(cmd, vdpafd, VIR_COMMAND_PASS_FD_CLOSE_PARENT);
+fdset = qemuVirCommandGetFDSet(cmd, vdpafd);
+if (!fdset)
+goto cleanup;
+virCommandAddArgList(cmd, "-add-fd", fdset, NULL);
+vdpafdName = qemuVirCommandGetDevSet(cmd, vdpafd);
+}
+
if (chardev)
virCommandAddArgList(cmd, "-chardev", chardev, NULL);
if (!(hostnetprops = qemuBuildHostNetStr(net,
tapfdName, tapfdSize,
vhostfdName, vhostfdSize,
- slirpfdName)))
+ slirpfdName, vdpafdName)))
goto cleanup;
if (!(host = virQEMUBuildNetdevCommandlineFromJSON(hostnetprops,
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 89d99b111f..8db51f93b1 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -99,7 +99,8 @@ virJSONValuePtr qemuBuildHostNetStr(virDomainNetDefPtr net,
size_t tapfdSize,
char **vhostfd,
size_t vhostfdSize,
-const char *slirpfd);
+const char *slirpfd,
+const char *vdpadev);
/* Current, best practice */
char *qemuBuildNicDevStr(virDomainDefPtr def,
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
inde
[libvirt PATCH v3 1/3] conf: Add support for vDPA network devices
This patch adds new schema and adds support for parsing and formatting
domain configurations that include vdpa devices.
vDPA network devices allow high-performance networking in a virtual
machine by providing a wire-speed data path. These devices require a
vendor-specific host driver but the data path follows the virtio
specification.
When a device on the host is bound to an appropriate vendor-specific
driver, it will create a chardev on the host at e.g. /dev/vhost-vdpa-0.
That chardev path can then be used to define a new interface with
type='vdpa'.
Signed-off-by: Jonathon Jongsma
---
docs/formatdomain.rst| 24
docs/schemas/domaincommon.rng| 15 +++
src/conf/domain_conf.c | 31 +++
src/conf/domain_conf.h | 4
src/conf/netdev_bandwidth_conf.c | 1 +
src/libxl/libxl_conf.c | 1 +
src/libxl/xen_common.c | 1 +
src/lxc/lxc_controller.c | 1 +
src/lxc/lxc_driver.c | 3 +++
src/lxc/lxc_process.c| 1 +
src/qemu/qemu_command.c | 3 +++
src/qemu/qemu_domain.c | 4 +++-
src/qemu/qemu_hotplug.c | 3 +++
src/qemu/qemu_interface.c| 2 ++
src/qemu/qemu_process.c | 2 ++
src/qemu/qemu_validate.c | 1 +
src/vmx/vmx.c| 1 +
tools/virsh-domain.c | 1 +
18 files changed, 98 insertions(+), 1 deletion(-)
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index 18e237c157..b0b38d8cf5 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -4643,6 +4643,30 @@ or stopping the guest.
...
+:anchor:``
+
+vDPA devices
+
+
+A vDPA network device can be used to provide wire speed network performance
+within a domain. A vDPA device is a specialized type of network device that
+uses a datapath that complies with the virtio specification but has a
+vendor-specific control path. To use such a device with libvirt, the host
+device must already be bound to the appropriate device-specific vDPA driver.
+This creates a vDPA char device (e.g. /dev/vhost-vdpa-0) that can be used to
+assign the device to a libvirt domain. :since:`Since 6.8.0 (QEMU only,
+requires QEMU 5.1.0 or newer)`
+
+::
+
+ ...
+
+
+
+
+
+ ...
+
:anchor:``
Teaming a virtio/hostdev NIC pair
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index a1d6d19e2f..4708e3550f 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3111,6 +3111,21 @@
+
+
+
+vdpa
+
+
+
+
+
+
+
+
+
+
+
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 72ac4f4191..cb3f6ae4c3 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -550,6 +550,7 @@ VIR_ENUM_IMPL(virDomainNet,
"direct",
"hostdev",
"udp",
+ "vdpa",
);
VIR_ENUM_IMPL(virDomainNetModel,
@@ -2502,6 +2503,10 @@ virDomainNetDefClear(virDomainNetDefPtr def)
def->data.vhostuser = NULL;
break;
+case VIR_DOMAIN_NET_TYPE_VDPA:
+VIR_FREE(def->data.vdpa.devicepath);
+break;
+
case VIR_DOMAIN_NET_TYPE_SERVER:
case VIR_DOMAIN_NET_TYPE_CLIENT:
case VIR_DOMAIN_NET_TYPE_MCAST:
@@ -12129,6 +12134,10 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
if (virDomainChrSourceReconnectDefParseXML(&reconnect, cur,
ctxt) < 0)
goto error;
+} else if (!dev
+ && def->type == VIR_DOMAIN_NET_TYPE_VDPA
+ && virXMLNodeNameEqual(cur, "source")) {
+dev = virXMLPropString(cur, "dev");
} else if (!def->virtPortProfile
&& virXMLNodeNameEqual(cur, "virtualport")) {
if (def->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
@@ -12386,6 +12395,16 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
}
break;
+case VIR_DOMAIN_NET_TYPE_VDPA:
+if (dev == NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("No 'dev' attribute "
+ "specified with "));
+goto error;
+}
+def->data.vdpa.devicepath = g_steal_pointer(&dev);
+break;
+
case VIR_DOMAIN_NET_TYPE_BRIDGE:
if (bridge == NULL) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -12775,6 +12794,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
case VIR_DOMAIN_NET_TYPE_DIRECT:
case VIR_DOMAIN_NET_TYPE_HOSTDEV:
case VIR_DOMAIN_NET_TYPE_UDP:
+case VIR_DOMAIN_NET_TYPE_VDPA:
break;
case VIR_DOMAIN_NET_TYPE_LAST:
[libvirt PATCH v3 0/3] Add support for vDPA network devices
vDPA network devices allow high-performance networking in a virtual machine by providing a wire-speed data path. These devices require a vendor-specific host driver but the data path follows the virtio specification. The support for vDPA devices was recently added to qemu. This allows libvirt to support these devices. This patchset requires that the device is configured on the host with the appropriate vendor-specific driver. This will create a chardev on the host at e.g. /dev/vhost-vdpa-0. That chardev path can then be used to define a new interface with type='vdpa'. Changes in v3: - rebased to latest master - various small fixes per comments on last review - investigated hotplug -- I have a preliminary libvirt patch, but it doesn't appear that hotplug is fully supported in qemu yet. - qemu never closes the fd when the device is removed, so the device cannot be reassigned to a different domain. - I decided not to post the preliminary hotplug patch until I can get more clarity from qemu developers. Jonathon Jongsma (3): conf: Add support for vDPA network devices qemu: add vhost-vdpa capability qemu: add vdpa support docs/formatdomain.rst | 24 docs/schemas/domaincommon.rng | 15 src/conf/domain_conf.c| 31 src/conf/domain_conf.h| 4 ++ src/conf/netdev_bandwidth_conf.c | 1 + src/libxl/libxl_conf.c| 1 + src/libxl/xen_common.c| 1 + src/lxc/lxc_controller.c | 1 + src/lxc/lxc_driver.c | 3 ++ src/lxc/lxc_process.c | 1 + src/qemu/qemu_capabilities.c | 4 ++ src/qemu/qemu_capabilities.h | 3 ++ src/qemu/qemu_command.c | 31 +++- src/qemu/qemu_command.h | 3 +- src/qemu/qemu_domain.c| 6 ++- src/qemu/qemu_hotplug.c | 15 +--- src/qemu/qemu_interface.c | 25 + src/qemu/qemu_interface.h | 2 + src/qemu/qemu_migration.c | 10 - src/qemu/qemu_process.c | 2 + src/qemu/qemu_validate.c | 15 src/vmx/vmx.c | 1 + .../caps_5.1.0.x86_64.xml | 1 + .../caps_5.2.0.x86_64.xml | 1 + .../net-vdpa.x86_64-latest.args | 37 +++ tests/qemuxml2argvdata/net-vdpa.xml | 28 ++ tests/qemuxml2argvmock.c | 11 +- tests/qemuxml2argvtest.c | 1 + tests/qemuxml2xmloutdata/net-vdpa.xml | 34 + tests/qemuxml2xmltest.c | 1 + tools/virsh-domain.c | 1 + 31 files changed, 303 insertions(+), 11 deletions(-) create mode 100644 tests/qemuxml2argvdata/net-vdpa.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/net-vdpa.xml create mode 100644 tests/qemuxml2xmloutdata/net-vdpa.xml -- 2.26.2
[PATCH] daemon: Fix a comment typo in libvirtd.conf.in
Signed-off-by: Jim Fehlig --- Pushing under the trivial rule. src/remote/libvirtd.conf.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/remote/libvirtd.conf.in b/src/remote/libvirtd.conf.in index ae6207bf54..ad049f636b 100644 --- a/src/remote/libvirtd.conf.in +++ b/src/remote/libvirtd.conf.in @@ -173,7 +173,7 @@ # # If libvirt was compiled with support for 'polkit', then # the systemd .socket files will use SocketMode=0666 which -# allows any user to connect and 'auth_iunix_rw' will default +# allows any user to connect and 'auth_unix_rw' will default # to 'polkit'. If you disable use of 'polkit' here, then it # is essential to change the systemd SocketMode parameter # back to 0600, to avoid an insecure configuration. -- 2.28.0
[PATCH] xen: Don't add dom0 twice on driver reload
When the xen driver loads, it probes libxl for some info about dom0 and
adds it to the virDomainObjList. The driver then looks for any domains
in stateDir and if they are still alive adds them to the list as well.
This logic is a bit flawed wrt handling driver reload and causes the
following error
internal error: unexpected domain Domain-0 already exists
A simple fix is to load all domains from stateDir first and then only
add dom0 if needed.
Signed-off-by: Jim Fehlig
---
src/libxl/libxl_driver.c | 46 +++-
1 file changed, 26 insertions(+), 20 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 161a6882f3..083738871d 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -604,27 +604,33 @@ libxlAddDom0(libxlDriverPrivatePtr driver)
goto cleanup;
}
-if (!(def = virDomainDefNew()))
-goto cleanup;
+/*
+ * On a driver reload dom0 will already exist. On host restart it must
+ * created.
+ */
+if ((vm = virDomainObjListFindByID(driver->domains, 0)) == NULL) {
+if (!(def = virDomainDefNew()))
+goto cleanup;
-def->id = 0;
-def->virtType = VIR_DOMAIN_VIRT_XEN;
-def->name = g_strdup("Domain-0");
+def->id = 0;
+def->virtType = VIR_DOMAIN_VIRT_XEN;
+def->name = g_strdup("Domain-0");
-def->os.type = VIR_DOMAIN_OSTYPE_XEN;
+def->os.type = VIR_DOMAIN_OSTYPE_XEN;
-if (virUUIDParse("----", def->uuid) < 0)
-goto cleanup;
+if (virUUIDParse("----", def->uuid) <
0)
+goto cleanup;
-if (!(vm = virDomainObjListAdd(driver->domains, def,
- driver->xmlopt,
- 0,
- NULL)))
-goto cleanup;
-def = NULL;
+if (!(vm = virDomainObjListAdd(driver->domains, def,
+ driver->xmlopt,
+ 0,
+ NULL)))
+goto cleanup;
+
+vm->persistent = 1;
+virDomainObjSetState(vm, VIR_DOMAIN_RUNNING,
VIR_DOMAIN_RUNNING_BOOTED);
+}
-vm->persistent = 1;
-virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_BOOTED);
if (virDomainDefSetVcpusMax(vm->def, d_info.vcpu_max_id + 1,
driver->xmlopt))
goto cleanup;
@@ -783,10 +789,6 @@ libxlStateInitialize(bool privileged,
if (!(libxl_driver->xmlopt = libxlCreateXMLConf(libxl_driver)))
goto error;
-/* Add Domain-0 */
-if (libxlAddDom0(libxl_driver) < 0)
-goto error;
-
/* Load running domains first. */
if (virDomainObjListLoadAllConfigs(libxl_driver->domains,
cfg->stateDir,
@@ -796,6 +798,10 @@ libxlStateInitialize(bool privileged,
NULL, NULL) < 0)
goto error;
+/* Add Domain-0 */
+if (libxlAddDom0(libxl_driver) < 0)
+goto error;
+
libxlReconnectDomains(libxl_driver);
/* Then inactive persistent configs */
--
2.28.0
Re: [PATCH v2] network: Only check kernel added routes in virNetDevIPCheckIPv6Forwarding
On 9/11/20 7:34 AM, Ian Wienand wrote:
The original motivation for adding virNetDevIPCheckIPv6Forwarding
(00d28a78b5d1f6eaf79f06ac59e31c568af9da37) was that networking routes
would disappear when ipv6 forwarding was enabled for an interface.
This is a fairly undocumented side-effect of the "accept_ra" sysctl
for an interface. 1 means the interface will accept_ra's if not
forwarding, 2 means always accept_RAs; but it is not explained that
enabling forwarding when accept_ra==1 will also clear any kernel RA
assigned routes, very likely breaking your networking.
The check to warn about this currently uses netlink to go through all
the routes and then look at the accept_ra status of the interfaces.
However, it has been noticed that this problem does not affect systems
where IPv6 RA configuration is handled in userspace, e.g. via tools
such as NetworkManager. In this case, the error message from libvirt
is spurious, and modifying the forwarding state will not affect the RA
state or disable your networking.
If you refer to the function rt6_purge_dflt_routers() in the kernel,
we can see that the routes being purged are only those with the
kernel's RTF_ADDRCONF flag set; that is, routes added by the kernel's
RA handling. Why does it do this? I think this is a Linux
implementation decision; it has always been like that and there are
some comments suggesting that it is because a router should be
statically configured, rather than accepting external configurations.
The solution implemented here is to convert the existing check into a
walk of /proc/net/ipv6_route and look for routes with this flag set.
We then check the accept_ra status for the interface, and if enabling
forwarding would break things raise an error.
This should hopefully avoid "interactive" users, who are likely to be
using NetworkManager and the like, having false warnings when enabling
IPv6, but retain the error check for users relying on kernel-based
IPv6 interface auto-configuration.
Signed-off-by: Ian Wienand
---
Cedric - I saw you've given your ACK; have you been able to try this
code on a system that actually handles RA in the kernel, and gotten the
error message? Every system I have readily available to test on does RA
in userspace, so I can't reproduce the error condition. If you've
actually seen it take effect, I'll push this with both our ACKs,
otherwise I guess I'll need to take some time to disable NetworkManager
on some machine and try it out.
My comments: Normally I would be against using /proc rather than netlink
to get this information, but according to an earlier email exchange we
had, apparently the RTF_ADDRCONF flag isn't exposed in netlink. And we
also have a precedent in the check for conflicting IPv4 routes in the
network driver (networkCheckRouteCollision(), which reads
/proc/net/route). (I was actually hoping to get rid of that function in
favor of reading the routing table via netlink as Cedric's code does,
and still I still might, but in this case it sounds like we'll have to
continue to use /proc.
So,
Reviewed-by: Laine Stump
(pending successful completion of CI (see below) and verification that
the error is triggered when appropriate. Once I've verified those two,
I'll push it).
Thanks for taking the time to dig down on this problem (which has been
nagging at me for awhile now) and figuring out a workable solution!
src/util/virnetdevip.c | 323 -
1 file changed, 124 insertions(+), 199 deletions(-)
diff --git a/src/util/virnetdevip.c b/src/util/virnetdevip.c
index 7bd5a75f85..e208089bd6 100644
--- a/src/util/virnetdevip.c
+++ b/src/util/virnetdevip.c
@@ -43,8 +43,11 @@
#ifdef __linux__
# include
# include
+# include
#endif
+#define PROC_NET_IPV6_ROUTE "/proc/net/ipv6_route"
+
#define VIR_FROM_THIS VIR_FROM_NONE
VIR_LOG_INIT("util.netdevip");
@@ -370,203 +373,6 @@ virNetDevIPRouteAdd(const char *ifname,
}
-static int
-virNetDevIPGetAcceptRA(const char *ifname)
-{
-g_autofree char *path = NULL;
-g_autofree char *buf = NULL;
-char *suffix;
-int accept_ra = -1;
-
-path = g_strdup_printf("/proc/sys/net/ipv6/conf/%s/accept_ra",
- ifname ? ifname : "all");
-
-if ((virFileReadAll(path, 512, &buf) < 0) ||
-(virStrToLong_i(buf, &suffix, 10, &accept_ra) < 0))
-return -1;
-
-return accept_ra;
-}
-
-struct virNetDevIPCheckIPv6ForwardingData {
-bool hasRARoutes;
-
-/* Devices with conflicting accept_ra */
-char **devices;
-size_t ndevices;
-};
-
-
-static int
-virNetDevIPCheckIPv6ForwardingAddIF(struct virNetDevIPCheckIPv6ForwardingData
*data,
-char **ifname)
-{
-size_t i;
-
-/* add ifname to the array if it's not already there
- * (ifname is char** so VIR_APPEND_ELEMENT() will move the
- * original pointer out of the way and avoid having it freed)
- */
-for (i = 0; i
Re: [PATCH 04/17] syntax-check: Don't forbid curly braces around single line condition body
On a Friday in 2020, Peter Krempa wrote:
This syntax rule doesn't make much sense, especially if there are so
much exceptions to it. Just remove it and adjust the coding style.
Signed-off-by: Peter Krempa
---
build-aux/check-spacing.pl | 36
before:
$ hyperfine 'make -C build/build-aux sc_spacing-check'
Benchmark #1: make -C build/build-aux sc_spacing-check
Time (mean ± σ): 1.385 s ± 0.023 s[User: 1.386 s, System: 0.022 s]
Range (min … max):1.356 s … 1.425 s10 runs
after:
$ hyperfine 'make -C build/build-aux sc_spacing-check'
Benchmark #1: make -C build/build-aux sc_spacing-check
Time (mean ± σ): 1.215 s ± 0.025 s[User: 1.217 s, System: 0.024 s]
Range (min … max):1.179 s … 1.259 s10 runs
Yay, less wasted CPU cycles.
docs/coding-style.rst | 8
2 files changed, 4 insertions(+), 40 deletions(-)
diff --git a/docs/coding-style.rst b/docs/coding-style.rst
index 942caf4e09..44e5265a60 100644
--- a/docs/coding-style.rst
+++ b/docs/coding-style.rst
@@ -258,15 +258,15 @@ comment, although use of a semicolon is not currently
rejected.
Curly braces
-Omit the curly braces around an ``if``, ``while``, ``for`` etc.
-body only when both that body and the condition itself occupy a
-single line. In every other case we require the braces. This
+Curly braces around an ``if``, ``while``, ``for`` etc. can be omitted if the
+body and the condition itself occupy only a single line.
+In every other case we require the braces. This
ensures that it is trivially easy to identify a
single-\ *statement* loop: each has only one *line* in its body.
::
- while (expr) // single line body; {} is forbidden
+ while (expr) // single line body; {} is optional
single_line_stmt();
::
Reviewed-by: Ján Tomko
Jano
signature.asc
Description: PGP signature
Re: device compatibility interface for live migration with assigned devices
On Fri, 11 Sep 2020 08:56:00 +0800 Yan Zhao wrote: > On Thu, Sep 10, 2020 at 12:02:44PM -0600, Alex Williamson wrote: > > On Thu, 10 Sep 2020 13:50:11 +0100 > > Sean Mooney wrote: > > > > > On Thu, 2020-09-10 at 14:38 +0200, Cornelia Huck wrote: > > > > On Wed, 9 Sep 2020 10:13:09 +0800 > > > > Yan Zhao wrote: > > > > > > > > > > > still, I'd like to put it more explicitly to make ensure it's not > > > > > > > missed: > > > > > > > the reason we want to specify compatible_type as a trait and check > > > > > > > whether target compatible_type is the superset of source > > > > > > > compatible_type is for the consideration of backward > > > > > > > compatibility. > > > > > > > e.g. > > > > > > > an old generation device may have a mdev type xxx-v4-yyy, while a > > > > > > > newer > > > > > > > generation device may be of mdev type xxx-v5-yyy. > > > > > > > with the compatible_type traits, the old generation device is > > > > > > > still > > > > > > > able to be regarded as compatible to newer generation device even > > > > > > > their > > > > > > > mdev types are not equal. > > > > > > > > > > > > If you want to support migration from v4 to v5, can't the > > > > > > (presumably > > > > > > newer) driver that supports v5 simply register the v4 type as well, > > > > > > so > > > > > > that the mdev can be created as v4? (Just like QEMU versioned > > > > > > machine > > > > > > types work.) > > > > > > > > > > yes, it should work in some conditions. > > > > > but it may not be that good in some cases when v5 and v4 in the name > > > > > string > > > > > of mdev type identify hardware generation (e.g. v4 for gen8, and v5 > > > > > for > > > > > gen9) > > > > > > > > > > e.g. > > > > > (1). when src mdev type is v4 and target mdev type is v5 as > > > > > software does not support it initially, and v4 and v5 identify > > > > > hardware > > > > > differences. > > > > > > > > My first hunch here is: Don't introduce types that may be compatible > > > > later. Either make them compatible, or make them distinct by design, > > > > and possibly add a different, compatible type later. > > > > > > > > > then after software upgrade, v5 is now compatible to v4, should the > > > > > software now downgrade mdev type from v5 to v4? > > > > > not sure if moving hardware generation info into a separate attribute > > > > > from mdev type name is better. e.g. remove v4, v5 in mdev type, while > > > > > use > > > > > compatible_pci_ids to identify compatibility. > > > > > > > > If the generations are compatible, don't mention it in the mdev type. > > > > If they aren't, use distinct types, so that management software doesn't > > > > have to guess. At least that would be my naive approach here. > > > yep that is what i would prefer to see too. > > > > > > > > > > > > > > (2) name string of mdev type is composed by "driver_name + type_name". > > > > > in some devices, e.g. qat, different generations of devices are > > > > > binding to > > > > > drivers of different names, e.g. "qat-v4", "qat-v5". > > > > > then though type_name is equal, mdev type is not equal. e.g. > > > > > "qat-v4-type1", "qat-v5-type1". > > > > > > > > I guess that shows a shortcoming of that "driver_name + type_name" > > > > approach? Or maybe I'm just confused. > > > yes i really dont like haveing the version in the mdev-type name > > > i would stongly perfger just qat-type-1 wehere qat is just there as a way > > > of namespacing. > > > although symmetric-cryto, asymmetric-cryto and compression woudl be a > > > better name then type-1, type-2, type-3 if > > > that is what they would end up mapping too. e.g. qat-compression or > > > qat-aes is a much better name then type-1 > > > higher layers of software are unlikely to parse the mdev names but as a > > > human looking at them its much eaiser to > > > understand if the names are meaningful. the qat prefix i think is > > > important however to make sure that your mdev-types > > > dont colide with other vendeors mdev types. so i woudl encurage all > > > vendors to prefix there mdev types with etiher the > > > device name or the vendor. > > > > +1 to all this, the mdev type is meant to indicate a software > > compatible interface, if different hardware versions can be software > > compatible, then don't make the job of finding a compatible device > > harder. The full type is a combination of the vendor driver name plus > > the vendor provided type name specifically in order to provide a type > > namespace per vendor driver. That's done at the mdev core level. > > Thanks, > > hi Alex, > got it. so do you suggest that vendors use consistent driver name over > generations of devices? > for qat, they create different modules for each generation. This > practice is not good if they want to support migration between devices > of different generations, right? > > and can I understand that we don't want support of migration betwe
Re: [PATCH 02/17] virDomainStorageNetworkParseHosts: Switch to a more modern XML parsing approach
On a Friday in 2020, Peter Krempa wrote:
Use XPath to get the host list instead of iterating through the nodes.
Signed-off-by: Peter Krempa
---
src/conf/domain_conf.c | 31 +--
1 file changed, 17 insertions(+), 14 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index acbc3f1c1e..ae7cb1e1c5 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8234,23 +8234,26 @@ virDomainStorageNetworkParseHost(xmlNodePtr hostnode,
static int
virDomainStorageNetworkParseHosts(xmlNodePtr node,
+ xmlXPathContextPtr ctxt,
virStorageNetHostDefPtr *hosts,
size_t *nhosts)
{
-xmlNodePtr child;
+g_autofree xmlNodePtr *hostnodes = NULL;
+ssize_t nhostnodes;
+size_t i;
+VIR_XPATH_NODE_AUTORESTORE(ctxt);
Please drop the ending semicolon so that other declarations can be added
without triggering -Wdeclaration-after-statement.
It contains a pragma to suppress the -Wunused-variable warning (with
Clang, IIRC):
commit 8cc177fc5d2c1ac76b256bd8d104d894fa9845ec
util: xml: use pragma in VIR_XPATH_NODE_AUTORESTORE
And placing a semicolon after it creates an empty statement.
(This was the only way I found that VIR_XPATH_NODE_AUTORESTORE would not
be considered a statement and -Wdeclaration-after-statement could be
enabled with both GCC and Clang)
Reviewed-by: Ján Tomko
Jano
signature.asc
Description: PGP signature
Re: [PATCH 01/17] virDomainHostdevSubsysSCSIiSCSIClear: Inline contents into only caller
On a Friday in 2020, Peter Krempa wrote: There's just one caller for the function. Move the code into the caller. Signed-off-by: Peter Krempa --- src/conf/domain_conf.c | 19 +-- 1 file changed, 5 insertions(+), 14 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [libvirt PATCH 00/21] Remove VIR_{ALLOC, ALLOC_N, REALLOC_N}_QUIET macros.
On a Friday in 2020, Tim Wiederhake wrote: See also https://www.redhat.com/archives/libvir-list/2020-September/msg00540.html and https://www.redhat.com/archives/libvir-list/2020-September/msg00610.html Tim Wiederhake (21): util: Use glib memory functions in virErrorCopyNew util: Use glib memory functions in virLastErrorObject util: Remove VIR_ALLOC_QUIET tests: Use glib memory function in testConfRoundTrip util: Use glib memory functions in virJSONValueGetArrayAsBitmap util: Remove VIR_ALLOC_N_QUIET I have pushed all the paches except these ^^^ qemu: Use glib memory functions in qemuDomainMasterKeyReadFile qemu: Use glib memory functions in qemuDomainLogContextRead qemu: Use glib memory functions in qemuProcessReadLog These could have been squashed into one commit: qemu: remove use of VIR_REALLOC_N_QUIET Jano src/qemu/qemu_domain.c | 4 ++-- src/qemu/qemu_process.c | 2 +- src/rpc/virnetmessage.c | 4 ++-- src/util/viralloc.h | 44 - src/util/virbitmap.c| 10 ++ src/util/virdevmapper.c | 4 +--- src/util/virerror.c | 30 ++-- src/util/virfile.c | 15 +- src/util/virjson.c | 4 ++-- src/util/virlog.c | 24 -- src/util/virthread.c| 5 + tests/virconftest.c | 29 ++- tests/virpcimock.c | 24 -- tools/vsh.c | 10 -- 14 files changed, 61 insertions(+), 148 deletions(-) -- 2.26.2 signature.asc Description: PGP signature
Re: [PATCH 2/5] node_device: detect CSS devices
On 9/9/20 9:03 AM, Cornelia Huck wrote:
[snip]
+static int
+udevProcessCSS(struct udev_device *device,
+ virNodeDeviceDefPtr def)
+{
+/* do not process EADM and CHSC devices to keep the list sane */
+if (STREQ(def->driver, "eadm_subchannel") ||
+STREQ(def->driver, "chsc_subchannel"))
[2] Also mentions message subchannel, although apparently there are no Linux
kernel drivers for that yet, IIUC that one would have to be added here as well
as some point, is that correct?
We have never seen those, but we would want to add them here if they
have no relevance for the user.
I think you want to filter message subchannels as well, my assumption
is that libvirt will only care about I/O subchannels.
In
https://www.kernel.org/doc/html/latest/driver-api/s390-drivers.html
it is stated that for
Message subchannels. No Linux driver currently exists.
Therefore I do not know how I could currently filter message subchannels
out. Due you want me to reverse the logic and just filter out everything
but "io_subchannel" instead (which I would regard as fencing the unknown)?
--
Mit freundlichen Grüßen/Kind regards
Boris Fiuczynski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
Re: [libvirt PATCH 21/21] util: Remove VIR_REALLOC_N_QUIET
On a Friday in 2020, Tim Wiederhake wrote: Signed-off-by: Tim Wiederhake --- src/util/viralloc.h | 15 --- 1 file changed, 15 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [libvirt PATCH 20/21] util: Use glib memory functions in virFileGetXAttrQuiet
On a Friday in 2020, Tim Wiederhake wrote: Signed-off-by: Tim Wiederhake --- src/util/virfile.c | 15 +-- 1 file changed, 5 insertions(+), 10 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [libvirt PATCH 17/21] tests: Use glib memory functions in add_fd
On a Friday in 2020, Tim Wiederhake wrote: Signed-off-by: Tim Wiederhake --- tests/virpcimock.c | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [libvirt PATCH 18/21] tests: Use glib memory functions in pci_driver_new
On a Friday in 2020, Tim Wiederhake wrote: Signed-off-by: Tim Wiederhake --- tests/virpcimock.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [libvirt PATCH 14/21] qemu: Use glib memory functions in qemuDomainMasterKeyReadFile
On a Friday in 2020, Tim Wiederhake wrote: Signed-off-by: Tim Wiederhake --- src/qemu/qemu_domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [libvirt PATCH 19/21] tools: Use glib memory functions in vshCompleterFilter
On a Friday in 2020, Tim Wiederhake wrote: Signed-off-by: Tim Wiederhake --- tools/vsh.c | 10 -- 1 file changed, 4 insertions(+), 6 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [libvirt PATCH 13/21] util: Remove VIR_ALLOC_N_QUIET
On a Friday in 2020, Tim Wiederhake wrote: Signed-off-by: Tim Wiederhake --- src/util/viralloc.h | 15 --- 1 file changed, 15 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [libvirt PATCH 16/21] qemu: Use glib memory functions in qemuProcessReadLog
On a Friday in 2020, Tim Wiederhake wrote: Signed-off-by: Tim Wiederhake --- src/qemu/qemu_process.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [libvirt PATCH 15/21] qemu: Use glib memory functions in qemuDomainLogContextRead
On a Friday in 2020, Tim Wiederhake wrote: Signed-off-by: Tim Wiederhake --- src/qemu/qemu_domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [libvirt PATCH 12/21] util: Use glib memory functions in virJSONValueGetArrayAsBitmap
On a Friday in 2020, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake
---
src/util/virjson.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/util/virjson.c b/src/util/virjson.c
index 6921eccb60..6511b4e641 100644
--- a/src/util/virjson.c
+++ b/src/util/virjson.c
@@ -1243,8 +1243,8 @@ virJSONValueGetArrayAsBitmap(const virJSONValue *val,
if (val->type != VIR_JSON_TYPE_ARRAY)
return -1;
-if (VIR_ALLOC_N_QUIET(elems, val->data.array.nvalues) < 0)
-return -1;
+/* elems might be NULL if val->data.array.nvalues is 0 */
Is this worthy of commenting? Even the calloc man page says this can
happen.
+elems = g_new0(unsigned long long, val->data.array.nvalues);
/* first pass converts array members to numbers and finds the maximum */
for (i = 0; i < val->data.array.nvalues; i++) {
--
2.26.2
Without the comment:
Reviewed-by: Ján Tomko
Jano
signature.asc
Description: PGP signature
Re: [libvirt PATCH 10/21] tests: Use glib memory function in testConfRoundTrip
On a Friday in 2020, Tim Wiederhake wrote:
This also fixes a (hypothetical) bug where testConfRoundTrip would return 0 if
virConfWriteMem() returned 0 and virTestCompareToFile failed.
The bug is not hypothetical, it renders all the tests using testConfRoundTrip
useless since they always pass even if the functionality they're
supposed to be testing give a different output.
Since it's a real bug, I think it deserves a separate commit.
Signed-off-by: Tim Wiederhake
---
tests/virconftest.c | 29 ++---
1 file changed, 10 insertions(+), 19 deletions(-)
diff --git a/tests/virconftest.c b/tests/virconftest.c
index ab29b5b712..c683344f49 100644
--- a/tests/virconftest.c
+++ b/tests/virconftest.c
@@ -32,40 +32,31 @@
static int testConfRoundTrip(const void *opaque)
{
const char *name = opaque;
-int ret = -1;
g_autoptr(virConf) conf = NULL;
int len = 1;
-char *buffer = NULL;
-char *srcfile = NULL;
-char *dstfile = NULL;
+g_autofree char *buffer = NULL;
+g_autofree char *srcfile = NULL;
+g_autofree char *dstfile = NULL;
srcfile = g_strdup_printf("%s/virconfdata/%s.conf", abs_srcdir, name);
dstfile = g_strdup_printf("%s/virconfdata/%s.out", abs_srcdir, name);
-if (VIR_ALLOC_N_QUIET(buffer, len) < 0) {
-fprintf(stderr, "out of memory\n");
-goto cleanup;
-}
+buffer = g_new0(char, len);
conf = virConfReadFile(srcfile, 0);
if (conf == NULL) {
fprintf(stderr, "Failed to process %s\n", srcfile);
-goto cleanup;
+return -1;
}
-ret = virConfWriteMem(buffer, &len, conf);
-if (ret < 0) {
+
+if (virConfWriteMem(buffer, &len, conf) < 0) {
With this change separated.
Reviewed-by: Ján Tomko
Jano
fprintf(stderr, "Failed to serialize %s back\n", srcfile);
-goto cleanup;
+return -1;
}
if (virTestCompareToFile(buffer, dstfile) < 0)
-goto cleanup;
+return -1;
-ret = 0;
- cleanup:
-VIR_FREE(srcfile);
-VIR_FREE(dstfile);
-VIR_FREE(buffer);
-return ret;
+return 0;
}
--
2.26.2
signature.asc
Description: PGP signature
Re: [libvirt PATCH 11/21] util: Use glib memory functions in virDevMapperGetTargetsImpl
On a Friday in 2020, Tim Wiederhake wrote: Signed-off-by: Tim Wiederhake --- src/util/virdevmapper.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH 4/5] node_device: detect DASD devices
On 9/8/20 5:50 PM, Erik Skultety wrote:
On Mon, Aug 24, 2020 at 01:59:14PM +0200, Bjoern Walk wrote:
From: Boris Fiuczynski
Make Direct Access Storage Devices (DASDs) available in the node_device driver.
Reviewed-by: Bjoern Walk
Signed-off-by: Boris Fiuczynski
---
src/node_device/node_device_udev.c | 24
1 file changed, 24 insertions(+)
diff --git a/src/node_device/node_device_udev.c
b/src/node_device/node_device_udev.c
index 1c4df709..5f9d67cc 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -871,6 +871,19 @@ udevProcessSD(struct udev_device *device,
}
+static int
+udevProcessDasd(struct udev_device *device,
+virNodeDeviceDefPtr def)
s/Dasd/DASD
Changed it
+{
+virNodeDevCapStoragePtr storage = &def->caps->data.storage;
+
+if (udevGetStringSysfsAttr(device, "device/uid", &storage->serial) < 0)
+return -1;
+
+return udevProcessDisk(device, def);
+}
+
+
/* This function exists to deal with the case in which a driver does
* not provide a device type in the usual place, but udev told us it's
* a storage device, and we can make a good guess at what kind of
@@ -891,6 +904,15 @@ udevKludgeStorageType(virNodeDeviceDefPtr def)
def->sysfs_path);
return 0;
}
+/* dasd disk */
+if (STRPREFIX(def->caps->data.storage.block, "/dev/dasd")) {
+def->caps->data.storage.drive_type = g_strdup("dasd");
+VIR_DEBUG("Found storage type '%s' for device "
+ "with sysfs path '%s'",
+ def->caps->data.storage.drive_type,
+ def->sysfs_path);
I understand why we would need it for /dev/vdX, but can udev not know the
drive_type from kernel? IOW Do we really need ^this hunk?
For DASDs there are currently no identifies in udev besides ID_PATH.
ID_TYPE=disk does not exist. That's why the DASDs fall through the
udevProcessStorage detection logic. Without this hunk the dasd devices
are being detected as storage devices but than end up as "Unsupported
storage type".
The short answer is yes. :-)
+return 0;
+}
VIR_DEBUG("Could not determine storage type "
"for device with sysfs path '%s'", def->sysfs_path);
return -1;
@@ -978,6 +1000,8 @@ udevProcessStorage(struct udev_device *device,
ret = udevProcessFloppy(device, def);
} else if (STREQ(def->caps->data.storage.drive_type, "sd")) {
ret = udevProcessSD(device, def);
+} else if (STREQ(def->caps->data.storage.drive_type, "dasd")) {
+ret = udevProcessDasd(device, def);
} else {
VIR_DEBUG("Unsupported storage type '%s'",
def->caps->data.storage.drive_type);
The rest looks good:
Reviewed-by: Erik Skultety
Thanks
--
Mit freundlichen Grüßen/Kind regards
Boris Fiuczynski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
Re: [libvirt PATCH 09/21] util: Remove VIR_ALLOC_QUIET
On a Friday in 2020, Tim Wiederhake wrote: Signed-off-by: Tim Wiederhake --- src/util/viralloc.h | 14 -- 1 file changed, 14 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [libvirt PATCH 07/21] util: Use glib memory functions in virLogFilterNew
On a Friday in 2020, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake
---
src/util/virlog.c | 24
1 file changed, 8 insertions(+), 16 deletions(-)
diff --git a/src/util/virlog.c b/src/util/virlog.c
index f6d0c6c050..de36da1a4a 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -1308,7 +1308,6 @@ virLogFilterNew(const char *match,
virLogPriority priority)
{
virLogFilterPtr ret = NULL;
-char *mdup = NULL;
size_t mlen = strlen(match);
if (priority < VIR_LOG_DEBUG || priority > VIR_LOG_ERROR) {
@@ -1317,23 +1316,16 @@ virLogFilterNew(const char *match,
return NULL;
}
+ret = g_new0(virLogFilter, 1);
+ret->priority = priority;
+
/* We must treat 'foo' as equiv to '*foo*' for g_pattern_match
- * todo substring matches, so add 2 extra bytes
+ * substring matches, so add 2 extra bytes
*/
Unrelated comment change.
-if (VIR_ALLOC_N_QUIET(mdup, mlen + 3) < 0)
-return NULL;
-
-mdup[0] = '*';
-memcpy(mdup + 1, match, mlen);
-mdup[mlen + 1] = '*';
-
-if (VIR_ALLOC_QUIET(ret) < 0) {
-VIR_FREE(mdup);
-return NULL;
-}
-
-ret->match = mdup;
-ret->priority = priority;
+ret->match = g_new0(char, mlen + 3);
+ret->match[0] = '*';
+memcpy(ret->match + 1, match, mlen);
+ret->match[mlen + 1] = '*';
return ret;
}
With the comment change removed:
Reviewed-by: Ján Tomko
Jano
signature.asc
Description: PGP signature
Re: [libvirt PATCH 08/21] util: Use glib memory functions in virThreadCreateFull
On a Friday in 2020, Tim Wiederhake wrote: Signed-off-by: Tim Wiederhake --- src/util/virthread.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [libvirt PATCH 06/21] util: Use glib memory functions in virSaveLastError
On a Friday in 2020, Tim Wiederhake wrote: Signed-off-by: Tim Wiederhake --- src/util/virerror.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [libvirt PATCH 05/21] util: Use glib memory functions in virLastErrorObject
On a Friday in 2020, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake
---
src/util/virerror.c | 18 ++
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/src/util/virerror.c b/src/util/virerror.c
index 4ba99defbb..99a0855b51 100644
--- a/src/util/virerror.c
+++ b/src/util/virerror.c
@@ -237,15 +237,17 @@ virErrorCopyNew(virErrorPtr err)
static virErrorPtr
virLastErrorObject(void)
{
-virErrorPtr err;
+g_autoptr(virError) err = NULL;
+
This function would also look nicer without g_auto - most of the code
paths don't want to free err.
Jano
err = virThreadLocalGet(&virLastErr);
-if (!err) {
-if (VIR_ALLOC_QUIET(err) < 0)
-return NULL;
-if (virThreadLocalSet(&virLastErr, err) < 0)
-VIR_FREE(err);
-}
-return err;
+if (err)
+return g_steal_pointer(&err);
+
+err = g_new0(virError, 1);
+if (virThreadLocalSet(&virLastErr, err) < 0)
+return NULL;
+
+return g_steal_pointer(&err);
}
--
2.26.2
signature.asc
Description: PGP signature
Re: [libvirt PATCH 04/21] util: Use glib memory functions in virErrorCopyNew
On a Friday in 2020, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake
---
src/util/virerror.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/src/util/virerror.c b/src/util/virerror.c
index 507a29f50f..4ba99defbb 100644
--- a/src/util/virerror.c
+++ b/src/util/virerror.c
@@ -223,15 +223,14 @@ virCopyError(virErrorPtr from,
virErrorPtr
virErrorCopyNew(virErrorPtr err)
{
-virErrorPtr ret;
+g_autoptr(virError) ret = NULL;
Using g_auto is not mandatory, sometimes it adds more
clutter to the code than it removes.
-if (VIR_ALLOC_QUIET(ret) < 0)
-return NULL;
+ret = g_new0(virError, 1);
if (virCopyError(err, ret) < 0)
-VIR_FREE(ret);
As of commit 18f377178a3bee6df4c63283ff61b75c21bc0d52 virCopyError
always returns 0, so this condition is pointless.
virCopyError(err, ret);
should be enough and it would remove the need for g_auto.
Jano
+return NULL;
-return ret;
+return g_steal_pointer(&ret);
}
--
2.26.2
signature.asc
Description: PGP signature
Re: [PATCH 5/5] node_device: mdev vfio-ccw support
On 9/8/20 6:01 PM, Erik Skultety wrote:
@@ -664,11 +695,11 @@ nodeDeviceGetMdevctlStartCommand(virNodeDeviceDefPtr def,
{
virCommandPtr cmd;
g_autofree char *json = NULL;
-g_autofree char *parent_pci = nodeDeviceFindAddressByName(def->parent);
+g_autofree char *parent_addr = nodeDeviceFindAddressByName(def->parent);
-if (!parent_pci) {
+if (!parent_addr) {
virReportError(VIR_ERR_NO_NODE_DEVICE,
- _("unable to find PCI address for parent device '%s'"),
def->parent);
+ _("unable to find address for parent device '%s'"),
def->parent);
I'm wondering whether "unable to find parent device '%s'" would not suffice,
since we're not specifying what type of address we were not able to find - I'm
not even sure the address information is important at all.
Erik
Erik,
how about
_("unable to find parent device '%s' by its address"), def->parent);
just to indicate the search criteria but I could also agree to a simple
_("unable to find parent device '%s'"), def->parent);
Your choice.
--
Mit freundlichen Grüßen/Kind regards
Boris Fiuczynski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
Re: [libvirt PATCH 03/21] util: Use glib memory functions in virBitmapNewQuiet
On a Friday in 2020, Tim Wiederhake wrote: Signed-off-by: Tim Wiederhake --- src/util/virbitmap.c | 10 ++ 1 file changed, 2 insertions(+), 8 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [libvirt PATCH 02/21] rpc: Use glib memory functions in virNetMessageSaveError
On a Friday in 2020, Tim Wiederhake wrote: Signed-off-by: Tim Wiederhake --- src/rpc/virnetmessage.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [libvirt PATCH 01/21] tests: Use glib memory functions in virpcimock.c
On a Friday in 2020, Tim Wiederhake wrote: Signed-off-by: Tim Wiederhake --- tests/virpcimock.c | 10 +++--- 1 file changed, 3 insertions(+), 7 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH] smp: drop support for deprecated (invalid topologies)
On Fri, Sep 11, 2020 at 09:32:02AM -0400, Igor Mammedov wrote:
> it's was deprecated since 3.1
>
> Support for invalid topologies is removed, the user must ensure
> that topologies described with -smp include all possible cpus,
> i.e. (sockets * cores * threads) == maxcpus or QEMU will
> exit with error.
>
> Signed-off-by: Igor Mammedov
Acked-by:
memory tree I guess?
> ---
> docs/system/deprecated.rst | 26 +-
> hw/core/machine.c | 16
> hw/i386/pc.c | 16
> 3 files changed, 21 insertions(+), 37 deletions(-)
>
> diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
> index 122717cfee..d737728fab 100644
> --- a/docs/system/deprecated.rst
> +++ b/docs/system/deprecated.rst
> @@ -47,19 +47,6 @@ The 'file' driver for drives is no longer appropriate for
> character or host
> devices and will only accept regular files (S_IFREG). The correct driver
> for these file types is 'host_cdrom' or 'host_device' as appropriate.
>
> -``-smp`` (invalid topologies) (since 3.1)
> -'
> -
> -CPU topology properties should describe whole machine topology including
> -possible CPUs.
> -
> -However, historically it was possible to start QEMU with an incorrect
> topology
> -where *n* <= *sockets* * *cores* * *threads* < *maxcpus*,
> -which could lead to an incorrect topology enumeration by the guest.
> -Support for invalid topologies will be removed, the user must ensure
> -topologies described with -smp include all possible cpus, i.e.
> -*sockets* * *cores* * *threads* = *maxcpus*.
> -
> ``-vnc acl`` (since 4.0.0)
> ''
>
> @@ -618,6 +605,19 @@ New machine versions (since 5.1) will not accept the
> option but it will still
> work with old machine types. User can check the QAPI schema to see if the
> legacy
> option is supported by looking at MachineInfo::numa-mem-supported property.
>
> +``-smp`` (invalid topologies) (removed 5.2)
> +'''
> +
> +CPU topology properties should describe whole machine topology including
> +possible CPUs.
> +
> +However, historically it was possible to start QEMU with an incorrect
> topology
> +where *n* <= *sockets* * *cores* * *threads* < *maxcpus*,
> +which could lead to an incorrect topology enumeration by the guest.
> +Support for invalid topologies is removed, the user must ensure
> +topologies described with -smp include all possible cpus, i.e.
> +*sockets* * *cores* * *threads* = *maxcpus*.
> +
> Block devices
> -
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index ea26d61237..09aee4ea52 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -754,23 +754,15 @@ static void smp_parse(MachineState *ms, QemuOpts *opts)
> exit(1);
> }
>
> -if (sockets * cores * threads > ms->smp.max_cpus) {
> -error_report("cpu topology: "
> - "sockets (%u) * cores (%u) * threads (%u) > "
> - "maxcpus (%u)",
> +if (sockets * cores * threads != ms->smp.max_cpus) {
> +error_report("Invalid CPU topology: "
> + "sockets (%u) * cores (%u) * threads (%u) "
> + "!= maxcpus (%u)",
> sockets, cores, threads,
> ms->smp.max_cpus);
> exit(1);
> }
>
> -if (sockets * cores * threads != ms->smp.max_cpus) {
> -warn_report("Invalid CPU topology deprecated: "
> -"sockets (%u) * cores (%u) * threads (%u) "
> -"!= maxcpus (%u)",
> -sockets, cores, threads,
> -ms->smp.max_cpus);
> -}
> -
> ms->smp.cpus = cpus;
> ms->smp.cores = cores;
> ms->smp.threads = threads;
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index d071da787b..fbde6b04e6 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -746,23 +746,15 @@ void pc_smp_parse(MachineState *ms, QemuOpts *opts)
> exit(1);
> }
>
> -if (sockets * dies * cores * threads > ms->smp.max_cpus) {
> -error_report("cpu topology: "
> - "sockets (%u) * dies (%u) * cores (%u) * threads
> (%u) > "
> - "maxcpus (%u)",
> +if (sockets * dies * cores * threads != ms->smp.max_cpus) {
> +error_report("Invalid CPU topology deprecated: "
> + "sockets (%u) * dies (%u) * cores (%u) * threads
> (%u) "
> + "!= maxcpus (%u)",
> sockets, dies, cores, threads,
> ms->smp.max_cpus);
> exit(1);
> }
>
> -if (sockets * dies * cores * threads != ms->smp.max_cpus) {
> -warn_report("Invalid CPU topology deprecated: "
> -
Re: [PATCH] cphp: remove deprecated cpu-add command(s)
* Igor Mammedov (imamm...@redhat.com) wrote:
> theses were deprecatedince since 4.0, remove both HMP and QMP variants.
>
> Users should use device_add commnad instead. To get list of
> possible CPUs and options, use 'info hotpluggable-cpus' HMP
> or query-hotpluggable-cpus QMP command.
>
> Signed-off-by: Igor Mammedov
> ---
> include/hw/boards.h | 1 -
> include/hw/i386/pc.h| 1 -
> include/monitor/hmp.h | 1 -
> docs/system/deprecated.rst | 25 +
> hmp-commands.hx | 15 --
> hw/core/machine-hmp-cmds.c | 12 -
> hw/core/machine-qmp-cmds.c | 12 -
> hw/i386/pc.c| 27 --
> hw/i386/pc_piix.c | 1 -
> hw/s390x/s390-virtio-ccw.c | 12 -
> qapi/machine.json | 24 -
> tests/qtest/cpu-plug-test.c | 100
> tests/qtest/test-hmp.c | 1 -
> 13 files changed, 21 insertions(+), 211 deletions(-)
>
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index bc5b82ad20..2163843bdb 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -173,7 +173,6 @@ struct MachineClass {
> void (*init)(MachineState *state);
> void (*reset)(MachineState *state);
> void (*wakeup)(MachineState *state);
> -void (*hot_add_cpu)(MachineState *state, const int64_t id, Error **errp);
> int (*kvm_type)(MachineState *machine, const char *arg);
> void (*smp_parse)(MachineState *ms, QemuOpts *opts);
>
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index fe52e165b2..ca8ff6cd27 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -137,7 +137,6 @@ extern int fd_bootchk;
>
> void pc_acpi_smi_interrupt(void *opaque, int irq, int level);
>
> -void pc_hot_add_cpu(MachineState *ms, const int64_t id, Error **errp);
> void pc_smp_parse(MachineState *ms, QemuOpts *opts);
>
> void pc_guest_info_init(PCMachineState *pcms);
> diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
> index c986cfd28b..642e9e91f9 100644
> --- a/include/monitor/hmp.h
> +++ b/include/monitor/hmp.h
> @@ -89,7 +89,6 @@ void hmp_chardev_add(Monitor *mon, const QDict *qdict);
> void hmp_chardev_change(Monitor *mon, const QDict *qdict);
> void hmp_chardev_remove(Monitor *mon, const QDict *qdict);
> void hmp_chardev_send_break(Monitor *mon, const QDict *qdict);
> -void hmp_cpu_add(Monitor *mon, const QDict *qdict);
> void hmp_object_add(Monitor *mon, const QDict *qdict);
> void hmp_object_del(Monitor *mon, const QDict *qdict);
> void hmp_info_memdev(Monitor *mon, const QDict *qdict);
> diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
> index 851dbdeb8a..122717cfee 100644
> --- a/docs/system/deprecated.rst
> +++ b/docs/system/deprecated.rst
> @@ -284,13 +284,6 @@ The ``query-cpus`` command is replaced by the
> ``query-cpus-fast`` command.
> The ``arch`` output member of the ``query-cpus-fast`` command is
> replaced by the ``target`` output member.
>
> -``cpu-add`` (since 4.0)
> -'''
> -
> -Use ``device_add`` for hotplugging vCPUs instead of ``cpu-add``. See
> -documentation of ``query-hotpluggable-cpus`` for additional
> -details.
> -
> ``query-events`` (since 4.0)
>
>
> @@ -306,12 +299,6 @@ the 'wait' field, which is only applicable to sockets in
> server mode
> Human Monitor Protocol (HMP) commands
> -
>
> -``cpu-add`` (since 4.0)
> -'''
> -
> -Use ``device_add`` for hotplugging vCPUs instead of ``cpu-add``. See
> -documentation of ``query-hotpluggable-cpus`` for additional details.
> -
> ``acl_show``, ``acl_reset``, ``acl_policy``, ``acl_add``, ``acl_remove``
> (since 4.0.0)
>
> ''
>
> @@ -514,6 +501,12 @@ QEMU Machine Protocol (QMP) commands
> The "autoload" parameter has been ignored since 2.12.0. All bitmaps
> are automatically loaded from qcow2 images.
>
> +``cpu-add`` (removed in 5.2)
> +
> +
> +Use ``device_add`` for hotplugging vCPUs instead of ``cpu-add``. See
> +documentation of ``query-hotpluggable-cpus`` for additional details.
> +
> Human Monitor Protocol (HMP) commands
> -
>
> @@ -523,6 +516,12 @@ The ``hub_id`` parameter of ``hostfwd_add`` /
> ``hostfwd_remove`` (removed in 5.0
> The ``[hub_id name]`` parameter tuple of the 'hostfwd_add' and
> 'hostfwd_remove' HMP commands has been replaced by ``netdev_id``.
>
> +``cpu-add`` (removed in 5.2)
> +
> +
> +Use ``device_add`` for hotplugging vCPUs instead of ``cpu-add``. See
> +documentation of ``query-hotpluggable-cpus`` for additional details.
> +
> Guest Emulator ISAs
> ---
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 60f395c276..d1e3e0e1c6 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands
[PATCH 00/17] qemu: Fix startup of VM with SCSI hostdev with long user-provided alias
Device alias was used to generate the backend nodename. This doesn't
work well if somebody specifies a very long useralias since qemu limits
nodename to 32 bytes. Stop basing the nodename on the alias.
Peter Krempa (17):
virDomainHostdevSubsysSCSIiSCSIClear: Inline contents into only caller
virDomainStorageNetworkParseHosts: Switch to a more modern XML parsing
approach
virDomainHostdevSubsysSCSIHostDefParseXML: Switch to a more modern XML
parsing approach
syntax-check: Don't forbid curly braces around single line condition
body
qemuxml2argvtest: hostdev-scsi-virtio-scsi: Add hostdev with useralias
conf: Add virStorageSource member for SCSI host device config data
tests: qemustatusxml2xmldata: Rename 'disk-secinfo-upgrade' case to
'upgrade'
tests: qemustatusxml2xmldata: Add local SCSI hostdev to 'upgrade' case
qemu: domain: Fill in (i)SCSI backend nodename if it is not present in
status XML
qemuBuildHostdevSCSI(A|De)tachPrepare: Use virStorageSource in def for
SCSI hostdevs
qemuBlockStorageSourceAttachData: remove 'storageNodeNameCopy'
qemu: domain: Extract preparation of hostdev specific data to a
separate function
qemuDomainSecretHostdevPrepare: remove
qemuDomainPrepareHostdev: Allocate backend nodenames in the prepare
function
qemuDomainPrepareHostdev: base hostdev secret object names on backend
alias
qemuDomainPrepareHostdev: Don't base backend nodename on device alias
qemuxml2argvtest: hostdev-scsi-virtio-scsi: Use longer user-alias for
SCSI hostdev
build-aux/check-spacing.pl| 36
docs/coding-style.rst | 8 +-
src/conf/domain_conf.c| 170 -
src/conf/domain_conf.h| 1 +
src/qemu/qemu_block.c | 1 -
src/qemu/qemu_block.h | 1 -
src/qemu/qemu_command.c | 74 +---
src/qemu/qemu_domain.c| 176 --
src/qemu/qemu_domain.h| 8 +-
src/qemu/qemu_hotplug.c | 2 +-
src/qemu/qemu_process.c | 21 +++
tests/qemustatusxml2xmldata/modern-in.xml | 1 +
...-secinfo-upgrade-in.xml => upgrade-in.xml} | 9 +
...ecinfo-upgrade-out.xml => upgrade-out.xml} | 20 ++
.../hostdev-scsi-lsi.x86_64-latest.args | 38 ++--
...hostdev-scsi-virtio-scsi.x86_64-2.8.0.args | 5 +
...hostdev-scsi-virtio-scsi.x86_64-4.1.0.args | 5 +
...ostdev-scsi-virtio-scsi.x86_64-latest.args | 36 ++--
.../hostdev-scsi-virtio-scsi.xml | 8 +
.../hostdev-scsi-virtio-scsi.xml | 8 +
tests/qemuxml2xmltest.c | 2 +-
21 files changed, 370 insertions(+), 260 deletions(-)
rename tests/qemustatusxml2xmldata/{disk-secinfo-upgrade-in.xml =>
upgrade-in.xml} (98%)
rename tests/qemustatusxml2xmldata/{disk-secinfo-upgrade-out.xml =>
upgrade-out.xml} (96%)
--
2.26.2
[PATCH 11/17] qemuBlockStorageSourceAttachData: remove 'storageNodeNameCopy'
This was a hack when we were locally regenerating the nodename so that
it's not leaked. Now that we use proper virStorageSource with
persistence it's no longer required.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_block.c | 1 -
src/qemu/qemu_block.h | 1 -
2 files changed, 2 deletions(-)
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 9095986f73..487b0a72e7 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -1557,7 +1557,6 @@
qemuBlockStorageSourceAttachDataFree(qemuBlockStorageSourceAttachDataPtr data)
virJSONValueFree(data->encryptsecretProps);
virJSONValueFree(data->tlsProps);
virJSONValueFree(data->tlsKeySecretProps);
-VIR_FREE(data->storageNodeNameCopy);
VIR_FREE(data->tlsAlias);
VIR_FREE(data->tlsKeySecretAlias);
VIR_FREE(data->authsecretAlias);
diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h
index 9aab620947..0701fc18d1 100644
--- a/src/qemu/qemu_block.h
+++ b/src/qemu/qemu_block.h
@@ -85,7 +85,6 @@ struct qemuBlockStorageSourceAttachData {
virJSONValuePtr storageProps;
const char *storageNodeName;
-char *storageNodeNameCopy; /* in some cases we don't have the
corresponding storage source */
bool storageAttached;
virJSONValuePtr storageSliceProps;
--
2.26.2
[PATCH 14/17] qemuDomainPrepareHostdev: Allocate backend nodenames in the prepare function
Allocate the nodename in the setup function rather than in the command
line generator.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_command.c | 7 ---
src/qemu/qemu_domain.c | 8
2 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index d92b967419..c5c587b97d 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5105,9 +5105,11 @@ qemuBuildHostdevSCSIAttachPrepare(virDomainHostdevDefPtr
hostdev,
switch ((virDomainHostdevSCSIProtocolType) scsisrc->protocol) {
case VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_NONE:
-if (!scsisrc->u.host.src &&
-!(scsisrc->u.host.src = virStorageSourceNew()))
+if (!scsisrc->u.host.src) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("SCSI host device data structure was not
initialized"));
return NULL;
+}
if (!(devstr = qemuBuildSCSIHostHostdevDrvStr(hostdev)))
return NULL;
@@ -5130,7 +5132,6 @@ qemuBuildHostdevSCSIAttachPrepare(virDomainHostdevDefPtr
hostdev,
return NULL;
}
-src->nodestorage = g_strdup_printf("libvirt-%s-backend",
hostdev->info->alias);
ret->storageNodeName = src->nodestorage;
*backendAlias = src->nodestorage;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index a751d0c205..d570ba892b 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -10409,6 +10409,10 @@ qemuDomainPrepareHostdev(virDomainHostdevDefPtr
hostdev,
switch ((virDomainHostdevSCSIProtocolType) scsisrc->protocol) {
case VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_NONE:
+virObjectUnref(scsisrc->u.host.src);
+if (!(scsisrc->u.host.src = virStorageSourceNew()))
+return -1;
+src = scsisrc->u.host.src;
break;
case VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI:
@@ -10422,6 +10426,10 @@ qemuDomainPrepareHostdev(virDomainHostdevDefPtr
hostdev,
}
if (src) {
+if (virQEMUCapsGet(priv->qemuCaps,
QEMU_CAPS_BLOCKDEV_HOSTDEV_SCSI)) {
+src->nodestorage = g_strdup_printf("libvirt-%s-backend",
hostdev->info->alias);
+}
+
if (src->auth) {
bool iscsiHasPS = virQEMUCapsGet(priv->qemuCaps,
QEMU_CAPS_ISCSI_PASSWORD_SECRET);
virSecretUsageType usageType = VIR_SECRET_USAGE_TYPE_ISCSI;
--
2.26.2
[PATCH 07/17] tests: qemustatusxml2xmldata: Rename 'disk-secinfo-upgrade' case to 'upgrade'
The test case tests other things besides disk secinfos, so we can make
it more universal.
Signed-off-by: Peter Krempa
---
.../{disk-secinfo-upgrade-in.xml => upgrade-in.xml} | 0
.../{disk-secinfo-upgrade-out.xml => upgrade-out.xml} | 0
tests/qemuxml2xmltest.c | 2 +-
3 files changed, 1 insertion(+), 1 deletion(-)
rename tests/qemustatusxml2xmldata/{disk-secinfo-upgrade-in.xml =>
upgrade-in.xml} (100%)
rename tests/qemustatusxml2xmldata/{disk-secinfo-upgrade-out.xml =>
upgrade-out.xml} (100%)
diff --git a/tests/qemustatusxml2xmldata/disk-secinfo-upgrade-in.xml
b/tests/qemustatusxml2xmldata/upgrade-in.xml
similarity index 100%
rename from tests/qemustatusxml2xmldata/disk-secinfo-upgrade-in.xml
rename to tests/qemustatusxml2xmldata/upgrade-in.xml
diff --git a/tests/qemustatusxml2xmldata/disk-secinfo-upgrade-out.xml
b/tests/qemustatusxml2xmldata/upgrade-out.xml
similarity index 100%
rename from tests/qemustatusxml2xmldata/disk-secinfo-upgrade-out.xml
rename to tests/qemustatusxml2xmldata/upgrade-out.xml
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 39a9da874f..2bf8dd5b14 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -1435,7 +1435,7 @@ mymain(void)
DO_TEST_STATUS("migration-in-params");
DO_TEST_STATUS("migration-out-params");
DO_TEST_STATUS("migration-out-nbd-tls");
-DO_TEST_STATUS("disk-secinfo-upgrade");
+DO_TEST_STATUS("upgrade");
DO_TEST_STATUS("blockjob-blockdev");
--
2.26.2
[PATCH 10/17] qemuBuildHostdevSCSI(A|De)tachPrepare: Use virStorageSource in def for SCSI hostdevs
Modify the attach/detach data generators to actually use the
virStorageSourceStructure embedded in the SCSI config data rather than
creating an ad-hoc internal one.
The modification will allow us to properly store the nodename used for
the backend in the status XML rather than re-generating it all the
time.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_command.c | 71 +++--
1 file changed, 47 insertions(+), 24 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index bd98b0a97c..d92b967419 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5053,24 +5053,35 @@
qemuBuildHostdevSCSIDetachPrepare(virDomainHostdevDefPtr hostdev,
virQEMUCapsPtr qemuCaps)
{
virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi;
-virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &scsisrc->u.iscsi;
g_autoptr(qemuBlockStorageSourceAttachData) ret =
g_new0(qemuBlockStorageSourceAttachData, 1);
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV_HOSTDEV_SCSI)) {
-if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) {
-qemuDomainStorageSourcePrivatePtr srcpriv =
QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(iscsisrc->src);
+virStorageSourcePtr src;
+qemuDomainStorageSourcePrivatePtr srcpriv;
-ret->storageNodeName = iscsisrc->src->nodestorage;
+switch ((virDomainHostdevSCSIProtocolType) scsisrc->protocol) {
+case VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_NONE:
+src = scsisrc->u.host.src;
+break;
-if (srcpriv && srcpriv->secinfo &&
-srcpriv->secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES)
-ret->authsecretAlias = g_strdup(srcpriv->secinfo->s.aes.alias);
-} else {
-ret->storageNodeNameCopy = g_strdup_printf("libvirt-%s-backend",
hostdev->info->alias);
-ret->storageNodeName = ret->storageNodeNameCopy;
+case VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI:
+src = scsisrc->u.iscsi.src;
+break;
+
+case VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_LAST:
+default:
+virReportEnumRangeError(virDomainHostdevSCSIProtocolType,
scsisrc->protocol);
+return NULL;
}
+srcpriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src);
+ret->storageNodeName = src->nodestorage;
ret->storageAttached = true;
+
+if (srcpriv && srcpriv->secinfo &&
+srcpriv->secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES)
+ret->authsecretAlias = g_strdup(srcpriv->secinfo->s.aes.alias);
+
} else {
ret->driveAlias = qemuAliasFromHostdev(hostdev);
ret->driveAdded = true;
@@ -5086,45 +5097,57 @@
qemuBuildHostdevSCSIAttachPrepare(virDomainHostdevDefPtr hostdev,
virQEMUCapsPtr qemuCaps)
{
virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi;
-virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &scsisrc->u.iscsi;
-virStorageSourcePtr src;
-g_autoptr(virStorageSource) localsrc = NULL;
g_autoptr(qemuBlockStorageSourceAttachData) ret =
g_new0(qemuBlockStorageSourceAttachData, 1);
+virStorageSourcePtr src = NULL;
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV_HOSTDEV_SCSI)) {
-if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) {
-src = iscsisrc->src;
-} else {
-g_autofree char *devstr = qemuBuildSCSIHostHostdevDrvStr(hostdev);
+g_autofree char *devstr = NULL;
-if (!devstr)
+switch ((virDomainHostdevSCSIProtocolType) scsisrc->protocol) {
+case VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_NONE:
+if (!scsisrc->u.host.src &&
+!(scsisrc->u.host.src = virStorageSourceNew()))
return NULL;
-if (!(src = localsrc = virStorageSourceNew()))
+if (!(devstr = qemuBuildSCSIHostHostdevDrvStr(hostdev)))
return NULL;
+src = scsisrc->u.host.src;
+
src->type = VIR_STORAGE_TYPE_BLOCK;
src->readonly = hostdev->readonly;
src->path = g_strdup_printf("/dev/%s", devstr);
+
+break;
+
+case VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI:
+src = scsisrc->u.iscsi.src;
+break;
+
+case VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_LAST:
+default:
+virReportEnumRangeError(virDomainHostdevSCSIProtocolType,
scsisrc->protocol);
+return NULL;
}
src->nodestorage = g_strdup_printf("libvirt-%s-backend",
hostdev->info->alias);
-ret->storageNodeNameCopy = g_strdup(src->nodestorage);
-ret->storageNodeName = ret->storageNodeNameCopy;
-*backendAlias = ret->storageNodeNameCopy;
+ret->storageNodeName = src->nodestorage;
+*backendAlias = src->nodest
[PATCH 05/17] qemuxml2argvtest: hostdev-scsi-virtio-scsi: Add hostdev with useralias
Add a SCSI host device with a user-specified alias to illustrate the
upcoming changes.
Signed-off-by: Peter Krempa
---
.../hostdev-scsi-virtio-scsi.x86_64-2.8.0.args| 3 +++
.../hostdev-scsi-virtio-scsi.x86_64-4.1.0.args| 3 +++
.../hostdev-scsi-virtio-scsi.x86_64-latest.args | 4
tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.xml | 8
tests/qemuxml2xmloutdata/hostdev-scsi-virtio-scsi.xml | 8
5 files changed, 26 insertions(+)
diff --git a/tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.x86_64-2.8.0.args
b/tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.x86_64-2.8.0.args
index c5a3c0ce61..07b7a5b113 100644
--- a/tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.x86_64-2.8.0.args
+++ b/tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.x86_64-2.8.0.args
@@ -37,6 +37,9 @@ drive=drive-hostdev0,id=hostdev0 \
-drive file=/dev/sg0,if=none,format=raw,id=drive-hostdev1,readonly=on \
-device scsi-generic,bus=scsi0.0,channel=0,scsi-id=4,lun=7,\
drive=drive-hostdev1,id=hostdev1 \
+-drive file=/dev/sg0,if=none,format=raw,id=drive-ua-test \
+-device
scsi-generic,bus=scsi0.0,channel=0,scsi-id=4,lun=6,drive=drive-ua-test,\
+id=ua-test \
-drive file=iscsi://example.org:3260/iqn.1992-01.com.example/0,if=none,\
format=raw,id=drive-hostdev2 \
-device scsi-generic,bus=scsi0.0,channel=0,scsi-id=2,lun=4,\
diff --git a/tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.x86_64-4.1.0.args
b/tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.x86_64-4.1.0.args
index f2591d6956..421edf90d0 100644
--- a/tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.x86_64-4.1.0.args
+++ b/tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.x86_64-4.1.0.args
@@ -36,6 +36,9 @@ drive=drive-hostdev0,id=hostdev0 \
-drive file=/dev/sg0,if=none,format=raw,id=drive-hostdev1,readonly=on \
-device scsi-generic,bus=scsi0.0,channel=0,scsi-id=4,lun=7,\
drive=drive-hostdev1,id=hostdev1 \
+-drive file=/dev/sg0,if=none,format=raw,id=drive-ua-test \
+-device
scsi-generic,bus=scsi0.0,channel=0,scsi-id=4,lun=6,drive=drive-ua-test,\
+id=ua-test \
-drive file.driver=iscsi,file.portal=example.org:3260,\
file.target=iqn.1992-01.com.example,file.lun=0,file.transport=tcp,if=none,\
format=raw,id=drive-hostdev2 \
diff --git a/tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.x86_64-latest.args
b/tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.x86_64-latest.args
index f86cbd7314..a2302d1089 100644
--- a/tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.x86_64-latest.args
@@ -42,6 +42,10 @@ drive=libvirt-hostdev0-backend,id=hostdev0 \
"node-name":"libvirt-hostdev1-backend","read-only":true}' \
-device scsi-generic,bus=scsi0.0,channel=0,scsi-id=4,lun=7,\
drive=libvirt-hostdev1-backend,id=hostdev1 \
+-blockdev '{"driver":"host_device","filename":"/dev/sg0",\
+"node-name":"libvirt-ua-test-backend","read-only":false}' \
+-device scsi-generic,bus=scsi0.0,channel=0,scsi-id=4,lun=6,\
+drive=libvirt-ua-test-backend,id=ua-test \
-blockdev '{"driver":"iscsi","portal":"example.org:3260",\
"target":"iqn.1992-01.com.example","lun":0,"transport":"tcp",\
"node-name":"libvirt-hostdev2-backend","read-only":false}' \
diff --git a/tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.xml
b/tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.xml
index f1caf80644..8da3fb1bfc 100644
--- a/tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.xml
+++ b/tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.xml
@@ -40,6 +40,14 @@
+
+
+
+
+
+
+
+
diff --git a/tests/qemuxml2xmloutdata/hostdev-scsi-virtio-scsi.xml
b/tests/qemuxml2xmloutdata/hostdev-scsi-virtio-scsi.xml
index 6c7e22d0c3..733d1d72a0 100644
--- a/tests/qemuxml2xmloutdata/hostdev-scsi-virtio-scsi.xml
+++ b/tests/qemuxml2xmloutdata/hostdev-scsi-virtio-scsi.xml
@@ -47,6 +47,14 @@
+
+
+
+
+
+
+
+
--
2.26.2
[PATCH 01/17] virDomainHostdevSubsysSCSIiSCSIClear: Inline contents into only caller
There's just one caller for the function. Move the code into the caller.
Signed-off-by: Peter Krempa
---
src/conf/domain_conf.c | 19 +--
1 file changed, 5 insertions(+), 14 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 72ac4f4191..acbc3f1c1e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3011,24 +3011,15 @@ virDomainHostdevDefNew(void)
}
-static void
-virDomainHostdevSubsysSCSIiSCSIClear(virDomainHostdevSubsysSCSIiSCSIPtr
iscsisrc)
-{
-if (!iscsisrc)
-return;
-
-virObjectUnref(iscsisrc->src);
-iscsisrc->src = NULL;
-}
-
-
static void
virDomainHostdevSubsysSCSIClear(virDomainHostdevSubsysSCSIPtr scsisrc)
{
-if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI)
-virDomainHostdevSubsysSCSIiSCSIClear(&scsisrc->u.iscsi);
-else
+if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) {
+virObjectUnref(scsisrc->u.iscsi.src);
+scsisrc->u.iscsi.src = NULL;
+} else {
VIR_FREE(scsisrc->u.host.adapter);
+}
}
--
2.26.2
[PATCH 02/17] virDomainStorageNetworkParseHosts: Switch to a more modern XML parsing approach
Use XPath to get the host list instead of iterating through the nodes.
Signed-off-by: Peter Krempa
---
src/conf/domain_conf.c | 31 +--
1 file changed, 17 insertions(+), 14 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index acbc3f1c1e..ae7cb1e1c5 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8234,23 +8234,26 @@ virDomainStorageNetworkParseHost(xmlNodePtr hostnode,
static int
virDomainStorageNetworkParseHosts(xmlNodePtr node,
+ xmlXPathContextPtr ctxt,
virStorageNetHostDefPtr *hosts,
size_t *nhosts)
{
-xmlNodePtr child;
+g_autofree xmlNodePtr *hostnodes = NULL;
+ssize_t nhostnodes;
+size_t i;
+VIR_XPATH_NODE_AUTORESTORE(ctxt);
-for (child = node->children; child; child = child->next) {
-if (child->type == XML_ELEMENT_NODE &&
-virXMLNodeNameEqual(child, "host")) {
-virStorageNetHostDef host;
+ctxt->node = node;
-if (virDomainStorageNetworkParseHost(child, &host) < 0)
-return -1;
-if (VIR_APPEND_ELEMENT(*hosts, *nhosts, host) < 0) {
-virStorageNetHostDefClear(&host);
-return -1;
-}
-}
+if ((nhostnodes = virXPathNodeSet("./host", ctxt, &hostnodes)) <= 0)
+return nhostnodes;
+
+*hosts = g_new0(virStorageNetHostDef, nhostnodes);
+*nhosts = nhostnodes;
+
+for (i = 0; i < nhostnodes; i++) {
+if (virDomainStorageNetworkParseHost(hostnodes[i], *hosts + i) < 0)
+return -1;
}
return 0;
@@ -8370,7 +8373,7 @@ virDomainHostdevSubsysSCSIiSCSIDefParseXML(xmlNodePtr
sourcenode,
return -1;
}
-if (virDomainStorageNetworkParseHosts(sourcenode, &iscsisrc->src->hosts,
+if (virDomainStorageNetworkParseHosts(sourcenode, ctxt,
&iscsisrc->src->hosts,
&iscsisrc->src->nhosts) < 0)
return -1;
@@ -9643,7 +9646,7 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node,
src->protocol == VIR_STORAGE_NET_PROTOCOL_HTTPS)
src->query = virXMLPropString(node, "query");
-if (virDomainStorageNetworkParseHosts(node, &src->hosts, &src->nhosts) < 0)
+if (virDomainStorageNetworkParseHosts(node, ctxt, &src->hosts,
&src->nhosts) < 0)
return -1;
virStorageSourceNetworkAssignDefaultPorts(src);
--
2.26.2
[PATCH 03/17] virDomainHostdevSubsysSCSIHostDefParseXML: Switch to a more modern XML parsing approach
Use XPath instead of iterating through the nodes.
Signed-off-by: Peter Krempa
---
src/conf/domain_conf.c | 98 +++---
1 file changed, 35 insertions(+), 63 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ae7cb1e1c5..05c2c63f58 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8262,89 +8262,61 @@ virDomainStorageNetworkParseHosts(xmlNodePtr node,
static int
virDomainHostdevSubsysSCSIHostDefParseXML(xmlNodePtr sourcenode,
+ xmlXPathContextPtr ctxt,
virDomainHostdevSubsysSCSIPtr
scsisrc)
{
-bool got_address = false, got_adapter = false;
-xmlNodePtr cur;
virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host;
g_autofree char *bus = NULL;
g_autofree char *target = NULL;
g_autofree char *unit = NULL;
+xmlNodePtr addressnode = NULL;
+VIR_XPATH_NODE_AUTORESTORE(ctxt);
-cur = sourcenode->children;
-while (cur != NULL) {
-if (cur->type == XML_ELEMENT_NODE) {
-if (virXMLNodeNameEqual(cur, "address")) {
-if (got_address) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
- _("more than one source addresses is "
- "specified for scsi hostdev"));
-return -1;
-}
-
-if (!(bus = virXMLPropString(cur, "bus")) ||
-!(target = virXMLPropString(cur, "target")) ||
-!(unit = virXMLPropString(cur, "unit"))) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
- _("'bus', 'target', and 'unit' must be
specified "
- "for scsi hostdev source address"));
-return -1;
-}
+ctxt->node = sourcenode;
-if (virStrToLong_uip(bus, NULL, 0, &scsihostsrc->bus) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _("cannot parse bus '%s'"), bus);
-return -1;
-}
+if (!(addressnode = virXPathNode("./address", ctxt))) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("'address' must be specified for scsi hostdev
source"));
+return -1;
+}
-if (virStrToLong_uip(target, NULL, 0,
-&scsihostsrc->target) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _("cannot parse target '%s'"), target);
-return -1;
-}
+if (!(bus = virXMLPropString(addressnode, "bus")) ||
+!(target = virXMLPropString(addressnode, "target")) ||
+!(unit = virXMLPropString(addressnode, "unit"))) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("'bus', 'target', and 'unit' must be specified "
+ "for scsi hostdev source address"));
+return -1;
+}
-if (virStrToLong_ullp(unit, NULL, 0, &scsihostsrc->unit) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _("cannot parse unit '%s'"), unit);
-return -1;
-}
+if (virStrToLong_uip(bus, NULL, 0, &scsihostsrc->bus) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("cannot parse bus '%s'"), bus);
+return -1;
+}
-got_address = true;
-} else if (virXMLNodeNameEqual(cur, "adapter")) {
-if (got_adapter) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
- _("more than one adapters is specified "
- "for scsi hostdev source"));
-return -1;
-}
-if (!(scsihostsrc->adapter = virXMLPropString(cur, "name"))) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
- _("'adapter' must be specified for scsi
hostdev source"));
-return -1;
-}
+if (virStrToLong_uip(target, NULL, 0, &scsihostsrc->target) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("cannot parse target '%s'"), target);
+return -1;
+}
-got_adapter = true;
-} else {
-virReportError(VIR_ERR_XML_ERROR,
- _("unsupported element '%s' of scsi hostdev
source"),
- cur->name);
-return -1;
-}
-}
-cur = cur->next;
+if (virStrToLong_ullp(unit, NULL, 0, &scsihostsrc->unit) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("cannot parse unit '%s'"), unit);
+
[PATCH 15/17] qemuDomainPrepareHostdev: base hostdev secret object names on backend alias
The secret object is used to pass data to the backend so it's better
fitting to base the secret object name on the SCSI host device backend
name.
Since we store the object alias in the status XML this modification is
safe in regards to existing guests.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_domain.c| 5 -
.../qemuxml2argvdata/hostdev-scsi-lsi.x86_64-latest.args | 8
.../hostdev-scsi-virtio-scsi.x86_64-latest.args | 8
3 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index d570ba892b..46f7caeb09 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -10426,8 +10426,11 @@ qemuDomainPrepareHostdev(virDomainHostdevDefPtr
hostdev,
}
if (src) {
+const char *backendalias = hostdev->info->alias;
+
if (virQEMUCapsGet(priv->qemuCaps,
QEMU_CAPS_BLOCKDEV_HOSTDEV_SCSI)) {
src->nodestorage = g_strdup_printf("libvirt-%s-backend",
hostdev->info->alias);
+backendalias = src->nodestorage;
}
if (src->auth) {
@@ -10441,7 +10444,7 @@ qemuDomainPrepareHostdev(virDomainHostdevDefPtr hostdev,
&src->auth->seclookupdef);
} else {
srcPriv->secinfo = qemuDomainSecretAESSetupFromSecret(priv,
-
hostdev->info->alias,
+
backendalias,
NULL,
usageType,
src->auth->username,
diff --git a/tests/qemuxml2argvdata/hostdev-scsi-lsi.x86_64-latest.args
b/tests/qemuxml2argvdata/hostdev-scsi-lsi.x86_64-latest.args
index d4599f6002..f768c2471b 100644
--- a/tests/qemuxml2argvdata/hostdev-scsi-lsi.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/hostdev-scsi-lsi.x86_64-latest.args
@@ -52,21 +52,21 @@ id=hostdev2 \
"node-name":"libvirt-hostdev3-backend","read-only":false}' \
-device scsi-generic,bus=scsi0.0,scsi-id=5,drive=libvirt-hostdev3-backend,\
id=hostdev3 \
--object secret,id=hostdev4-secret0,\
+-object secret,id=libvirt-hostdev4-backend-secret0,\
data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\
keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \
-blockdev '{"driver":"iscsi","portal":"example.org:3260",\
"target":"iqn.1992-01.com.example:storage","lun":1,"transport":"tcp",\
-"user":"myname","password-secret":"hostdev4-secret0",\
+"user":"myname","password-secret":"libvirt-hostdev4-backend-secret0",\
"node-name":"libvirt-hostdev4-backend","read-only":false}' \
-device scsi-generic,bus=scsi0.0,scsi-id=3,drive=libvirt-hostdev4-backend,\
id=hostdev4 \
--object secret,id=hostdev5-secret0,\
+-object secret,id=libvirt-hostdev5-backend-secret0,\
data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\
keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \
-blockdev '{"driver":"iscsi","portal":"example.org:3260",\
"target":"iqn.1992-01.com.example:storage","lun":2,"transport":"tcp",\
-"user":"myname","password-secret":"hostdev5-secret0",\
+"user":"myname","password-secret":"libvirt-hostdev5-backend-secret0",\
"node-name":"libvirt-hostdev5-backend","read-only":false}' \
-device scsi-generic,bus=scsi0.0,scsi-id=2,drive=libvirt-hostdev5-backend,\
id=hostdev5 \
diff --git a/tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.x86_64-latest.args
b/tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.x86_64-latest.args
index a2302d1089..0beefabd27 100644
--- a/tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.x86_64-latest.args
@@ -56,21 +56,21 @@ drive=libvirt-hostdev2-backend,id=hostdev2 \
"node-name":"libvirt-hostdev3-backend","read-only":false}' \
-device scsi-generic,bus=scsi0.0,channel=0,scsi-id=2,lun=5,\
drive=libvirt-hostdev3-backend,id=hostdev3 \
--object secret,id=hostdev4-secret0,\
+-object secret,id=libvirt-hostdev4-backend-secret0,\
data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\
keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \
-blockdev '{"driver":"iscsi","portal":"example.org:3260",\
"target":"iqn.1992-01.com.example:storage","lun":1,"transport":"tcp",\
-"user":"myname","password-secret":"hostdev4-secret0",\
+"user":"myname","password-secret":"libvirt-hostdev4-backend-secret0",\
"node-name":"libvirt-hostdev4-backend","read-only":false}' \
-device scsi-generic,bus=scsi0.0,channel=0,scsi-id=3,lun=4,\
drive=libvirt-hostdev4-backend,id=hostdev4 \
--object secret,id=hostdev5-secret0,\
+-object secret,id=libvirt-hostdev5-backend-secret0,\
data=9ea
[PATCH 17/17] qemuxml2argvtest: hostdev-scsi-virtio-scsi: Use longer user-alias for SCSI hostdev
Test that we can cope with a long useralias when generating SCSI hostdev
commandline.
Signed-off-by: Peter Krempa
---
.../hostdev-scsi-virtio-scsi.x86_64-2.8.0.args| 8 +---
.../hostdev-scsi-virtio-scsi.x86_64-4.1.0.args| 8 +---
.../hostdev-scsi-virtio-scsi.x86_64-latest.args | 2 +-
tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.xml | 2 +-
tests/qemuxml2xmloutdata/hostdev-scsi-virtio-scsi.xml | 2 +-
5 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.x86_64-2.8.0.args
b/tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.x86_64-2.8.0.args
index 07b7a5b113..3f007db2c3 100644
--- a/tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.x86_64-2.8.0.args
+++ b/tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.x86_64-2.8.0.args
@@ -37,9 +37,11 @@ drive=drive-hostdev0,id=hostdev0 \
-drive file=/dev/sg0,if=none,format=raw,id=drive-hostdev1,readonly=on \
-device scsi-generic,bus=scsi0.0,channel=0,scsi-id=4,lun=7,\
drive=drive-hostdev1,id=hostdev1 \
--drive file=/dev/sg0,if=none,format=raw,id=drive-ua-test \
--device
scsi-generic,bus=scsi0.0,channel=0,scsi-id=4,lun=6,drive=drive-ua-test,\
-id=ua-test \
+-drive file=/dev/sg0,if=none,format=raw,\
+id=drive-ua-7996c8dc-a4fa-4012-b76f-043d20144263 \
+-device scsi-generic,bus=scsi0.0,channel=0,scsi-id=4,lun=6,\
+drive=drive-ua-7996c8dc-a4fa-4012-b76f-043d20144263,\
+id=ua-7996c8dc-a4fa-4012-b76f-043d20144263 \
-drive file=iscsi://example.org:3260/iqn.1992-01.com.example/0,if=none,\
format=raw,id=drive-hostdev2 \
-device scsi-generic,bus=scsi0.0,channel=0,scsi-id=2,lun=4,\
diff --git a/tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.x86_64-4.1.0.args
b/tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.x86_64-4.1.0.args
index 421edf90d0..eb11d8f733 100644
--- a/tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.x86_64-4.1.0.args
+++ b/tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.x86_64-4.1.0.args
@@ -36,9 +36,11 @@ drive=drive-hostdev0,id=hostdev0 \
-drive file=/dev/sg0,if=none,format=raw,id=drive-hostdev1,readonly=on \
-device scsi-generic,bus=scsi0.0,channel=0,scsi-id=4,lun=7,\
drive=drive-hostdev1,id=hostdev1 \
--drive file=/dev/sg0,if=none,format=raw,id=drive-ua-test \
--device
scsi-generic,bus=scsi0.0,channel=0,scsi-id=4,lun=6,drive=drive-ua-test,\
-id=ua-test \
+-drive file=/dev/sg0,if=none,format=raw,\
+id=drive-ua-7996c8dc-a4fa-4012-b76f-043d20144263 \
+-device scsi-generic,bus=scsi0.0,channel=0,scsi-id=4,lun=6,\
+drive=drive-ua-7996c8dc-a4fa-4012-b76f-043d20144263,\
+id=ua-7996c8dc-a4fa-4012-b76f-043d20144263 \
-drive file.driver=iscsi,file.portal=example.org:3260,\
file.target=iqn.1992-01.com.example,file.lun=0,file.transport=tcp,if=none,\
format=raw,id=drive-hostdev2 \
diff --git a/tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.x86_64-latest.args
b/tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.x86_64-latest.args
index 147ab40c9a..47c3f09db5 100644
--- a/tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.x86_64-latest.args
@@ -45,7 +45,7 @@ drive=libvirt-3-backend,id=hostdev1 \
-blockdev '{"driver":"host_device","filename":"/dev/sg0",\
"node-name":"libvirt-4-backend","read-only":false}' \
-device scsi-generic,bus=scsi0.0,channel=0,scsi-id=4,lun=6,\
-drive=libvirt-4-backend,id=ua-test \
+drive=libvirt-4-backend,id=ua-7996c8dc-a4fa-4012-b76f-043d20144263 \
-blockdev '{"driver":"iscsi","portal":"example.org:3260",\
"target":"iqn.1992-01.com.example","lun":0,"transport":"tcp",\
"node-name":"libvirt-5-backend","read-only":false}' \
diff --git a/tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.xml
b/tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.xml
index 8da3fb1bfc..4903a75d13 100644
--- a/tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.xml
+++ b/tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.xml
@@ -45,7 +45,7 @@
-
+
diff --git a/tests/qemuxml2xmloutdata/hostdev-scsi-virtio-scsi.xml
b/tests/qemuxml2xmloutdata/hostdev-scsi-virtio-scsi.xml
index 733d1d72a0..3a2b10d815 100644
--- a/tests/qemuxml2xmloutdata/hostdev-scsi-virtio-scsi.xml
+++ b/tests/qemuxml2xmloutdata/hostdev-scsi-virtio-scsi.xml
@@ -52,7 +52,7 @@
-
+
--
2.26.2
[PATCH 13/17] qemuDomainSecretHostdevPrepare: remove
The function is no longer used once we setup per-hostdev data.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_domain.c | 50 --
src/qemu/qemu_domain.h | 4
2 files changed, 54 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 1289201764..a751d0c205 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1381,56 +1381,6 @@ qemuDomainSecretHostdevDestroy(virDomainHostdevDefPtr
hostdev)
}
-/* qemuDomainSecretHostdevPrepare:
- * @priv: pointer to domain private object
- * @hostdev: Pointer to a hostdev definition
- *
- * For the right host device, generate the qemuDomainSecretInfo structure.
- *
- * Returns 0 on success, -1 on failure
- */
-int
-qemuDomainSecretHostdevPrepare(qemuDomainObjPrivatePtr priv,
- virDomainHostdevDefPtr hostdev)
-{
-if (virHostdevIsSCSIDevice(hostdev)) {
-virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi;
-virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &scsisrc->u.iscsi;
-virStorageSourcePtr src = iscsisrc->src;
-
-if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI &&
-src->auth) {
-bool iscsiHasPS = virQEMUCapsGet(priv->qemuCaps,
QEMU_CAPS_ISCSI_PASSWORD_SECRET);
-virSecretUsageType usageType = VIR_SECRET_USAGE_TYPE_ISCSI;
-qemuDomainStorageSourcePrivatePtr srcPriv;
-
-if (!(src->privateData = qemuDomainStorageSourcePrivateNew()))
-return -1;
-
-srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src);
-
-if (!qemuDomainSupportsEncryptedSecret(priv) || !iscsiHasPS) {
-srcPriv->secinfo = qemuDomainSecretInfoNewPlain(usageType,
-
src->auth->username,
-
&src->auth->seclookupdef);
-} else {
-srcPriv->secinfo = qemuDomainSecretAESSetupFromSecret(priv,
-
hostdev->info->alias,
- NULL,
-
usageType,
-
src->auth->username,
-
&src->auth->seclookupdef);
-}
-
-if (!srcPriv->secinfo)
-return -1;
-}
-}
-
-return 0;
-}
-
-
void
qemuDomainSecretChardevDestroy(virDomainChrSourceDefPtr dev)
{
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 6abd896119..77d6bfc810 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -851,10 +851,6 @@ qemuDomainSecretInfoTLSNew(qemuDomainObjPrivatePtr priv,
void qemuDomainSecretHostdevDestroy(virDomainHostdevDefPtr disk)
ATTRIBUTE_NONNULL(1);
-int qemuDomainSecretHostdevPrepare(qemuDomainObjPrivatePtr priv,
- virDomainHostdevDefPtr hostdev)
-ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
-
void qemuDomainSecretChardevDestroy(virDomainChrSourceDefPtr dev)
ATTRIBUTE_NONNULL(1);
--
2.26.2
[PATCH 12/17] qemu: domain: Extract preparation of hostdev specific data to a separate function
Historically we've prepared secrets for all objects in one place. This
doesn't make much sense and it's semantically more appealing to prepare
everything for a single device type in one place.
Move the setup of the (iSCSI|SCSI) hostdev secrets into a new function
which will be used to setup other things as well in the future.
This is a similar approach we do for disks.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_domain.c | 59 -
src/qemu/qemu_domain.h | 4 +++
src/qemu/qemu_hotplug.c | 2 +-
src/qemu/qemu_process.c | 21 +++
4 files changed, 78 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 89f2c2c09b..1289201764 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1596,13 +1596,7 @@ qemuDomainSecretPrepare(virQEMUDriverPtr driver,
g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
size_t i;
-/* disk secrets are prepared when preparing disks */
-
-for (i = 0; i < vm->def->nhostdevs; i++) {
-if (qemuDomainSecretHostdevPrepare(priv,
- vm->def->hostdevs[i]) < 0)
-return -1;
-}
+/* disk and hostdev secrets are prepared when preparing internal data */
for (i = 0; i < vm->def->nserials; i++) {
if (qemuDomainSecretChardevPrepare(cfg, priv,
@@ -10455,6 +10449,57 @@ qemuDomainPrepareDiskSource(virDomainDiskDefPtr disk,
}
+int
+qemuDomainPrepareHostdev(virDomainHostdevDefPtr hostdev,
+ qemuDomainObjPrivatePtr priv)
+{
+if (virHostdevIsSCSIDevice(hostdev)) {
+virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi;
+virStorageSourcePtr src = NULL;
+
+switch ((virDomainHostdevSCSIProtocolType) scsisrc->protocol) {
+case VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_NONE:
+break;
+
+case VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI:
+src = scsisrc->u.iscsi.src;
+break;
+
+case VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_LAST:
+default:
+virReportEnumRangeError(virDomainHostdevSCSIProtocolType,
scsisrc->protocol);
+return -1;
+}
+
+if (src) {
+if (src->auth) {
+bool iscsiHasPS = virQEMUCapsGet(priv->qemuCaps,
QEMU_CAPS_ISCSI_PASSWORD_SECRET);
+virSecretUsageType usageType = VIR_SECRET_USAGE_TYPE_ISCSI;
+qemuDomainStorageSourcePrivatePtr srcPriv =
qemuDomainStorageSourcePrivateFetch(src);
+
+if (!qemuDomainSupportsEncryptedSecret(priv) || !iscsiHasPS) {
+srcPriv->secinfo = qemuDomainSecretInfoNewPlain(usageType,
+
src->auth->username,
+
&src->auth->seclookupdef);
+} else {
+srcPriv->secinfo = qemuDomainSecretAESSetupFromSecret(priv,
+
hostdev->info->alias,
+ NULL,
+
usageType,
+
src->auth->username,
+
&src->auth->seclookupdef);
+}
+
+if (!srcPriv->secinfo)
+return -1;
+}
+}
+}
+
+return 0;
+}
+
+
/**
* qemuDomainDiskCachemodeFlags:
*
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index adba79aded..6abd896119 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -970,6 +970,10 @@ qemuDomainDiskCachemodeFlags(int cachemode,
bool *direct,
bool *noflush);
+int
+qemuDomainPrepareHostdev(virDomainHostdevDefPtr hostdev,
+ qemuDomainObjPrivatePtr priv);
+
char * qemuDomainGetManagedPRSocketPath(qemuDomainObjPrivatePtr priv);
bool qemuDomainDefHasManagedPR(virDomainObjPtr vm);
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index e2c6e14c2e..f20b8e9a56 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2604,7 +2604,7 @@ qemuDomainAttachHostSCSIDevice(virQEMUDriverPtr driver,
if (qemuAssignDeviceHostdevAlias(vm->def, &hostdev->info->alias, -1) < 0)
goto cleanup;
-if (qemuDomainSecretHostdevPrepare(priv, hostdev) < 0)
+if (qemuDomainPrepareHostdev(hostdev, priv) < 0)
goto cleanup;
if (!(data = qemuBuildHostdevSCSIAttachPrepare(hostdev, &backendalias,
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index dd60fb0ddf..79e72aaf2a 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -6213,6 +6213,23 @@
[PATCH 09/17] qemu: domain: Fill in (i)SCSI backend nodename if it is not present in status XML
For upgrade reasons so that we can modify the used nodename we must
generate the old version for all status XMLs which don't have it stored
explicitly.
The change will be required as using the user-provided alias may result
in too-long nodenames which will be rejected by qemu.
Add code which fills in the appropriate old value and add test cases to
validate that it's added and also that existing nodenames are not
overwritten.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_domain.c | 55 +
tests/qemustatusxml2xmldata/modern-in.xml | 1 +
tests/qemustatusxml2xmldata/upgrade-in.xml | 1 +
tests/qemustatusxml2xmldata/upgrade-out.xml | 12 +
4 files changed, 69 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 0ed132a829..89f2c2c09b 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5301,6 +5301,57 @@
qemuDomainDeviceHostdevDefPostParseRestoreSecAlias(virDomainHostdevDefPtr hostde
}
+/**
+ * qemuDomainDeviceHostdevDefPostParseRestoreBackendAlias:
+ *
+ * Re-generate backend alias if it wasn't stored in the status XML by an older
+ * libvirtd.
+ *
+ * Note that qemuCaps should be always present for a status XML.
+ */
+static int
+qemuDomainDeviceHostdevDefPostParseRestoreBackendAlias(virDomainHostdevDefPtr
hostdev,
+ virQEMUCapsPtr qemuCaps,
+ unsigned int parseFlags)
+{
+virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi;
+virStorageSourcePtr src;
+
+if (!(parseFlags & VIR_DOMAIN_DEF_PARSE_STATUS))
+return 0;
+
+if (!qemuCaps ||
+hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS ||
+hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI ||
+!virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV_HOSTDEV_SCSI))
+return 0;
+
+switch ((virDomainHostdevSCSIProtocolType) scsisrc->protocol) {
+case VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_NONE:
+if (!scsisrc->u.host.src &&
+!(scsisrc->u.host.src = virStorageSourceNew()))
+return -1;
+
+src = scsisrc->u.host.src;
+break;
+
+case VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI:
+src = scsisrc->u.iscsi.src;
+break;
+
+case VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_LAST:
+default:
+virReportEnumRangeError(virDomainHostdevSCSIProtocolType,
scsisrc->protocol);
+return -1;
+}
+
+if (!src->nodestorage)
+src->nodestorage = g_strdup_printf("libvirt-%s-backend",
hostdev->info->alias);
+
+return 0;
+}
+
+
static int
qemuDomainHostdevDefMdevPostParse(virDomainHostdevSubsysMediatedDevPtr mdevsrc,
virQEMUCapsPtr qemuCaps)
@@ -5327,6 +5378,10 @@ qemuDomainHostdevDefPostParse(virDomainHostdevDefPtr
hostdev,
parseFlags) < 0)
return -1;
+if (qemuDomainDeviceHostdevDefPostParseRestoreBackendAlias(hostdev,
qemuCaps,
+ parseFlags) < 0)
+return -1;
+
if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV &&
qemuDomainHostdevDefMdevPostParse(&subsys->u.mdev, qemuCaps) < 0)
diff --git a/tests/qemustatusxml2xmldata/modern-in.xml
b/tests/qemustatusxml2xmldata/modern-in.xml
index 0a3990bd3e..5a5d5b8983 100644
--- a/tests/qemustatusxml2xmldata/modern-in.xml
+++ b/tests/qemustatusxml2xmldata/modern-in.xml
@@ -234,6 +234,7 @@
+
diff --git a/tests/qemustatusxml2xmldata/upgrade-in.xml
b/tests/qemustatusxml2xmldata/upgrade-in.xml
index 1942eacfa4..7fa56429f4 100644
--- a/tests/qemustatusxml2xmldata/upgrade-in.xml
+++ b/tests/qemustatusxml2xmldata/upgrade-in.xml
@@ -232,6 +232,7 @@
+
diff --git a/tests/qemustatusxml2xmldata/upgrade-out.xml
b/tests/qemustatusxml2xmldata/upgrade-out.xml
index ee3c1b49b0..962d25ce2e 100644
--- a/tests/qemustatusxml2xmldata/upgrade-out.xml
+++ b/tests/qemustatusxml2xmldata/upgrade-out.xml
@@ -232,6 +232,7 @@
+
@@ -517,6 +518,9 @@
+
+
+
@@ -532,6 +536,9 @@
+
+
+
@@ -547,6 +554,11 @@
+
+
+
+
+
--
2.26.2
[PATCH 16/17] qemuDomainPrepareHostdev: Don't base backend nodename on device alias
QEMU's blockdev nodenames which are used to back SCSI/iSCSI hostdevs are
limited to 32 characters. If a user passes a very long user alias as
name of the host device it's easy to end up with a too-long nodename.
To prevent this from happening don't base the nodename on the possibly
user-specified alias but on the normal sequential node name generator.
We then store the name in the status XML for further use.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_domain.c| 3 +-
.../hostdev-scsi-lsi.x86_64-latest.args | 38 ---
...ostdev-scsi-virtio-scsi.x86_64-latest.args | 36 +-
3 files changed, 36 insertions(+), 41 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 46f7caeb09..2444e47173 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -10429,7 +10429,8 @@ qemuDomainPrepareHostdev(virDomainHostdevDefPtr hostdev,
const char *backendalias = hostdev->info->alias;
if (virQEMUCapsGet(priv->qemuCaps,
QEMU_CAPS_BLOCKDEV_HOSTDEV_SCSI)) {
-src->nodestorage = g_strdup_printf("libvirt-%s-backend",
hostdev->info->alias);
+src->id = qemuDomainStorageIdNew(priv);
+src->nodestorage = g_strdup_printf("libvirt-%d-backend",
src->id);
backendalias = src->nodestorage;
}
diff --git a/tests/qemuxml2argvdata/hostdev-scsi-lsi.x86_64-latest.args
b/tests/qemuxml2argvdata/hostdev-scsi-lsi.x86_64-latest.args
index f768c2471b..a5b200543a 100644
--- a/tests/qemuxml2argvdata/hostdev-scsi-lsi.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/hostdev-scsi-lsi.x86_64-latest.args
@@ -35,41 +35,35 @@ file=/tmp/lib/domain--1-QEMUGuest2/master-key.aes \
"file":"libvirt-1-storage"}' \
-device ide-hd,bus=ide.0,unit=0,drive=libvirt-1-format,id=ide0-0-0,bootindex=1
\
-blockdev '{"driver":"host_device","filename":"/dev/sg0",\
-"node-name":"libvirt-hostdev0-backend","read-only":false}' \
--device scsi-generic,bus=scsi0.0,scsi-id=7,drive=libvirt-hostdev0-backend,\
-id=hostdev0 \
+"node-name":"libvirt-2-backend","read-only":false}' \
+-device scsi-generic,bus=scsi0.0,scsi-id=7,drive=libvirt-2-backend,id=hostdev0
\
-blockdev '{"driver":"host_device","filename":"/dev/sg0",\
-"node-name":"libvirt-hostdev1-backend","read-only":true}' \
--device scsi-generic,bus=scsi0.0,scsi-id=6,drive=libvirt-hostdev1-backend,\
-id=hostdev1 \
+"node-name":"libvirt-3-backend","read-only":true}' \
+-device scsi-generic,bus=scsi0.0,scsi-id=6,drive=libvirt-3-backend,id=hostdev1
\
-blockdev '{"driver":"iscsi","portal":"example.org:3260",\
"target":"iqn.1992-01.com.example","lun":0,"transport":"tcp",\
-"node-name":"libvirt-hostdev2-backend","read-only":false}' \
--device scsi-generic,bus=scsi0.0,scsi-id=4,drive=libvirt-hostdev2-backend,\
-id=hostdev2 \
+"node-name":"libvirt-4-backend","read-only":false}' \
+-device scsi-generic,bus=scsi0.0,scsi-id=4,drive=libvirt-4-backend,id=hostdev2
\
-blockdev '{"driver":"iscsi","portal":"example.org:3260",\
"target":"iqn.1992-01.com.example","lun":1,"transport":"tcp",\
-"node-name":"libvirt-hostdev3-backend","read-only":false}' \
--device scsi-generic,bus=scsi0.0,scsi-id=5,drive=libvirt-hostdev3-backend,\
-id=hostdev3 \
--object secret,id=libvirt-hostdev4-backend-secret0,\
+"node-name":"libvirt-5-backend","read-only":false}' \
+-device scsi-generic,bus=scsi0.0,scsi-id=5,drive=libvirt-5-backend,id=hostdev3
\
+-object secret,id=libvirt-6-backend-secret0,\
data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\
keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \
-blockdev '{"driver":"iscsi","portal":"example.org:3260",\
"target":"iqn.1992-01.com.example:storage","lun":1,"transport":"tcp",\
-"user":"myname","password-secret":"libvirt-hostdev4-backend-secret0",\
-"node-name":"libvirt-hostdev4-backend","read-only":false}' \
--device scsi-generic,bus=scsi0.0,scsi-id=3,drive=libvirt-hostdev4-backend,\
-id=hostdev4 \
--object secret,id=libvirt-hostdev5-backend-secret0,\
+"user":"myname","password-secret":"libvirt-6-backend-secret0",\
+"node-name":"libvirt-6-backend","read-only":false}' \
+-device scsi-generic,bus=scsi0.0,scsi-id=3,drive=libvirt-6-backend,id=hostdev4
\
+-object secret,id=libvirt-7-backend-secret0,\
data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\
keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \
-blockdev '{"driver":"iscsi","portal":"example.org:3260",\
"target":"iqn.1992-01.com.example:storage","lun":2,"transport":"tcp",\
-"user":"myname","password-secret":"libvirt-hostdev5-backend-secret0",\
-"node-name":"libvirt-hostdev5-backend","read-only":false}' \
--device scsi-generic,bus=scsi0.0,scsi-id=2,drive=libvirt-hostdev5-backend,\
-id=hostdev5 \
+"user":"myname","password-secret":"libvirt-7-backend-secret0",\
+"node-name":"libvirt-7-backend","read-only":false}' \
+-device scsi-generic,bus=scsi0.0,scsi-id=2,drive=libvirt-7-backend,id=hostdev
[PATCH 06/17] conf: Add virStorageSource member for SCSI host device config data
The backend for the SCSI host device is a storage source. While the
definition doesn't look like that it's converted to a storage source
when the VM is running.
Add the storage source to the definition object and also parse/format
it's private data which will be used for internal state storage while
the VM is running.
Note that the virStorageSourcePtr may not be allocated all the time so
the private data parser allocates it if there is any private data
present.
Signed-off-by: Peter Krempa
---
src/conf/domain_conf.c | 24 ++--
src/conf/domain_conf.h | 1 +
2 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 05c2c63f58..f93cd60c2b 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3019,6 +3019,8 @@
virDomainHostdevSubsysSCSIClear(virDomainHostdevSubsysSCSIPtr scsisrc)
scsisrc->u.iscsi.src = NULL;
} else {
VIR_FREE(scsisrc->u.host.adapter);
+virObjectUnref(scsisrc->u.host.src);
+scsisrc->u.host.src = NULL;
}
}
@@ -8263,7 +8265,9 @@ virDomainStorageNetworkParseHosts(xmlNodePtr node,
static int
virDomainHostdevSubsysSCSIHostDefParseXML(xmlNodePtr sourcenode,
xmlXPathContextPtr ctxt,
- virDomainHostdevSubsysSCSIPtr
scsisrc)
+ virDomainHostdevSubsysSCSIPtr
scsisrc,
+ unsigned int flags,
+ virDomainXMLOptionPtr xmlopt)
{
virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host;
g_autofree char *bus = NULL;
@@ -8313,6 +8317,16 @@ virDomainHostdevSubsysSCSIHostDefParseXML(xmlNodePtr
sourcenode,
return -1;
}
+if (flags & VIR_DOMAIN_DEF_PARSE_STATUS &&
+xmlopt && xmlopt->privateData.storageParse) {
+if ((ctxt->node = virXPathNode("./privateData", ctxt))) {
+if (!scsihostsrc->src &&
+!(scsihostsrc->src = virStorageSourceNew()))
+return -1;
+if (xmlopt->privateData.storageParse(ctxt, scsihostsrc->src) < 0)
+return -1;
+}
+}
return 0;
}
@@ -8413,7 +8427,8 @@ virDomainHostdevSubsysSCSIDefParseXML(xmlNodePtr
sourcenode,
switch ((virDomainHostdevSCSIProtocolType) scsisrc->protocol) {
case VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_NONE:
-return virDomainHostdevSubsysSCSIHostDefParseXML(sourcenode, ctxt,
scsisrc);
+return virDomainHostdevSubsysSCSIHostDefParseXML(sourcenode, ctxt,
scsisrc,
+ flags, xmlopt);
case VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI:
return virDomainHostdevSubsysSCSIiSCSIDefParseXML(sourcenode, scsisrc,
ctxt,
@@ -26305,6 +26320,11 @@ virDomainHostdevDefFormatSubsysSCSI(virBufferPtr buf,
virBufferAsprintf(&sourceChildBuf, " bus='%u' target='%u' unit='%llu'",
scsihostsrc->bus, scsihostsrc->target,
scsihostsrc->unit);
virBufferAddLit(&sourceChildBuf, "/>\n");
+
+if (scsihostsrc->src &&
+virDomainDiskSourceFormatPrivateData(&sourceChildBuf,
scsihostsrc->src,
+ flags, xmlopt) < 0)
+return -1;
}
virXMLFormatElement(buf, "source", &sourceAttrBuf, &sourceChildBuf);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 14a376350c..cf76f340ee 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -245,6 +245,7 @@ struct _virDomainHostdevSubsysSCSIHost {
unsigned bus;
unsigned target;
unsigned long long unit;
+virStorageSourcePtr src;
};
struct _virDomainHostdevSubsysSCSIiSCSI {
--
2.26.2
[PATCH 08/17] tests: qemustatusxml2xmldata: Add local SCSI hostdev to 'upgrade' case
Add a local SCSI host device to validate upcoming generated data. Signed-off-by: Peter Krempa --- tests/qemustatusxml2xmldata/upgrade-in.xml | 8 tests/qemustatusxml2xmldata/upgrade-out.xml | 8 2 files changed, 16 insertions(+) diff --git a/tests/qemustatusxml2xmldata/upgrade-in.xml b/tests/qemustatusxml2xmldata/upgrade-in.xml index 8023857ffe..1942eacfa4 100644 --- a/tests/qemustatusxml2xmldata/upgrade-in.xml +++ b/tests/qemustatusxml2xmldata/upgrade-in.xml @@ -511,6 +511,14 @@ + + + + + + + + diff --git a/tests/qemustatusxml2xmldata/upgrade-out.xml b/tests/qemustatusxml2xmldata/upgrade-out.xml index d5534fb0f5..ee3c1b49b0 100644 --- a/tests/qemustatusxml2xmldata/upgrade-out.xml +++ b/tests/qemustatusxml2xmldata/upgrade-out.xml @@ -543,6 +543,14 @@ + + + + + + + + -- 2.26.2
[PATCH 04/17] syntax-check: Don't forbid curly braces around single line condition body
This syntax rule doesn't make much sense, especially if there are so
much exceptions to it. Just remove it and adjust the coding style.
Signed-off-by: Peter Krempa
---
build-aux/check-spacing.pl | 36
docs/coding-style.rst | 8
2 files changed, 4 insertions(+), 40 deletions(-)
diff --git a/build-aux/check-spacing.pl b/build-aux/check-spacing.pl
index 33377f3dd3..72901b75f9 100755
--- a/build-aux/check-spacing.pl
+++ b/build-aux/check-spacing.pl
@@ -24,11 +24,6 @@ my $ret = 0;
my $incomment = 0;
foreach my $file (@ARGV) {
-# Per-file variables for multiline Curly Bracket (cb_) check
-my $cb_linenum = 0;
-my $cb_code = "";
-my $cb_scolon = 0;
-
open FILE, $file;
while (defined (my $line = )) {
@@ -160,37 +155,6 @@ foreach my $file (@ARGV) {
print "$file:$.: $line";
$ret = 1;
}
-
-# One line conditional statements with one line bodies should
-# not use curly brackets.
-if ($data =~ /^\s*(if|while|for)\b.*\{$/) {
-$cb_linenum = $.;
-$cb_code = $line;
-$cb_scolon = 0;
-}
-
-# We need to check for exactly one semicolon inside the body,
-# because empty statements (e.g. with comment only) are
-# allowed
-if ($cb_linenum == $. - 1 && $data =~ /^[^;]*;[^;]*$/) {
-$cb_code .= $line;
-$cb_scolon = 1;
-}
-
-if ($data =~ /^\s*}\s*$/ &&
-$cb_linenum == $. - 2 &&
-$cb_scolon) {
-
-print "Curly brackets around single-line body:\n";
-print "$file:$cb_linenum-$.:\n$cb_code$line";
-$ret = 1;
-
-# There _should_ be no need to reset the values; but to
-# keep my inner peace...
-$cb_linenum = 0;
-$cb_scolon = 0;
-$cb_code = "";
-}
}
close FILE;
}
diff --git a/docs/coding-style.rst b/docs/coding-style.rst
index 942caf4e09..44e5265a60 100644
--- a/docs/coding-style.rst
+++ b/docs/coding-style.rst
@@ -258,15 +258,15 @@ comment, although use of a semicolon is not currently
rejected.
Curly braces
-Omit the curly braces around an ``if``, ``while``, ``for`` etc.
-body only when both that body and the condition itself occupy a
-single line. In every other case we require the braces. This
+Curly braces around an ``if``, ``while``, ``for`` etc. can be omitted if the
+body and the condition itself occupy only a single line.
+In every other case we require the braces. This
ensures that it is trivially easy to identify a
single-\ *statement* loop: each has only one *line* in its body.
::
- while (expr) // single line body; {} is forbidden
+ while (expr) // single line body; {} is optional
single_line_stmt();
::
--
2.26.2
Re: [libvirt PATCH v3 0/8] Replace VIR_{ALLOC, ALLOC_N, FREE} in src/cpu/*.
On a Friday in 2020, Tim Wiederhake wrote: V1: https://www.redhat.com/archives/libvir-list/2020-September/msg00540.html V2: https://www.redhat.com/archives/libvir-list/2020-September/msg00610.html Tim Wiederhake (8): cpu_ppc64: Use g_auto* in ppc64MakeCPUData cpu_map: Use g_auto* in loadData cpu_map: Use g_auto* in loadIncludes cpu_x86: Use g_auto* in virX86CpuIncompatible cpu_map: Remove unnecessary variable in loadData cpu: Replace VIR_ALLOC with g_new0 cpu: Replace VIR_ALLOC_N with g_new0 cpu: Replace VIR_FREE with g_free src/cpu/cpu.c | 6 ++--- src/cpu/cpu_map.c | 18 ++ src/cpu/cpu_ppc64.c | 59 - src/cpu/cpu_x86.c | 11 - 4 files changed, 35 insertions(+), 59 deletions(-) Pushed. Jano signature.asc Description: PGP signature
[PATCH] smp: drop support for deprecated (invalid topologies)
it's was deprecated since 3.1
Support for invalid topologies is removed, the user must ensure
that topologies described with -smp include all possible cpus,
i.e. (sockets * cores * threads) == maxcpus or QEMU will
exit with error.
Signed-off-by: Igor Mammedov
---
docs/system/deprecated.rst | 26 +-
hw/core/machine.c | 16
hw/i386/pc.c | 16
3 files changed, 21 insertions(+), 37 deletions(-)
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 122717cfee..d737728fab 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -47,19 +47,6 @@ The 'file' driver for drives is no longer appropriate for
character or host
devices and will only accept regular files (S_IFREG). The correct driver
for these file types is 'host_cdrom' or 'host_device' as appropriate.
-``-smp`` (invalid topologies) (since 3.1)
-'
-
-CPU topology properties should describe whole machine topology including
-possible CPUs.
-
-However, historically it was possible to start QEMU with an incorrect topology
-where *n* <= *sockets* * *cores* * *threads* < *maxcpus*,
-which could lead to an incorrect topology enumeration by the guest.
-Support for invalid topologies will be removed, the user must ensure
-topologies described with -smp include all possible cpus, i.e.
-*sockets* * *cores* * *threads* = *maxcpus*.
-
``-vnc acl`` (since 4.0.0)
''
@@ -618,6 +605,19 @@ New machine versions (since 5.1) will not accept the
option but it will still
work with old machine types. User can check the QAPI schema to see if the
legacy
option is supported by looking at MachineInfo::numa-mem-supported property.
+``-smp`` (invalid topologies) (removed 5.2)
+'''
+
+CPU topology properties should describe whole machine topology including
+possible CPUs.
+
+However, historically it was possible to start QEMU with an incorrect topology
+where *n* <= *sockets* * *cores* * *threads* < *maxcpus*,
+which could lead to an incorrect topology enumeration by the guest.
+Support for invalid topologies is removed, the user must ensure
+topologies described with -smp include all possible cpus, i.e.
+*sockets* * *cores* * *threads* = *maxcpus*.
+
Block devices
-
diff --git a/hw/core/machine.c b/hw/core/machine.c
index ea26d61237..09aee4ea52 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -754,23 +754,15 @@ static void smp_parse(MachineState *ms, QemuOpts *opts)
exit(1);
}
-if (sockets * cores * threads > ms->smp.max_cpus) {
-error_report("cpu topology: "
- "sockets (%u) * cores (%u) * threads (%u) > "
- "maxcpus (%u)",
+if (sockets * cores * threads != ms->smp.max_cpus) {
+error_report("Invalid CPU topology: "
+ "sockets (%u) * cores (%u) * threads (%u) "
+ "!= maxcpus (%u)",
sockets, cores, threads,
ms->smp.max_cpus);
exit(1);
}
-if (sockets * cores * threads != ms->smp.max_cpus) {
-warn_report("Invalid CPU topology deprecated: "
-"sockets (%u) * cores (%u) * threads (%u) "
-"!= maxcpus (%u)",
-sockets, cores, threads,
-ms->smp.max_cpus);
-}
-
ms->smp.cpus = cpus;
ms->smp.cores = cores;
ms->smp.threads = threads;
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index d071da787b..fbde6b04e6 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -746,23 +746,15 @@ void pc_smp_parse(MachineState *ms, QemuOpts *opts)
exit(1);
}
-if (sockets * dies * cores * threads > ms->smp.max_cpus) {
-error_report("cpu topology: "
- "sockets (%u) * dies (%u) * cores (%u) * threads (%u)
> "
- "maxcpus (%u)",
+if (sockets * dies * cores * threads != ms->smp.max_cpus) {
+error_report("Invalid CPU topology deprecated: "
+ "sockets (%u) * dies (%u) * cores (%u) * threads (%u)
"
+ "!= maxcpus (%u)",
sockets, dies, cores, threads,
ms->smp.max_cpus);
exit(1);
}
-if (sockets * dies * cores * threads != ms->smp.max_cpus) {
-warn_report("Invalid CPU topology deprecated: "
-"sockets (%u) * dies (%u) * cores (%u) * threads (%u) "
-"!= maxcpus (%u)",
-sockets, dies, cores, threads,
-ms->smp.max_cpus);
-}
-
ms->smp.cpus = cpus;
ms->smp.cores = cores;
ms->smp.threads = threads;
--
2.27.0
[libvirt PATCH v3 1/8] cpu_ppc64: Use g_auto* in ppc64MakeCPUData
Signed-off-by: Tim Wiederhake
Reviewed-by: Ján Tomko
---
src/cpu/cpu_ppc64.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c
index 28fbfea9ae..c0d09db696 100644
--- a/src/cpu/cpu_ppc64.c
+++ b/src/cpu/cpu_ppc64.c
@@ -399,7 +399,7 @@ static virCPUDataPtr
ppc64MakeCPUData(virArch arch,
virCPUppc64Data *data)
{
-virCPUDataPtr cpuData;
+g_autoptr(virCPUData) cpuData = NULL;
if (VIR_ALLOC(cpuData) < 0)
return NULL;
@@ -407,9 +407,9 @@ ppc64MakeCPUData(virArch arch,
cpuData->arch = arch;
if (ppc64DataCopy(&cpuData->data.ppc64, data) < 0)
-VIR_FREE(cpuData);
+return NULL;
-return cpuData;
+return g_steal_pointer(&cpuData);
}
static virCPUCompareResult
--
2.26.2
[libvirt PATCH v3 2/8] cpu_map: Use g_auto* in loadData
Signed-off-by: Tim Wiederhake
Reviewed-by: Ján Tomko
---
src/cpu/cpu_map.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/cpu/cpu_map.c b/src/cpu/cpu_map.c
index 65d244e011..53c8cbba6b 100644
--- a/src/cpu/cpu_map.c
+++ b/src/cpu/cpu_map.c
@@ -55,8 +55,9 @@ loadData(const char *mapfile,
}
for (i = 0; i < n; i++) {
-char *name = virXMLPropString(nodes[i], "name");
-if (!name) {
+g_autofree char *name = NULL;
+
+if (!(name = virXMLPropString(nodes[i], "name"))) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("cannot find %s name in CPU map '%s'"), element,
mapfile);
return -1;
@@ -64,7 +65,6 @@ loadData(const char *mapfile,
VIR_DEBUG("Load %s name %s", element, name);
ctxt->node = nodes[i];
rv = callback(ctxt, name, data);
-VIR_FREE(name);
if (rv < 0)
return -1;
}
--
2.26.2
[libvirt PATCH v3 7/8] cpu: Replace VIR_ALLOC_N with g_new0
Signed-off-by: Tim Wiederhake
Reviewed-by: Ján Tomko
---
src/cpu/cpu_ppc64.c | 16
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c
index ca2cfa0a67..6477b4bce7 100644
--- a/src/cpu/cpu_ppc64.c
+++ b/src/cpu/cpu_ppc64.c
@@ -134,9 +134,7 @@ ppc64DataCopy(virCPUppc64Data *dst, const virCPUppc64Data
*src)
{
size_t i;
-if (VIR_ALLOC_N(dst->pvr, src->len) < 0)
-return -1;
-
+dst->pvr = g_new0(virCPUppc64PVR, src->len);
dst->len = src->len;
for (i = 0; i < src->len; i++) {
@@ -343,9 +341,7 @@ ppc64ModelParse(xmlXPathContextPtr ctxt,
return -1;
}
-if (VIR_ALLOC_N(model->data.pvr, n) < 0)
-return -1;
-
+model->data.pvr = g_new0(virCPUppc64PVR, n);
model->data.len = n;
for (i = 0; i < n; i++) {
@@ -603,10 +599,7 @@ virCPUppc64GetHost(virCPUDefPtr cpu,
return -1;
data = &cpuData->data.ppc64;
-
-if (VIR_ALLOC_N(data->pvr, 1) < 0)
-return -1;
-
+data->pvr = g_new0(virCPUppc64PVR, 1);
data->len = 1;
#if defined(__powerpc__) || defined(__powerpc64__)
@@ -732,8 +725,7 @@ virCPUppc64DriverGetModels(char ***models)
return -1;
if (models) {
-if (VIR_ALLOC_N(*models, map->nmodels + 1) < 0)
-return -1;
+*models = g_new0(char*, map->nmodels + 1);
for (i = 0; i < map->nmodels; i++)
(*models)[i] = g_strdup(map->models[i]->name);
--
2.26.2
[libvirt PATCH v3 8/8] cpu: Replace VIR_FREE with g_free
Note the use of g_clear_pointer(..., g_free) in ppc64DataClear and
virCPUx86Baseline.
Signed-off-by: Tim Wiederhake
Reviewed-by: Ján Tomko
---
src/cpu/cpu.c | 2 +-
src/cpu/cpu_ppc64.c | 18 +-
src/cpu/cpu_x86.c | 8
3 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c
index c3eef52c79..188c5d86b5 100644
--- a/src/cpu/cpu.c
+++ b/src/cpu/cpu.c
@@ -315,7 +315,7 @@ virCPUDataFree(virCPUDataPtr data)
if ((driver = cpuGetSubDriver(data->arch)) && driver->dataFree)
driver->dataFree(data);
else
-VIR_FREE(data);
+g_free(data);
}
diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c
index 6477b4bce7..2fedcd25da 100644
--- a/src/cpu/cpu_ppc64.c
+++ b/src/cpu/cpu_ppc64.c
@@ -126,7 +126,7 @@ ppc64DataClear(virCPUppc64Data *data)
if (!data)
return;
-VIR_FREE(data->pvr);
+g_clear_pointer(&data->pvr, g_free);
}
static int
@@ -151,8 +151,8 @@ ppc64VendorFree(virCPUppc64VendorPtr vendor)
if (!vendor)
return;
-VIR_FREE(vendor->name);
-VIR_FREE(vendor);
+g_free(vendor->name);
+g_free(vendor);
}
G_DEFINE_AUTOPTR_CLEANUP_FUNC(virCPUppc64Vendor, ppc64VendorFree);
@@ -177,8 +177,8 @@ ppc64ModelFree(virCPUppc64ModelPtr model)
return;
ppc64DataClear(&model->data);
-VIR_FREE(model->name);
-VIR_FREE(model);
+g_free(model->name);
+g_free(model);
}
G_DEFINE_AUTOPTR_CLEANUP_FUNC(virCPUppc64Model, ppc64ModelFree);
@@ -261,13 +261,13 @@ ppc64MapFree(virCPUppc64MapPtr map)
for (i = 0; i < map->nmodels; i++)
ppc64ModelFree(map->models[i]);
-VIR_FREE(map->models);
+g_free(map->models);
for (i = 0; i < map->nvendors; i++)
ppc64VendorFree(map->vendors[i]);
-VIR_FREE(map->vendors);
+g_free(map->vendors);
-VIR_FREE(map);
+g_free(map);
}
G_DEFINE_AUTOPTR_CLEANUP_FUNC(virCPUppc64Map, ppc64MapFree);
@@ -584,7 +584,7 @@ virCPUppc64DataFree(virCPUDataPtr data)
return;
ppc64DataClear(&data->data.ppc64);
-VIR_FREE(data);
+g_free(data);
}
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index db1b2e55a1..0e533c62e1 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -493,7 +493,7 @@ virCPUx86DataFree(virCPUDataPtr data)
return;
virCPUx86DataClear(&data->data.x86);
-VIR_FREE(data);
+g_free(data);
}
@@ -2207,7 +2207,7 @@ x86Decode(virCPUDefPtr cpu,
if (x86FeatureIsMigratable(cpuModel->features[i].name, map)) {
i++;
} else {
-VIR_FREE(cpuModel->features[i].name);
+g_free(cpuModel->features[i].name);
VIR_DELETE_ELEMENT_INPLACE(cpuModel->features, i,
cpuModel->nfeatures);
}
@@ -2892,7 +2892,7 @@ virCPUx86Baseline(virCPUDefPtr *cpus,
cpu->fallback = VIR_CPU_FALLBACK_FORBID;
if (!outputVendor)
-VIR_FREE(cpu->vendor);
+g_clear_pointer(&cpu->vendor, g_free);
return g_steal_pointer(&cpu);
}
@@ -2914,7 +2914,7 @@ x86UpdateHostModel(virCPUDefPtr guest,
return -1;
if (guest->vendor_id) {
-VIR_FREE(updated->vendor_id);
+g_free(updated->vendor_id);
updated->vendor_id = g_strdup(guest->vendor_id);
}
--
2.26.2
[libvirt PATCH v3 4/8] cpu_x86: Use g_auto* in virX86CpuIncompatible
Signed-off-by: Tim Wiederhake
Reviewed-by: Ján Tomko
---
src/cpu/cpu_x86.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index fdb665b01d..db1b2e55a1 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -1797,7 +1797,7 @@ virCPUx86DataParse(xmlXPathContextPtr ctxt)
*/
#define virX86CpuIncompatible(MSG, CPU_DEF) \
do { \
-char *flagsStr = NULL; \
+g_autofree char *flagsStr = NULL; \
if (!(flagsStr = x86FeatureNames(map, ", ", (CPU_DEF { \
virReportOOMError(); \
return VIR_CPU_COMPARE_ERROR; \
@@ -1805,7 +1805,6 @@ virCPUx86DataParse(xmlXPathContextPtr ctxt)
if (message) \
*message = g_strdup_printf("%s: %s", _(MSG), flagsStr); \
VIR_DEBUG("%s: %s", MSG, flagsStr); \
-VIR_FREE(flagsStr); \
} while (0)
--
2.26.2
[libvirt PATCH v3 3/8] cpu_map: Use g_auto* in loadIncludes
Signed-off-by: Tim Wiederhake
---
src/cpu/cpu_map.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/src/cpu/cpu_map.c b/src/cpu/cpu_map.c
index 53c8cbba6b..743547c6d1 100644
--- a/src/cpu/cpu_map.c
+++ b/src/cpu/cpu_map.c
@@ -125,18 +125,16 @@ loadIncludes(xmlXPathContextPtr ctxt,
return -1;
for (i = 0; i < n; i++) {
-char *filename = virXMLPropString(nodes[i], "filename");
+g_autofree char *filename = virXMLPropString(nodes[i], "filename");
+
if (!filename) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Missing 'filename' in CPU map include"));
return -1;
}
VIR_DEBUG("Finding CPU map include '%s'", filename);
-if (cpuMapLoadInclude(filename, vendorCB, featureCB, modelCB, data) <
0) {
-VIR_FREE(filename);
+if (cpuMapLoadInclude(filename, vendorCB, featureCB, modelCB, data) <
0)
return -1;
-}
-VIR_FREE(filename);
}
return 0;
--
2.26.2
[libvirt PATCH v3 6/8] cpu: Replace VIR_ALLOC with g_new0
Signed-off-by: Tim Wiederhake
Reviewed-by: Ján Tomko
---
src/cpu/cpu.c | 4 +---
src/cpu/cpu_ppc64.c | 19 +--
2 files changed, 6 insertions(+), 17 deletions(-)
diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c
index 69e4205e4b..c3eef52c79 100644
--- a/src/cpu/cpu.c
+++ b/src/cpu/cpu.c
@@ -286,9 +286,7 @@ virCPUDataNew(virArch arch)
{
virCPUDataPtr data;
-if (VIR_ALLOC(data) < 0)
-return NULL;
-
+data = g_new0(virCPUData, 1);
data->arch = arch;
return data;
diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c
index c0d09db696..ca2cfa0a67 100644
--- a/src/cpu/cpu_ppc64.c
+++ b/src/cpu/cpu_ppc64.c
@@ -189,9 +189,7 @@ ppc64ModelCopy(const virCPUppc64Model *model)
{
g_autoptr(virCPUppc64Model) copy = NULL;
-if (VIR_ALLOC(copy) < 0)
-return NULL;
-
+copy = g_new0(virCPUppc64Model, 1);
copy->name = g_strdup(model->name);
if (ppc64DataCopy(©->data, &model->data) < 0)
@@ -283,9 +281,7 @@ ppc64VendorParse(xmlXPathContextPtr ctxt G_GNUC_UNUSED,
virCPUppc64MapPtr map = data;
g_autoptr(virCPUppc64Vendor) vendor = NULL;
-if (VIR_ALLOC(vendor) < 0)
-return -1;
-
+vendor = g_new0(virCPUppc64Vendor, 1);
vendor->name = g_strdup(name);
if (ppc64VendorFind(map, vendor->name)) {
@@ -314,9 +310,7 @@ ppc64ModelParse(xmlXPathContextPtr ctxt,
size_t i;
int n;
-if (VIR_ALLOC(model) < 0)
-return -1;
-
+model = g_new0(virCPUppc64Model, 1);
model->name = g_strdup(name);
if (ppc64ModelFind(map, model->name)) {
@@ -386,8 +380,7 @@ ppc64LoadMap(void)
{
g_autoptr(virCPUppc64Map) map = NULL;
-if (VIR_ALLOC(map) < 0)
-return NULL;
+map = g_new0(virCPUppc64Map, 1);
if (cpuMapLoad("ppc64", ppc64VendorParse, NULL, ppc64ModelParse, map) < 0)
return NULL;
@@ -401,9 +394,7 @@ ppc64MakeCPUData(virArch arch,
{
g_autoptr(virCPUData) cpuData = NULL;
-if (VIR_ALLOC(cpuData) < 0)
-return NULL;
-
+cpuData = g_new0(virCPUData, 1);
cpuData->arch = arch;
if (ppc64DataCopy(&cpuData->data.ppc64, data) < 0)
--
2.26.2
[libvirt PATCH v3 5/8] cpu_map: Remove unnecessary variable in loadData
Signed-off-by: Tim Wiederhake
Reviewed-by: Ján Tomko
---
src/cpu/cpu_map.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/src/cpu/cpu_map.c b/src/cpu/cpu_map.c
index 743547c6d1..6baaa6 100644
--- a/src/cpu/cpu_map.c
+++ b/src/cpu/cpu_map.c
@@ -43,7 +43,6 @@ loadData(const char *mapfile,
g_autofree xmlNodePtr *nodes = NULL;
int n;
size_t i;
-int rv;
if ((n = virXPathNodeSet(element, ctxt, &nodes)) < 0)
return -1;
@@ -64,8 +63,7 @@ loadData(const char *mapfile,
}
VIR_DEBUG("Load %s name %s", element, name);
ctxt->node = nodes[i];
-rv = callback(ctxt, name, data);
-if (rv < 0)
+if (callback(ctxt, name, data) < 0)
return -1;
}
--
2.26.2
[libvirt PATCH v3 0/8] Replace VIR_{ALLOC, ALLOC_N, FREE} in src/cpu/*.
V1: https://www.redhat.com/archives/libvir-list/2020-September/msg00540.html V2: https://www.redhat.com/archives/libvir-list/2020-September/msg00610.html Tim Wiederhake (8): cpu_ppc64: Use g_auto* in ppc64MakeCPUData cpu_map: Use g_auto* in loadData cpu_map: Use g_auto* in loadIncludes cpu_x86: Use g_auto* in virX86CpuIncompatible cpu_map: Remove unnecessary variable in loadData cpu: Replace VIR_ALLOC with g_new0 cpu: Replace VIR_ALLOC_N with g_new0 cpu: Replace VIR_FREE with g_free src/cpu/cpu.c | 6 ++--- src/cpu/cpu_map.c | 18 ++ src/cpu/cpu_ppc64.c | 59 - src/cpu/cpu_x86.c | 11 - 4 files changed, 35 insertions(+), 59 deletions(-) -- 2.26.2
Re: [libvirt] [PATCH 03/12] network: Check for active network during networkGetDHCPLeases
On a Friday in 2020, Lin Ma wrote:
It doesn't make sense querying dhcp leases for interfaces against an inactive
network, This patch adds a check to see if the network is active.
Why would the network need to be active? the leases are still there:
$ virsh net-destroy default
Network default destroyed
$ virsh net-dhcp-leases default
Expiry Time MAC address Protocol IP address
Hostname Client ID or DUID
2020-09-11 16:16:59 52:54:00:55:7c:df ipv4 192.168.122.183/24 -
01:52:54:00:55:7c:df
Jano
Signed-off-by: Lin Ma
---
src/network/bridge_driver.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 87d7acab06..1dffc2309f 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -4068,6 +4068,13 @@ networkGetDHCPLeases(virNetworkPtr net,
if (virNetworkGetDHCPLeasesEnsureACL(net->conn, def) < 0)
goto cleanup;
+if (!virNetworkObjIsActive(obj)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+ _("network '%s' is not active"),
+ def->name);
+goto error;
+}
+
/* Retrieve custom leases file location */
custom_lease_file = networkDnsmasqLeaseFileNameCustom(driver, def->bridge);
--
2.26.0
signature.asc
Description: PGP signature
Re: [libvirt] [PATCH 03/12] network: Check for active network during networkGetDHCPLeases
On Fri, Sep 11, 2020 at 03:06:10PM +0800, Lin Ma wrote: > It doesn't make sense querying dhcp leases for interfaces against an inactive > network, This patch adds a check to see if the network is active. > > Signed-off-by: Lin Ma > --- Reviewed-by: Erik Skultety
Re: [libvirt PATCH v2 8/8] cpu: Replace VIR_FREE with g_free
On a Friday in 2020, Tim Wiederhake wrote: Note the use of g_clear_pointer(..., g_free) in ppc64DataClear and virCPUx86Baseline. Signed-off-by: Tim Wiederhake --- src/cpu/cpu.c | 2 +- src/cpu/cpu_ppc64.c | 18 +- src/cpu/cpu_x86.c | 8 3 files changed, 14 insertions(+), 14 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [libvirt PATCH v2 6/8] cpu: Replace VIR_ALLOC with g_new0
On a Friday in 2020, Tim Wiederhake wrote: Signed-off-by: Tim Wiederhake --- src/cpu/cpu.c | 4 +--- src/cpu/cpu_ppc64.c | 19 +-- 2 files changed, 6 insertions(+), 17 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [libvirt PATCH v2 5/8] cpu_map: Remove unnecessary variable in loadData
On a Friday in 2020, Tim Wiederhake wrote: Signed-off-by: Tim Wiederhake --- src/cpu/cpu_map.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [libvirt PATCH v2 4/8] cpu_x86: Use g_auto* in virX86CpuIncompatible
On a Friday in 2020, Tim Wiederhake wrote: Signed-off-by: Tim Wiederhake --- src/cpu/cpu_x86.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [libvirt PATCH v2 3/8] cpu_map: Use g_auto* in loadIncludes
On a Friday in 2020, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake
---
src/cpu/cpu_map.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/src/cpu/cpu_map.c b/src/cpu/cpu_map.c
index 53c8cbba6b..ac98a14e2e 100644
--- a/src/cpu/cpu_map.c
+++ b/src/cpu/cpu_map.c
@@ -125,18 +125,16 @@ loadIncludes(xmlXPathContextPtr ctxt,
return -1;
for (i = 0; i < n; i++) {
-char *filename = virXMLPropString(nodes[i], "filename");
-if (!filename) {
+g_autofree char *filename = NULL;
+
You can assign to filename right away, there's no need to change the condtion.
+if (!(filename = virXMLPropString(nodes[i], "filename"))) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Missing 'filename' in CPU map include"));
return -1;
}
VIR_DEBUG("Finding CPU map include '%s'", filename);
-if (cpuMapLoadInclude(filename, vendorCB, featureCB, modelCB, data) <
0) {
-VIR_FREE(filename);
+if (cpuMapLoadInclude(filename, vendorCB, featureCB, modelCB, data) <
0)
return -1;
-}
-VIR_FREE(filename);
}
Reviewed-by: Ján Tomko
Jano
signature.asc
Description: PGP signature
Re: [libvirt PATCH v2 2/8] cpu_map: Use g_auto* in loadData
On a Friday in 2020, Tim Wiederhake wrote: Signed-off-by: Tim Wiederhake --- src/cpu/cpu_map.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
[libvirt PATCH] qemu: qemuDomainPMSuspendAgent: Don't assign to 'ret' in a conditional
When the guest agent isn't running, we still report success on a PM suspend action even though we logged an error correctly, this is because we poisoned the 'ret' value a few lines above. Fixes: a663a860819287e041c3de672aad1d8543098ecc Signed-off-by: Erik Skultety --- Pushed as trivial. src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2e46931c71..fcdee31971 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16770,7 +16770,7 @@ qemuDomainPMSuspendAgent(virQEMUDriverPtr driver, if (qemuDomainObjBeginAgentJob(driver, vm, QEMU_AGENT_JOB_MODIFY) < 0) return -1; -if ((ret = virDomainObjCheckActive(vm)) < 0) +if (virDomainObjCheckActive(vm) < 0) goto endjob; if (!qemuDomainAgentAvailable(vm, true)) -- 2.26.2
Re: [PATCH] cphp: remove deprecated cpu-add command(s)
What does cphp in the subject mean? On 11/09/2020 14.33, Igor Mammedov wrote: > theses were deprecatedince since 4.0, remove both HMP and QMP variants. deprecated since > Users should use device_add commnad instead. To get list of command > possible CPUs and options, use 'info hotpluggable-cpus' HMP > or query-hotpluggable-cpus QMP command. > > Signed-off-by: Igor Mammedov > --- > include/hw/boards.h | 1 - > include/hw/i386/pc.h| 1 - > include/monitor/hmp.h | 1 - > docs/system/deprecated.rst | 25 + > hmp-commands.hx | 15 -- > hw/core/machine-hmp-cmds.c | 12 - > hw/core/machine-qmp-cmds.c | 12 - > hw/i386/pc.c| 27 -- > hw/i386/pc_piix.c | 1 - > hw/s390x/s390-virtio-ccw.c | 12 - > qapi/machine.json | 24 - > tests/qtest/cpu-plug-test.c | 100 > tests/qtest/test-hmp.c | 1 - > 13 files changed, 21 insertions(+), 211 deletions(-) With the typos fixed: Reviewed-by: Thomas Huth
Re: [libvirt] [PATCH 01/12] qemu: qemuDomainPMSuspendForDuration: Check availability of agent
On Fri, Sep 11, 2020 at 01:39:32PM +0200, Ján Tomko wrote: > On a Friday in 2020, Erik Skultety wrote: > > On Fri, Sep 11, 2020 at 03:06:08PM +0800, Lin Ma wrote: > > > It requires a guest agent configured and running in the domain's guest > > > OS, So check qemu agent during qemuDomainPMSuspendForDuration(). > > > > > > Signed-off-by: Lin Ma > > > --- > > > > qemuDomainPMSuspendAgent later in that same function already checks it, we > > don't need to check it twice. > > > > It only does so after calling qemuDomainQueryWakeupSuspendSupport, which > requires grabbing a MODIFY job and executing a QMP command. Why does a query grab a MODIFY job anyway? > > I think doing this check upfront is reasonable. We will error out as > soon as we know it, just for the price for two extra duplicit lines. True, it will save a fraction of time, I guess I could live with that. More importantly though, this patch uncovered a bug in the code where we report a successful suspend even when the agent is not connected and we logged an error accordingly. Erik > > Jano > > > NACK > > > > > src/qemu/qemu_driver.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > > > index 2e46931c71..bd287f259e 100644 > > > --- a/src/qemu/qemu_driver.c > > > +++ b/src/qemu/qemu_driver.c > > > @@ -16820,6 +16820,9 @@ qemuDomainPMSuspendForDuration(virDomainPtr dom, > > > if (virDomainPMSuspendForDurationEnsureACL(dom->conn, vm->def) < 0) > > > goto cleanup; > > > > > > +if (!qemuDomainAgentAvailable(vm, true)) > > > +goto cleanup; > > > + > > > /* > > > * The case we want to handle here is when QEMU has the API (i.e. > > > * QEMU_CAPS_QUERY_CURRENT_MACHINE is set). Otherwise, do not > > > interfere > > > -- > > > 2.26.0 > > > > >
Re: [libvirt PATCH v2 1/8] cpu_ppc64: Use g_auto* in ppc64MakeCPUData
On a Friday in 2020, Tim Wiederhake wrote: Signed-off-by: Tim Wiederhake --- src/cpu/cpu_ppc64.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
[PATCH] cphp: remove deprecated cpu-add command(s)
theses were deprecatedince since 4.0, remove both HMP and QMP variants.
Users should use device_add commnad instead. To get list of
possible CPUs and options, use 'info hotpluggable-cpus' HMP
or query-hotpluggable-cpus QMP command.
Signed-off-by: Igor Mammedov
---
include/hw/boards.h | 1 -
include/hw/i386/pc.h| 1 -
include/monitor/hmp.h | 1 -
docs/system/deprecated.rst | 25 +
hmp-commands.hx | 15 --
hw/core/machine-hmp-cmds.c | 12 -
hw/core/machine-qmp-cmds.c | 12 -
hw/i386/pc.c| 27 --
hw/i386/pc_piix.c | 1 -
hw/s390x/s390-virtio-ccw.c | 12 -
qapi/machine.json | 24 -
tests/qtest/cpu-plug-test.c | 100
tests/qtest/test-hmp.c | 1 -
13 files changed, 21 insertions(+), 211 deletions(-)
diff --git a/include/hw/boards.h b/include/hw/boards.h
index bc5b82ad20..2163843bdb 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -173,7 +173,6 @@ struct MachineClass {
void (*init)(MachineState *state);
void (*reset)(MachineState *state);
void (*wakeup)(MachineState *state);
-void (*hot_add_cpu)(MachineState *state, const int64_t id, Error **errp);
int (*kvm_type)(MachineState *machine, const char *arg);
void (*smp_parse)(MachineState *ms, QemuOpts *opts);
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index fe52e165b2..ca8ff6cd27 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -137,7 +137,6 @@ extern int fd_bootchk;
void pc_acpi_smi_interrupt(void *opaque, int irq, int level);
-void pc_hot_add_cpu(MachineState *ms, const int64_t id, Error **errp);
void pc_smp_parse(MachineState *ms, QemuOpts *opts);
void pc_guest_info_init(PCMachineState *pcms);
diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
index c986cfd28b..642e9e91f9 100644
--- a/include/monitor/hmp.h
+++ b/include/monitor/hmp.h
@@ -89,7 +89,6 @@ void hmp_chardev_add(Monitor *mon, const QDict *qdict);
void hmp_chardev_change(Monitor *mon, const QDict *qdict);
void hmp_chardev_remove(Monitor *mon, const QDict *qdict);
void hmp_chardev_send_break(Monitor *mon, const QDict *qdict);
-void hmp_cpu_add(Monitor *mon, const QDict *qdict);
void hmp_object_add(Monitor *mon, const QDict *qdict);
void hmp_object_del(Monitor *mon, const QDict *qdict);
void hmp_info_memdev(Monitor *mon, const QDict *qdict);
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 851dbdeb8a..122717cfee 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -284,13 +284,6 @@ The ``query-cpus`` command is replaced by the
``query-cpus-fast`` command.
The ``arch`` output member of the ``query-cpus-fast`` command is
replaced by the ``target`` output member.
-``cpu-add`` (since 4.0)
-'''
-
-Use ``device_add`` for hotplugging vCPUs instead of ``cpu-add``. See
-documentation of ``query-hotpluggable-cpus`` for additional
-details.
-
``query-events`` (since 4.0)
@@ -306,12 +299,6 @@ the 'wait' field, which is only applicable to sockets in
server mode
Human Monitor Protocol (HMP) commands
-
-``cpu-add`` (since 4.0)
-'''
-
-Use ``device_add`` for hotplugging vCPUs instead of ``cpu-add``. See
-documentation of ``query-hotpluggable-cpus`` for additional details.
-
``acl_show``, ``acl_reset``, ``acl_policy``, ``acl_add``, ``acl_remove``
(since 4.0.0)
''
@@ -514,6 +501,12 @@ QEMU Machine Protocol (QMP) commands
The "autoload" parameter has been ignored since 2.12.0. All bitmaps
are automatically loaded from qcow2 images.
+``cpu-add`` (removed in 5.2)
+
+
+Use ``device_add`` for hotplugging vCPUs instead of ``cpu-add``. See
+documentation of ``query-hotpluggable-cpus`` for additional details.
+
Human Monitor Protocol (HMP) commands
-
@@ -523,6 +516,12 @@ The ``hub_id`` parameter of ``hostfwd_add`` /
``hostfwd_remove`` (removed in 5.0
The ``[hub_id name]`` parameter tuple of the 'hostfwd_add' and
'hostfwd_remove' HMP commands has been replaced by ``netdev_id``.
+``cpu-add`` (removed in 5.2)
+
+
+Use ``device_add`` for hotplugging vCPUs instead of ``cpu-add``. See
+documentation of ``query-hotpluggable-cpus`` for additional details.
+
Guest Emulator ISAs
---
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 60f395c276..d1e3e0e1c6 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1761,21 +1761,6 @@ SRST
Executes a qemu-io command on the given block device.
ERST
-{
-.name = "cpu-add",
-.args_type = "id:i",
-.params = "id",
-.help = "add cpu (deprecated, use device_add instead)",
-.cmd= hmp
Re: [PATCH v2] network: Only check kernel added routes in virNetDevIPCheckIPv6Forwarding
Hi Ian,
ACK. I've seen a commented VIR_DEBUG though that you surely want to
remove before pushing. I also really like the verbose commit message
explaining all the reasoning behind the change, thanks for the hard
work.
--
Cedric
On Fri, 2020-09-11 at 21:34 +1000, Ian Wienand wrote:
> The original motivation for adding virNetDevIPCheckIPv6Forwarding
> (00d28a78b5d1f6eaf79f06ac59e31c568af9da37) was that networking routes
> would disappear when ipv6 forwarding was enabled for an interface.
>
> This is a fairly undocumented side-effect of the "accept_ra" sysctl
> for an interface. 1 means the interface will accept_ra's if not
> forwarding, 2 means always accept_RAs; but it is not explained that
> enabling forwarding when accept_ra==1 will also clear any kernel RA
> assigned routes, very likely breaking your networking.
>
> The check to warn about this currently uses netlink to go through all
> the routes and then look at the accept_ra status of the interfaces.
>
> However, it has been noticed that this problem does not affect systems
> where IPv6 RA configuration is handled in userspace, e.g. via tools
> such as NetworkManager. In this case, the error message from libvirt
> is spurious, and modifying the forwarding state will not affect the RA
> state or disable your networking.
>
> If you refer to the function rt6_purge_dflt_routers() in the kernel,
> we can see that the routes being purged are only those with the
> kernel's RTF_ADDRCONF flag set; that is, routes added by the kernel's
> RA handling. Why does it do this? I think this is a Linux
> implementation decision; it has always been like that and there are
> some comments suggesting that it is because a router should be
> statically configured, rather than accepting external configurations.
>
> The solution implemented here is to convert the existing check into a
> walk of /proc/net/ipv6_route and look for routes with this flag set.
> We then check the accept_ra status for the interface, and if enabling
> forwarding would break things raise an error.
>
> This should hopefully avoid "interactive" users, who are likely to be
> using NetworkManager and the like, having false warnings when enabling
> IPv6, but retain the error check for users relying on kernel-based
> IPv6 interface auto-configuration.
>
> Signed-off-by: Ian Wienand
> ---
> src/util/virnetdevip.c | 323 -
> 1 file changed, 124 insertions(+), 199 deletions(-)
>
> diff --git a/src/util/virnetdevip.c b/src/util/virnetdevip.c
> index 7bd5a75f85..e208089bd6 100644
> --- a/src/util/virnetdevip.c
> +++ b/src/util/virnetdevip.c
> @@ -43,8 +43,11 @@
> #ifdef __linux__
> # include
> # include
> +# include
> #endif
>
> +#define PROC_NET_IPV6_ROUTE "/proc/net/ipv6_route"
> +
> #define VIR_FROM_THIS VIR_FROM_NONE
>
> VIR_LOG_INIT("util.netdevip");
> @@ -370,203 +373,6 @@ virNetDevIPRouteAdd(const char *ifname,
> }
>
>
> -static int
> -virNetDevIPGetAcceptRA(const char *ifname)
> -{
> -g_autofree char *path = NULL;
> -g_autofree char *buf = NULL;
> -char *suffix;
> -int accept_ra = -1;
> -
> -path = g_strdup_printf("/proc/sys/net/ipv6/conf/%s/accept_ra",
> - ifname ? ifname : "all");
> -
> -if ((virFileReadAll(path, 512, &buf) < 0) ||
> -(virStrToLong_i(buf, &suffix, 10, &accept_ra) < 0))
> -return -1;
> -
> -return accept_ra;
> -}
> -
> -struct virNetDevIPCheckIPv6ForwardingData {
> -bool hasRARoutes;
> -
> -/* Devices with conflicting accept_ra */
> -char **devices;
> -size_t ndevices;
> -};
> -
> -
> -static int
> -virNetDevIPCheckIPv6ForwardingAddIF(struct
> virNetDevIPCheckIPv6ForwardingData *data,
> -char **ifname)
> -{
> -size_t i;
> -
> -/* add ifname to the array if it's not already there
> - * (ifname is char** so VIR_APPEND_ELEMENT() will move the
> - * original pointer out of the way and avoid having it freed)
> - */
> -for (i = 0; i < data->ndevices; i++) {
> -if (STREQ(data->devices[i], *ifname))
> -return 0;
> -}
> -return VIR_APPEND_ELEMENT(data->devices, data->ndevices, *ifname);
> -}
> -
> -
> -static int
> -virNetDevIPCheckIPv6ForwardingCallback(struct nlmsghdr *resp,
> - void *opaque)
> -{
> -struct rtmsg *rtmsg = NLMSG_DATA(resp);
> -struct virNetDevIPCheckIPv6ForwardingData *data = opaque;
> -struct rtattr *rta_attr;
> -int accept_ra = -1;
> -int ifindex = -1;
> -g_autofree char *ifname = NULL;
> -
> -/* Ignore messages other than route ones */
> -if (resp->nlmsg_type != RTM_NEWROUTE)
> -return 0;
> -
> -/* No need to do anything else for non RA routes */
> -if (rtmsg->rtm_protocol != RTPROT_RA)
> -return 0;
> -
> -rta_attr = (struct rtattr *)nlmsg_find_attr(resp, sizeof(struct rtmsg),
> RTA_OIF);
> -if (rta_attr) {
> -/* This i
Re: [PATCH 1/2] virnuma: Assume numa_bitmask_isbitset() exists
On Fri, Sep 11, 2020 at 01:45:11PM +0200, Michal Privoznik wrote: > This function was introduced in the 2.0.6 release which happened > in December 2010. I think it is safe to assume that all libnuma > we deal with have the function. Yeah, there's a fair bit of historical baggage we could clean up. I mean we're still asking for # define NUMA_VERSION1_COMPATIBILITY 1 which is probably not relevant since RHEL6 > > Signed-off-by: Michal Privoznik > --- > meson.build| 5 + > src/util/virnuma.c | 6 +++--- > 2 files changed, 4 insertions(+), 7 deletions(-) Reviewed-by: Daniel P. Berrangé 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 2/2] virnuma: Use numa_nodes_ptr when checking available NUMA nodes
On Fri, Sep 11, 2020 at 01:45:12PM +0200, Michal Privoznik wrote: > In v6.7.0-rc1~86 I've tried to fix a problem where we were not > detecting NUMA nodes properly because we misused behaviour of a > libnuma API and as it turned out the behaviour was correct for > hosts with 64 CPUs in one NUMA node. So I changed the code to use > nodemask_isset(&numa_all_nodes, ..) instead and it fixed the > problem on such hosts. However, what I did not realize is that > numa_all_nodes does not reflect all NUMA nodes visible to > userspace, it contains only those nodes that the process > (libvirtd) an allocate memory from, which can be only a subset of > all NUMA nodes. The bitmask that contains all NUMA nodes visible > to userspace and which one I should have used is: numa_nodes_ptr. > For curious ones: > > https://github.com/numactl/numactl/commit/4a22f2238234155e11e3e2717c011864722b767b > > And as I was fixing virNumaGetNodeCPUs() I came to realize that > we already have a function that wraps the correct bitmask: > virNumaNodeIsAvailable(). > > Fixes: 24d7d85208f812a45686b32a0561cc9c5c9a49c9 > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1876956 > Signed-off-by: Michal Privoznik > --- > src/util/virnuma.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Daniel P. Berrangé Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
[PATCH 2/2] virnuma: Use numa_nodes_ptr when checking available NUMA nodes
In v6.7.0-rc1~86 I've tried to fix a problem where we were not
detecting NUMA nodes properly because we misused behaviour of a
libnuma API and as it turned out the behaviour was correct for
hosts with 64 CPUs in one NUMA node. So I changed the code to use
nodemask_isset(&numa_all_nodes, ..) instead and it fixed the
problem on such hosts. However, what I did not realize is that
numa_all_nodes does not reflect all NUMA nodes visible to
userspace, it contains only those nodes that the process
(libvirtd) an allocate memory from, which can be only a subset of
all NUMA nodes. The bitmask that contains all NUMA nodes visible
to userspace and which one I should have used is: numa_nodes_ptr.
For curious ones:
https://github.com/numactl/numactl/commit/4a22f2238234155e11e3e2717c011864722b767b
And as I was fixing virNumaGetNodeCPUs() I came to realize that
we already have a function that wraps the correct bitmask:
virNumaNodeIsAvailable().
Fixes: 24d7d85208f812a45686b32a0561cc9c5c9a49c9
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1876956
Signed-off-by: Michal Privoznik
---
src/util/virnuma.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/virnuma.c b/src/util/virnuma.c
index b8cf8b4510..39f0f30917 100644
--- a/src/util/virnuma.c
+++ b/src/util/virnuma.c
@@ -260,7 +260,7 @@ virNumaGetNodeCPUs(int node,
*cpus = NULL;
-if (!nodemask_isset(&numa_all_nodes, node)) {
+if (!virNumaNodeIsAvailable(node)) {
VIR_DEBUG("NUMA topology for cell %d is not available, ignoring",
node);
return -2;
}
--
2.26.2
[PATCH 1/2] virnuma: Assume numa_bitmask_isbitset() exists
This function was introduced in the 2.0.6 release which happened
in December 2010. I think it is safe to assume that all libnuma
we deal with have the function.
Signed-off-by: Michal Privoznik
---
meson.build| 5 +
src/util/virnuma.c | 6 +++---
2 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/meson.build b/meson.build
index bcb978292b..195d7cd784 100644
--- a/meson.build
+++ b/meson.build
@@ -1239,13 +1239,10 @@ else
intl_dep = dependency('', required: false)
endif
+numactl_version = '2.0.6'
numactl_dep = cc.find_library('numa', required: get_option('numactl'))
if numactl_dep.found()
conf.set('WITH_NUMACTL', 1)
-
- if cc.has_function('numa_bitmask_isbitset', dependencies: [ numactl_dep ])
-conf.set('WITH_NUMA_BITMASK_ISBITSET', 1)
- endif
endif
openwsman_version = '2.2.3'
diff --git a/src/util/virnuma.c b/src/util/virnuma.c
index 2872ce3c5e..b8cf8b4510 100644
--- a/src/util/virnuma.c
+++ b/src/util/virnuma.c
@@ -423,7 +423,7 @@ virNumaGetMaxCPUs(void)
}
-#if WITH_NUMACTL && WITH_NUMA_BITMASK_ISBITSET
+#if WITH_NUMACTL
/**
* virNumaNodeIsAvailable:
* @node: node to check
@@ -493,7 +493,7 @@ virNumaGetDistances(int node,
return 0;
}
-#else /* !(WITH_NUMACTL && WITH_NUMA_BITMASK_ISBITSET) */
+#else /* !WITH_NUMACTL */
bool
virNumaNodeIsAvailable(int node)
@@ -518,7 +518,7 @@ virNumaGetDistances(int node G_GNUC_UNUSED,
VIR_DEBUG("NUMA distance information isn't available on this host");
return 0;
}
-#endif /* !(WITH_NUMACTL && WITH_NUMA_BITMASK_ISBITSET) */
+#endif /* !WITH_NUMACTL */
/* currently all the huge page stuff below is linux only */
--
2.26.2
[PATCH 0/2] Definitive fix for NUMA nodes detection
See 2/2 for explanation. Michal Prívozník (2): virnuma: Assume numa_bitmask_isbitset() exists virnuma: Use numa_nodes_ptr when checking available NUMA nodes meson.build| 5 + src/util/virnuma.c | 8 2 files changed, 5 insertions(+), 8 deletions(-) -- 2.26.2
[libvirt PATCH 07/21] util: Use glib memory functions in virLogFilterNew
Signed-off-by: Tim Wiederhake
---
src/util/virlog.c | 24
1 file changed, 8 insertions(+), 16 deletions(-)
diff --git a/src/util/virlog.c b/src/util/virlog.c
index f6d0c6c050..de36da1a4a 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -1308,7 +1308,6 @@ virLogFilterNew(const char *match,
virLogPriority priority)
{
virLogFilterPtr ret = NULL;
-char *mdup = NULL;
size_t mlen = strlen(match);
if (priority < VIR_LOG_DEBUG || priority > VIR_LOG_ERROR) {
@@ -1317,23 +1316,16 @@ virLogFilterNew(const char *match,
return NULL;
}
+ret = g_new0(virLogFilter, 1);
+ret->priority = priority;
+
/* We must treat 'foo' as equiv to '*foo*' for g_pattern_match
- * todo substring matches, so add 2 extra bytes
+ * substring matches, so add 2 extra bytes
*/
-if (VIR_ALLOC_N_QUIET(mdup, mlen + 3) < 0)
-return NULL;
-
-mdup[0] = '*';
-memcpy(mdup + 1, match, mlen);
-mdup[mlen + 1] = '*';
-
-if (VIR_ALLOC_QUIET(ret) < 0) {
-VIR_FREE(mdup);
-return NULL;
-}
-
-ret->match = mdup;
-ret->priority = priority;
+ret->match = g_new0(char, mlen + 3);
+ret->match[0] = '*';
+memcpy(ret->match + 1, match, mlen);
+ret->match[mlen + 1] = '*';
return ret;
}
--
2.26.2
[libvirt PATCH 00/21] Remove VIR_{ALLOC, ALLOC_N, REALLOC_N}_QUIET macros.
See also https://www.redhat.com/archives/libvir-list/2020-September/msg00540.html and https://www.redhat.com/archives/libvir-list/2020-September/msg00610.html Tim Wiederhake (21): tests: Use glib memory functions in virpcimock.c rpc: Use glib memory functions in virNetMessageSaveError util: Use glib memory functions in virBitmapNewQuiet util: Use glib memory functions in virErrorCopyNew util: Use glib memory functions in virLastErrorObject util: Use glib memory functions in virSaveLastError util: Use glib memory functions in virLogFilterNew util: Use glib memory functions in virThreadCreateFull util: Remove VIR_ALLOC_QUIET tests: Use glib memory function in testConfRoundTrip util: Use glib memory functions in virDevMapperGetTargetsImpl util: Use glib memory functions in virJSONValueGetArrayAsBitmap util: Remove VIR_ALLOC_N_QUIET qemu: Use glib memory functions in qemuDomainMasterKeyReadFile qemu: Use glib memory functions in qemuDomainLogContextRead qemu: Use glib memory functions in qemuProcessReadLog tests: Use glib memory functions in add_fd tests: Use glib memory functions in pci_driver_new tools: Use glib memory functions in vshCompleterFilter util: Use glib memory functions in virFileGetXAttrQuiet util: Remove VIR_REALLOC_N_QUIET src/qemu/qemu_domain.c | 4 ++-- src/qemu/qemu_process.c | 2 +- src/rpc/virnetmessage.c | 4 ++-- src/util/viralloc.h | 44 - src/util/virbitmap.c| 10 ++ src/util/virdevmapper.c | 4 +--- src/util/virerror.c | 30 ++-- src/util/virfile.c | 15 +- src/util/virjson.c | 4 ++-- src/util/virlog.c | 24 -- src/util/virthread.c| 5 + tests/virconftest.c | 29 ++- tests/virpcimock.c | 24 -- tools/vsh.c | 10 -- 14 files changed, 61 insertions(+), 148 deletions(-) -- 2.26.2
[libvirt PATCH 06/21] util: Use glib memory functions in virSaveLastError
Signed-off-by: Tim Wiederhake --- src/util/virerror.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/util/virerror.c b/src/util/virerror.c index 99a0855b51..df4205043a 100644 --- a/src/util/virerror.c +++ b/src/util/virerror.c @@ -405,8 +405,7 @@ virSaveLastError(void) virErrorPtr to; int saved_errno = errno; -if (VIR_ALLOC_QUIET(to) < 0) -return NULL; +to = g_new0(virError, 1); virCopyLastError(to); errno = saved_errno; -- 2.26.2
[libvirt PATCH 08/21] util: Use glib memory functions in virThreadCreateFull
Signed-off-by: Tim Wiederhake
---
src/util/virthread.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/src/util/virthread.c b/src/util/virthread.c
index 7f23399835..814f5313aa 100644
--- a/src/util/virthread.c
+++ b/src/util/virthread.c
@@ -253,11 +253,8 @@ int virThreadCreateFull(virThreadPtr thread,
if ((err = pthread_attr_init(&attr)) != 0)
goto cleanup;
-if (VIR_ALLOC_QUIET(args) < 0) {
-err = ENOMEM;
-goto cleanup;
-}
+args = g_new0(struct virThreadArgs, 1);
args->func = func;
args->name = g_strdup(name);
args->worker = worker;
--
2.26.2
[libvirt PATCH 21/21] util: Remove VIR_REALLOC_N_QUIET
Signed-off-by: Tim Wiederhake --- src/util/viralloc.h | 15 --- 1 file changed, 15 deletions(-) diff --git a/src/util/viralloc.h b/src/util/viralloc.h index c60148724d..a50cd5d632 100644 --- a/src/util/viralloc.h +++ b/src/util/viralloc.h @@ -105,21 +105,6 @@ void virDisposeString(char **strptr) */ #define VIR_REALLOC_N(ptr, count) virReallocN(&(ptr), sizeof(*(ptr)), (count)) -/** - * VIR_REALLOC_N_QUIET: - * @ptr: pointer to hold address of allocated memory - * @count: number of elements to allocate - * - * Re-allocate an array of 'count' elements, each sizeof(*ptr) - * bytes long and store the address of allocated memory in - * 'ptr'. If 'ptr' grew, the added memory is uninitialized. - * - * This macro is safe to use on arguments with side effects. - * - * Returns 0 on success, aborts on OOM - */ -#define VIR_REALLOC_N_QUIET(ptr, count) VIR_REALLOC_N(ptr, count) - /** * VIR_EXPAND_N: * @ptr: pointer to hold address of allocated memory -- 2.26.2
[libvirt PATCH 18/21] tests: Use glib memory functions in pci_driver_new
Signed-off-by: Tim Wiederhake
---
tests/virpcimock.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tests/virpcimock.c b/tests/virpcimock.c
index c8aa8f3f01..787172d24c 100644
--- a/tests/virpcimock.c
+++ b/tests/virpcimock.c
@@ -701,12 +701,12 @@ pci_driver_new(const char *name, ...)
if ((device = va_arg(args, int)) == -1)
ABORT("Invalid vendor device pair for driver %s", name);
-if (VIR_REALLOC_N_QUIET(driver->vendor, driver->len + 1) < 0 ||
-VIR_REALLOC_N_QUIET(driver->device, driver->len + 1) < 0)
-ABORT_OOM();
-
+driver->vendor = g_renew(int, driver->vendor, driver->len + 1);
driver->vendor[driver->len] = vendor;
+
+driver->device = g_renew(int, driver->device, driver->len + 1);
driver->device[driver->len] = device;
+
driver->len++;
}
--
2.26.2
[libvirt PATCH 17/21] tests: Use glib memory functions in add_fd
Signed-off-by: Tim Wiederhake
---
tests/virpcimock.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/tests/virpcimock.c b/tests/virpcimock.c
index 6b1f2f2a5a..c8aa8f3f01 100644
--- a/tests/virpcimock.c
+++ b/tests/virpcimock.c
@@ -305,11 +305,7 @@ add_fd(int fd, const char *path)
fd, path, cb.fd, cb.path);
}
-if (VIR_REALLOC_N_QUIET(callbacks, nCallbacks + 1) < 0) {
-errno = ENOMEM;
-return -1;
-}
-
+callbacks = g_renew(struct fdCallback, callbacks, nCallbacks + 1);
callbacks[nCallbacks].path = g_strdup(path);
callbacks[nCallbacks++].fd = fd;
--
2.26.2
[libvirt PATCH 04/21] util: Use glib memory functions in virErrorCopyNew
Signed-off-by: Tim Wiederhake
---
src/util/virerror.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/src/util/virerror.c b/src/util/virerror.c
index 507a29f50f..4ba99defbb 100644
--- a/src/util/virerror.c
+++ b/src/util/virerror.c
@@ -223,15 +223,14 @@ virCopyError(virErrorPtr from,
virErrorPtr
virErrorCopyNew(virErrorPtr err)
{
-virErrorPtr ret;
+g_autoptr(virError) ret = NULL;
-if (VIR_ALLOC_QUIET(ret) < 0)
-return NULL;
+ret = g_new0(virError, 1);
if (virCopyError(err, ret) < 0)
-VIR_FREE(ret);
+return NULL;
-return ret;
+return g_steal_pointer(&ret);
}
--
2.26.2
[libvirt PATCH 15/21] qemu: Use glib memory functions in qemuDomainLogContextRead
Signed-off-by: Tim Wiederhake --- src/qemu/qemu_domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 6efc6102bd..785cee6f18 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -6357,7 +6357,7 @@ ssize_t qemuDomainLogContextRead(qemuDomainLogContextPtr ctxt, buf[got] = '\0'; -ignore_value(VIR_REALLOC_N_QUIET(buf, got + 1)); +buf = g_renew(char, buf, got + 1); buflen = got; } -- 2.26.2
[libvirt PATCH 01/21] tests: Use glib memory functions in virpcimock.c
Signed-off-by: Tim Wiederhake
---
tests/virpcimock.c | 10 +++---
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/tests/virpcimock.c b/tests/virpcimock.c
index 92b6f810d8..6b1f2f2a5a 100644
--- a/tests/virpcimock.c
+++ b/tests/virpcimock.c
@@ -427,9 +427,7 @@ pci_device_create_iommu(const struct pciDevice *dev,
return;
}
-if (VIR_ALLOC_QUIET(iommuGroup) < 0)
-ABORT_OOM();
-
+iommuGroup = g_new0(struct pciIommuGroup, 1);
iommuGroup->iommu = dev->iommuGroup;
iommuGroup->nDevicesBoundToVFIO = 0; /* No device bound to VFIO by default
*/
@@ -469,8 +467,7 @@ pci_device_new_from_stub(const struct pciDevice *data)
c = strchr(c, ':');
}
-if (VIR_ALLOC_QUIET(dev) < 0)
-ABORT_OOM();
+dev = g_new0(struct pciDevice, 1);
configSrc = g_strdup_printf("%s/virpcitestdata/%s.config", abs_srcdir, id);
@@ -694,8 +691,7 @@ pci_driver_new(const char *name, ...)
int vendor, device;
g_autofree char *driverpath = NULL;
-if (VIR_ALLOC_QUIET(driver) < 0)
-ABORT_OOM();
+driver = g_new0(struct pciDriver, 1);
driver->name = g_strdup(name);
if (!(driverpath = pci_driver_get_path(driver, NULL, true)))
ABORT_OOM();
--
2.26.2
[libvirt PATCH 14/21] qemu: Use glib memory functions in qemuDomainMasterKeyReadFile
Signed-off-by: Tim Wiederhake --- src/qemu/qemu_domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 0ed132a829..6efc6102bd 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -511,7 +511,7 @@ qemuDomainMasterKeyReadFile(qemuDomainObjPrivatePtr priv) goto error; } -ignore_value(VIR_REALLOC_N_QUIET(masterKey, masterKeyLen)); +masterKey = g_renew(uint8_t, masterKey, masterKeyLen); priv->masterKey = masterKey; priv->masterKeyLen = masterKeyLen; -- 2.26.2
[libvirt PATCH 20/21] util: Use glib memory functions in virFileGetXAttrQuiet
Signed-off-by: Tim Wiederhake
---
src/util/virfile.c | 15 +--
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 90156845df..61d2c16072 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -4327,8 +4327,7 @@ virFileGetXAttrQuiet(const char *path,
const char *name,
char **value)
{
-char *buf = NULL;
-int ret = -1;
+g_autofree char *buf = NULL;
/* We might be racing with somebody who sets the same attribute. */
while (1) {
@@ -4337,15 +4336,14 @@ virFileGetXAttrQuiet(const char *path,
/* The first call determines how many bytes we need to allocate. */
if ((need = getxattr(path, name, NULL, 0)) < 0)
-goto cleanup;
+return -1;
-if (VIR_REALLOC_N_QUIET(buf, need + 1) < 0)
-goto cleanup;
+buf = g_renew(char, buf, need + 1);
if ((got = getxattr(path, name, buf, need)) < 0) {
if (errno == ERANGE)
continue;
-goto cleanup;
+return -1;
}
buf[got] = '\0';
@@ -4353,10 +4351,7 @@ virFileGetXAttrQuiet(const char *path,
}
*value = g_steal_pointer(&buf);
-ret = 0;
- cleanup:
-VIR_FREE(buf);
-return ret;
+return 0;
}
/**
--
2.26.2
[libvirt PATCH 09/21] util: Remove VIR_ALLOC_QUIET
Signed-off-by: Tim Wiederhake --- src/util/viralloc.h | 14 -- 1 file changed, 14 deletions(-) diff --git a/src/util/viralloc.h b/src/util/viralloc.h index 833f85f62e..ae967f2556 100644 --- a/src/util/viralloc.h +++ b/src/util/viralloc.h @@ -75,20 +75,6 @@ void virDisposeString(char **strptr) */ #define VIR_ALLOC(ptr) virAlloc(&(ptr), sizeof(*(ptr))) -/** - * VIR_ALLOC_QUIET: - * @ptr: pointer to hold address of allocated memory - * - * Allocate sizeof(*ptr) bytes of memory and store - * the address of allocated memory in 'ptr'. Fill the - * newly allocated memory with zeros. - * - * This macro is safe to use on arguments with side effects. - * - * Returns 0 on success, aborts on OOM - */ -#define VIR_ALLOC_QUIET(ptr) VIR_ALLOC(ptr) - /** * VIR_ALLOC_N: * @ptr: pointer to hold address of allocated memory -- 2.26.2
[libvirt PATCH 05/21] util: Use glib memory functions in virLastErrorObject
Signed-off-by: Tim Wiederhake
---
src/util/virerror.c | 18 ++
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/src/util/virerror.c b/src/util/virerror.c
index 4ba99defbb..99a0855b51 100644
--- a/src/util/virerror.c
+++ b/src/util/virerror.c
@@ -237,15 +237,17 @@ virErrorCopyNew(virErrorPtr err)
static virErrorPtr
virLastErrorObject(void)
{
-virErrorPtr err;
+g_autoptr(virError) err = NULL;
+
err = virThreadLocalGet(&virLastErr);
-if (!err) {
-if (VIR_ALLOC_QUIET(err) < 0)
-return NULL;
-if (virThreadLocalSet(&virLastErr, err) < 0)
-VIR_FREE(err);
-}
-return err;
+if (err)
+return g_steal_pointer(&err);
+
+err = g_new0(virError, 1);
+if (virThreadLocalSet(&virLastErr, err) < 0)
+return NULL;
+
+return g_steal_pointer(&err);
}
--
2.26.2
