Re: [PATCH] util: Add phys_port_name support on virPCIGetNetName
On 12/10/20 11:51 AM, Adrian Chiris wrote: Hi, Would be great to get a pair of eyes on this Patch, Thanks! I've looked at it several times and every time would just end up shaking my head wondering why there isn't one definitive symlink in the VF's sysfs for the netdev of the physical port. (Part of my lack of response from the last time is that I didn't know how to respond since I didn't understand why such roundabout logic should be needed to answer such a basic question, decided I should look at it again before responding, and then forgot about it :-() Anyway, this time I'm determined to not get up until I've got it figured out (or at least understand exactly what I don't have figured out)... -Original Message- From: Dmytro Linkin Sent: Tuesday, October 27, 2020 10:58 AM To: libvir-list@redhat.com Cc: Laine Stump ; Adrian Chiris ; Moshe Levi Subject: Re: [PATCH] util: Add phys_port_name support on virPCIGetNetName On Mon, Sep 28, 2020 at 12:56:12PM +0300, Dmytro Linkin wrote: On Tue, Sep 22, 2020 at 08:31:15AM -0400, Laine Stump wrote: On 8/28/20 6:53 AM, Dmytro Linkin wrote: Current virPCIGetNetName() logic is to get net device name by checking it's phys_port_id, if caller provide it, or by it's index (eg, by it's position at sysfs net directory). This approach worked fine up until linux kernel version 5.8, where NVIDIA Mellanox driver implemented linking of VFs' representors to PCI device in switchdev mode. This mean that device's sysfs net directory will hold multiple net devices. Ex.: $ ls '/sys/bus/pci/devices/:82:00.0/net' ens1f0 eth0 eth1 Most switch devices support phys_port_name instead of phys_port_id, so virPCIGetNetName() will try to get PF name by it's index - 0. The problem here is that the PF nedev entry may not be the first. To fix that, for switch devices, we introduce a new logic to select the PF uplink netdev according to the content of phys_port_name. Extend virPCIGetNetName() with physPortNameRegex variable to get proper device by it's phys_port_name scheme, for ex., "p[0-9]+$" to get PF, "pf[0-9]+vf[0-9]+$" to get VF or "p1$" to get exact net device. So now virPCIGetNetName() logic work in following sequence: - filter by phys_port_id, if it's provided, or - filter by phys_port_name, if it's regex provided, or - get net device by it's index (position) in sysfs net directory. Also, make getting content of iface sysfs files more generic. Signed-off-by: Dmytro Linkin Reviewed-by: Adrian Chiris [...] +/* Represents format of PF's phys_port_name in switchdev mode: + * 'p%u' or 'p%us%u'. New line checked since value is readed from sysfs file. + */ +# define VIR_PF_PHYS_PORT_NAME_REGEX ((char +*)"(p[0-9]+$)|(p[0-9]+s[0-9]+$)") + I've come back to look at this patch several times since it was posted (sorry for the extreme delay in responding), but just can't figure out what it's doing with this regex and why the regex is necessary. Not having access to the hardware that it works with is a bit of a problem, but perhaps I could get a better idea if you gave a full example of sysfs contents? My concern with using a regex is that it might work just fine when using one method for net device naming, but break if that was changed. Also, it seems counterintuitive that it would be necessary to look for a device with a name matching a specific pattern; why isn't there simply a single symbolic link somewhere in the sysfs tree for the net device that just directly points at its physical port? That would be so much simpler and more reliable (or at least it would give the *perception* of being more reliable). I'll emphasize that regex is being used for phys_port_name, NOT net device name (they are not the same). Anyway, I'll give an example. Lets look how virPCIGetNetName() currently work. At first it try to read phys_port_id of every net device in net folder, like: $ cd '/sys/bus/pci/devices/:80:02.0/:82:00.0/' $ cat net/ens1f0/phys_port_id Imagine we have single pci dual port nic (I hope named it right), eg. net folder holds net devices for both ports, so read operation will be successfull and function will return name of first or second port. For single port or dual pci nics (or for drivers which didn't implemented phys_port_id) read fails and function fallback to picking up device by it's index, which not really fine (I'll describe it later) but work 'cause net folder usualy contains only one net device. But kernel patch brought third case which not work. Here are ifaces of ConnectX-5 NIC: $ ls -l /sys/class/net | cut -d ' ' -f 9- ens1f0 -> ../../devices/pci:80/:80:02.0/:82:00.0/net/ens1f0 ens1f1 -> ../../devices/pci:80/:80:02.0/:82:00.1/net/ens1f1 ens1f2 -> ../../devices/pci:80/:80:02.0/:82:00.2/net/ens1f2 ens1f3 -> ../../devices/pci:80/:80:02.0/:82:00.3/net/ens1f3 ens1f0_0 -> ../../devices/pci:80/:80:02.0/:82:00.0/net/ens1f0_0 ens1f0_1 ->
Re: [PATCH 4/4] lxd_domain: Require that VIR_LXC_DOMAIN_NAMESPACE_SOURCE_NONE is zero
On 12/16/20 4:13 PM, Michal Privoznik wrote: Our parser code relies on the fact that VIR_LXC_DOMAIN_NAMESPACE_SOURCE_NONE has value of zero and thus uses g_new0(). But strictly speaking, this is not mandated by the enum typedef. Fix that. Is there really any C compiler that doesn't make the first value a 0 by default? (If so, I wonder why?) Reviewed-by: Laine Stump
Re: [PATCH 3/4] lxc: Rework lxcDomainDefNamespaceParse()
On 12/16/20 4:13 PM, Michal Privoznik wrote: While fixing our schema for I've looked into the parser and realized it could use some treating. Signed-off-by: Michal Privoznik Huh. Some er... "interesting" code you're replacing there :-) Reviewed-by: Laine Stump
Re: [PATCH 2/4] lxc: Allow NULL argument to lxcDomainDefNamespaceFree()
On 12/16/20 4:12 PM, Michal Privoznik wrote: As all other free functions, NULL should be accepted. Even though there currently is no caller that would pass NULL, there will be in future patches. Signed-off-by: Michal Privoznik Reviewed-by: Laine Stump
Re: [PATCH 1/4] schema: Allow lxc:namepsace children to appear individually
On 12/16/20 4:12 PM, Michal Privoznik wrote: Since its introduction in v1.2.19-rc1~8 our schema mandates that LXC domain namespace child elements appear either all three at once or not at all: This is not mandated by our parser though. Neither by code that later uses it (virLXCProcessSetupNamespaces()). Relax the schema. Reviewed-by: Laine Stump
[RFC PATCH 4/6] conf: Added NFS XML format/parse methods
Signed-off-by: Ryan Gahagan
---
src/conf/domain_conf.c | 33 +
1 file changed, 33 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b301ac0a08..565ca680c9 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6888,6 +6888,23 @@ virDomainStorageNetworkParseHosts(xmlNodePtr node,
}
+static void
+virDomainStorageNetworkParseNFS(xmlNodePtr node,
+ xmlXPathContextPtr ctxt,
+ virStorageSourcePtr src)
+{
+xmlNodePtr nfsnode = NULL;
+VIR_XPATH_NODE_AUTORESTORE(ctxt);
+
+ctxt->node = node;
+
+if ((nfsnode = virXPathNode("./nfs", ctxt))) {
+src->nfs_user = virXMLPropString(nfsnode, "user");
+src->nfs_group = virXMLPropString(nfsnode, "group");
+}
+}
+
+
static int
virDomainHostdevSubsysSCSIHostDefParseXML(xmlNodePtr sourcenode,
xmlXPathContextPtr ctxt,
@@ -8252,6 +8269,9 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node,
if (virDomainStorageNetworkParseHosts(node, ctxt, >hosts,
>nhosts) < 0)
return -1;
+if (src->protocol == VIR_STORAGE_NET_PROTOCOL_NFS)
+virDomainStorageNetworkParseNFS(node, ctxt, src);
+
virStorageSourceNetworkAssignDefaultPorts(src);
virStorageSourceInitiatorParseXML(ctxt, >initiator);
@@ -23805,6 +23825,19 @@ virDomainDiskSourceFormatNetwork(virBufferPtr attrBuf,
virBufferAddLit(childBuf, "/>\n");
}
+if (src->protocol == VIR_STORAGE_NET_PROTOCOL_NFS &&
+(src->nfs_user || src->nfs_group)) {
+virBufferAddLit(childBuf, "nfs_user)
+virBufferEscapeString(childBuf, " user='%s'", src->nfs_user);
+if (src->nfs_group)
+virBufferEscapeString(childBuf, " group='%s'", src->nfs_group);
+
+virBufferAddLit(childBuf, "/>\n");
+}
+
+
virBufferEscapeString(childBuf, "\n", src->snapshot);
virBufferEscapeString(childBuf, "\n", src->configFile);
--
2.29.0
[RFC PATCH 1/6] conf: Add NFS disk protocol
Signed-off-by: Ryan Gahagan
---
src/libxl/libxl_conf.c| 1 +
src/libxl/xen_xl.c| 1 +
src/qemu/qemu_block.c | 3 +++
src/qemu/qemu_command.c | 1 +
src/qemu/qemu_domain.c| 10 ++
src/qemu/qemu_snapshot.c | 3 +++
src/util/virstoragefile.c | 6 ++
src/util/virstoragefile.h | 1 +
8 files changed, 26 insertions(+)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 00748e21e8..6a8ae27f54 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -941,6 +941,7 @@ libxlMakeNetworkDiskSrcStr(virStorageSourcePtr src,
case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG:
case VIR_STORAGE_NET_PROTOCOL_SSH:
case VIR_STORAGE_NET_PROTOCOL_VXHS:
+case VIR_STORAGE_NET_PROTOCOL_NFS:
case VIR_STORAGE_NET_PROTOCOL_LAST:
case VIR_STORAGE_NET_PROTOCOL_NONE:
virReportError(VIR_ERR_NO_SUPPORT,
diff --git a/src/libxl/xen_xl.c b/src/libxl/xen_xl.c
index ba0942601f..17b93d0f5c 100644
--- a/src/libxl/xen_xl.c
+++ b/src/libxl/xen_xl.c
@@ -1600,6 +1600,7 @@ xenFormatXLDiskSrcNet(virStorageSourcePtr src)
case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG:
case VIR_STORAGE_NET_PROTOCOL_SSH:
case VIR_STORAGE_NET_PROTOCOL_VXHS:
+case VIR_STORAGE_NET_PROTOCOL_NFS:
case VIR_STORAGE_NET_PROTOCOL_LAST:
case VIR_STORAGE_NET_PROTOCOL_NONE:
virReportError(VIR_ERR_NO_SUPPORT,
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 4640e339c0..b224a550f3 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -1180,6 +1180,7 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr
src,
return NULL;
break;
+case VIR_STORAGE_NET_PROTOCOL_NFS:
case VIR_STORAGE_NET_PROTOCOL_NONE:
case VIR_STORAGE_NET_PROTOCOL_LAST:
virReportEnumRangeError(virStorageNetProtocol, src->protocol);
@@ -2111,6 +2112,7 @@ qemuBlockGetBackingStoreString(virStorageSourcePtr src,
case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG:
case VIR_STORAGE_NET_PROTOCOL_RBD:
case VIR_STORAGE_NET_PROTOCOL_VXHS:
+case VIR_STORAGE_NET_PROTOCOL_NFS:
case VIR_STORAGE_NET_PROTOCOL_SSH:
case VIR_STORAGE_NET_PROTOCOL_LAST:
case VIR_STORAGE_NET_PROTOCOL_NONE:
@@ -2502,6 +2504,7 @@
qemuBlockStorageSourceCreateGetStorageProps(virStorageSourcePtr src,
case VIR_STORAGE_NET_PROTOCOL_NBD:
case VIR_STORAGE_NET_PROTOCOL_ISCSI:
case VIR_STORAGE_NET_PROTOCOL_VXHS:
+case VIR_STORAGE_NET_PROTOCOL_NFS:
case VIR_STORAGE_NET_PROTOCOL_HTTP:
case VIR_STORAGE_NET_PROTOCOL_HTTPS:
case VIR_STORAGE_NET_PROTOCOL_FTP:
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index b06a086e18..c58f39ebf1 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1044,6 +1044,7 @@ qemuBuildNetworkDriveStr(virStorageSourcePtr src,
_("'ssh' protocol is not yet supported"));
return NULL;
+case VIR_STORAGE_NET_PROTOCOL_NFS:
case VIR_STORAGE_NET_PROTOCOL_LAST:
case VIR_STORAGE_NET_PROTOCOL_NONE:
virReportError(VIR_ERR_INTERNAL_ERROR,
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index bfb6e23942..692bc925c6 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4626,6 +4626,14 @@ qemuDomainValidateStorageSource(virStorageSourcePtr src,
return -1;
}
+/* NFS protocol may only be used if current QEMU supports blockdev */
+if (actualType == VIR_STORAGE_TYPE_NETWORK &&
+src->protocol == VIR_STORAGE_NET_PROTOCL_NFS &&
+!blockdev) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("'nfs' protocol is not supported with this QEMU
binary"));
+}
+
return 0;
}
@@ -9630,6 +9638,8 @@ qemuDomainPrepareStorageSourceTLS(virStorageSourcePtr src,
case VIR_STORAGE_NET_PROTOCOL_FTP:
case VIR_STORAGE_NET_PROTOCOL_FTPS:
case VIR_STORAGE_NET_PROTOCOL_TFTP:
+case VIR_STORAGE_NET_PROTOCOL_NFS:
+/* Assumed NFS doesn't support TLS (needs Kerberos) */
case VIR_STORAGE_NET_PROTOCOL_SSH:
if (src->haveTLS == VIR_TRISTATE_BOOL_YES) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 15494c3415..7e89a8839b 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -413,6 +413,7 @@
qemuSnapshotPrepareDiskExternalInactive(virDomainSnapshotDiskDefPtr snapdisk,
case VIR_STORAGE_NET_PROTOCOL_TFTP:
case VIR_STORAGE_NET_PROTOCOL_SSH:
case VIR_STORAGE_NET_PROTOCOL_VXHS:
+case VIR_STORAGE_NET_PROTOCOL_NFS:
case VIR_STORAGE_NET_PROTOCOL_LAST:
virReportError(VIR_ERR_INTERNAL_ERROR,
_("external inactive snapshots are not supported on
"
@@ -501,6 +502,7 @@
[RFC PATCH 5/6] qemu: Added NFS JSON props methods
Signed-off-by: Ryan Gahagan
---
src/qemu/qemu_block.c | 48 +-
src/qemu/qemu_domain.c | 39 ++
2 files changed, 86 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index b224a550f3..c0f47eedc0 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -674,6 +674,42 @@ qemuBlockStorageSourceGetVxHSProps(virStorageSourcePtr src,
}
+static virJSONValuePtr
+qemuBlockStorageSourceGetNFSProps(virStorageSourcePtr src)
+{
+g_autoptr(virJSONValue) server = NULL;
+virJSONValuePtr ret = NULL;
+
+if (src->nhosts != 1) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("NFS protocol accepts only one host"));
+return NULL;
+}
+
+if (!(server =
qemuBlockStorageSourceBuildJSONInetSocketAddress(>hosts[0])))
+return NULL;
+
+/* NFS disk specification example:
+ * { driver:"nfs",
+ * user: "0",
+ * group: "0",
+ * server: {type:"tcp", host:"1.2.3.4", port:}}
+ */
+ignore_value(virJSONValueObjectCreate(,
+ "a:server", , NULL));
+
+if (src->nfs_uid != -1 &&
+virJSONValueObjectAdd(ret, "i:user", src->nfs_uid, NULL) < 0)
+return NULL;
+
+if (src->nfs_gid != -1 &&
+virJSONValueObjectAdd(ret, "i:group", src->nfs_gid, NULL) < 0)
+return NULL;
+
+return ret;
+}
+
+
static virJSONValuePtr
qemuBlockStorageSourceGetCURLProps(virStorageSourcePtr src,
bool onlytarget)
@@ -1181,6 +1217,11 @@
qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src,
break;
case VIR_STORAGE_NET_PROTOCOL_NFS:
+driver = "nfs";
+if (!(fileprops = qemuBlockStorageSourceGetNFSProps(src)))
+return NULL;
+break;
+
case VIR_STORAGE_NET_PROTOCOL_NONE:
case VIR_STORAGE_NET_PROTOCOL_LAST:
virReportEnumRangeError(virStorageNetProtocol, src->protocol);
@@ -2500,11 +2541,16 @@
qemuBlockStorageSourceCreateGetStorageProps(virStorageSourcePtr src,
return -1;
break;
+case VIR_STORAGE_NET_PROTOCOL_NFS:
+driver = "nfs";
+if (!(location = qemuBlockStorageSourceGetNFSProps(src)))
+return -1;
+break;
+
/* unsupported/impossible */
case VIR_STORAGE_NET_PROTOCOL_NBD:
case VIR_STORAGE_NET_PROTOCOL_ISCSI:
case VIR_STORAGE_NET_PROTOCOL_VXHS:
-case VIR_STORAGE_NET_PROTOCOL_NFS:
case VIR_STORAGE_NET_PROTOCOL_HTTP:
case VIR_STORAGE_NET_PROTOCOL_HTTPS:
case VIR_STORAGE_NET_PROTOCOL_FTP:
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 692bc925c6..65c751e1d9 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -9598,6 +9598,42 @@
qemuProcessPrepareStorageSourceTLSNBD(virStorageSourcePtr src,
}
+/* qemuPrepareStorageSourceNFS:
+ * @src: source for a disk
+ *
+ * If src is an NFS source, translate nfs_user and nfs_group
+ * into a uid and gid field. If these strings are empty (ie "")
+ * then provide the hypervisor default uid and gid.
+ */
+ static int
+ qemuDomainPrepareStorageSourceNFS(virStorageSourcePtr src)
+ {
+if (virStorageSourceGetActualType(src) != VIR_STORAGE_TYPE_NETWORK)
+return 0;
+
+if ((virStorageNetProtocol) src->protocol != VIR_STORAGE_NET_PROTOCOL_NFS)
+return 0;
+
+if (src->nfs_user) {
+if (virGetUserID(src->nfs_user, >nfs_uid) < 0)
+return -1;
+} else {
+/* -1 indicates default UID */
+src->nfs_uid = -1;
+}
+
+if (src->nfs_group) {
+if (virGetGroupID(src->nfs_group, >nfs_gid) < 0)
+return -1;
+} else {
+/* -1 indicates default GID */
+src->nfs_gid = -1;
+}
+
+return 0;
+}
+
+
/* qemuProcessPrepareStorageSourceTLS:
* @source: source for a disk
* @cfg: driver configuration
@@ -10409,6 +10445,9 @@
qemuDomainPrepareStorageSourceBlockdev(virDomainDiskDefPtr disk,
priv) < 0)
return -1;
+if (qemuDomainPrepareStorageSourceNFS(src) < 0)
+return -1;
+
return 0;
}
--
2.29.0
[RFC PATCH 6/6] util: Added a backing store NFS parser
Signed-off-by: Ryan Gahagan
---
src/util/virstoragefile.c | 37 +
1 file changed, 37 insertions(+)
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index cff6dabd9e..341eac2eb7 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -3805,6 +3805,42 @@ virStorageSourceParseBackingJSONVxHS(virStorageSourcePtr
src,
}
+static int
+virStorageSourceParseBackingJSONNFS(virStorageSourcePtr src,
+virJSONValuePtr json,
+const char *jsonstr G_GNUC_UNUSED,
+int opaque G_GNUC_UNUSED)
+{
+virJSONValuePtr server = virJSONValueObjectGetObject(json, "server");
+int gotUID = virJSONValueObjectGetNumberInt(json, "user", (int
*)(>nfs_uid));
+int gotGID = virJSONValueObjectGetNumberInt(json, "group", (int
*)(>nfs_gid));
+
+if (!server) {
+virReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("missing 'server' attribute in JSON backing
definition for NFS volume"));
+return -1;
+}
+
+if (gotUID < 0 || gotGID < 0) {
+virReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("missing 'user' or 'group' attribute in JSON backing
definition for NFS volume"));
+return -1;
+}
+
+src->type = VIR_STORAGE_TYPE_NETWORK;
+src->protocol = VIR_STORAGE_NET_PROTOCOL_NFS;
+
+src->hosts = g_new0(virStorageNetHostDef, 1);
+src->nhosts = 1;
+
+if (virStorageSourceParseBackingJSONInetSocketAddress(src->hosts,
+ server) < 0)
+return -1;
+
+return 0;
+}
+
+
static int
virStorageSourceParseBackingJSONNVMe(virStorageSourcePtr src,
virJSONValuePtr json,
@@ -3864,6 +3900,7 @@ static const struct virStorageSourceJSONDriverParser
jsonParsers[] = {
{"ssh", false, virStorageSourceParseBackingJSONSSH, 0},
{"rbd", false, virStorageSourceParseBackingJSONRBD, 0},
{"raw", true, virStorageSourceParseBackingJSONRaw, 0},
+{"nfs", false, virStorageSourceParseBackingJSONNFS, 0},
{"vxhs", false, virStorageSourceParseBackingJSONVxHS, 0},
{"nvme", false, virStorageSourceParseBackingJSONNVMe, 0},
};
--
2.29.0
RFC PATCH: Issue 90 (Test Clarification)
We addressed the feedback from our previous RFC patch for the most part.
Under src/util/virstoragefile.c, we left a cast to an integer pointer
that Peter mentioned because we were unable to provide a better
solution. We've written some tests for our code but our testing
environment is not working locally (meson doesn't even recognize the
project as a meson build project) and so we can't regenerate output or
test our tests.
It's probably bad practice but the only solution we could think of that
would allow us to check our tests was just to email you what we've got.
Sorry if it's not up to standard, but please let us know if there's a
better way to do it or if you spot any problems in these tests.
Under tests/qemuxml2xmltest.c:
DO_TEST_CAPS_LATEST("disk-network-nfs");
The same line would be in tests/qemuxml2argvtest.c
We created the file tests/qemuxml2argvdata/disk-network-nfs.xml:
QEMUGuest1
c7a5fdbd-edaf-9455-926a-d65c16db1809
219136
219136
1
hvm
destroy
restart
destroy
/usr/bin/qemu-system-x86_64
eb90327c-8302-4725-9e1b-4e85ed4dc251
We have under tests/qemublocktest.c:
TEST_JSON_FORMAT_NET(“\n”
“ \n”
“ \n”
"\n”);
and
TEST_IMAGE_CREATE(“network-nfs-qcow2”, NULL);
And finally under tests/virstoragetest.c:
TEST_BACKING_PARSE(“json:{\”file\”:{\”driver\”:\”nfs\”,”
“\”user\”:\”USER\”,”
“\”group\”:\”GROUP\”,”
“\”server\”: { \”host\”:\”example.com\”,”
“\”port\”:\”2049\””
”}”
“}”
“}”,
“\n”
“ \n”
“ \n”
“\n”);
Again, sorry if this looks awful. Please let us know if there's a more
practical way to do this because submitting a commit with these tests
would guarantee that the tests fail and the commit wouldn't be mergeable
due to our environment issues, or if you see anything wrong with these
tests.
[RFC PATCH 3/6] docs: added rng schema and formatdomain for NFS
Signed-off-by: Ryan Gahagan
---
docs/formatdomain.rst | 11 +-
docs/schemas/domaincommon.rng | 38 +++
2 files changed, 48 insertions(+), 1 deletion(-)
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index 512939679b..23a7bca643 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -2370,6 +2370,14 @@ paravirtualized driver is specified via the ``disk``
element.
+
+
+
+
+
+
+
+
@@ -2491,7 +2499,7 @@ paravirtualized driver is specified via the ``disk``
element.
``network``
The ``protocol`` attribute specifies the protocol to access to the
requested image. Possible values are "nbd", "iscsi", "rbd", "sheepdog",
- "gluster", "vxhs", "http", "https", "ftp", ftps", or "tftp".
+ "gluster", "vxhs", "nfs", "http", "https", "ftp", ftps", or "tftp".
For any ``protocol`` other than ``nbd`` an additional attribute ``name``
is mandatory to specify which volume/image will be used.
@@ -2601,6 +2609,7 @@ paravirtualized driver is specified via the ``disk``
element.
sheepdog one of the sheepdog servers (default is localhost:7000) zero or
one 7000
gluster a server running glusterd daemonone or
more ( :since:`Since 2.1.0` ), just one prior to that 24007
vxhs a server running Veritas HyperScale daemon only
one
+ nfs a server running Network File Systemonly
one ( :since:`Since 7.0.0` )2049
===
gluster supports "tcp", "rdma", "unix" as valid values for the transport
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 795b654feb..6b321b1ca3 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1774,6 +1774,27 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
@@ -2039,6 +2060,22 @@
+
+
+
+
+
+nfs
+
+
+
+
+
+
+
+
+
+
network
@@ -2053,6 +2090,7 @@
+
--
2.29.0
[RFC PATCH 2/6] util: Added nfs params to virStorageSource
Signed-off-by: Ryan Gahagan
---
src/util/virstoragefile.c | 8
src/util/virstoragefile.h | 5 +
2 files changed, 13 insertions(+)
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 5a57e5d12d..cff6dabd9e 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -2446,6 +2446,11 @@ virStorageSourceCopy(const virStorageSource *src,
def->ssh_host_key_check_disabled = src->ssh_host_key_check_disabled;
def->ssh_user = g_strdup(src->ssh_user);
+def->nfs_user = g_strdup(src->nfs_user);
+def->nfs_group = g_strdup(src->nfs_group);
+def->nfs_uid = src->nfs_uid;
+def->nfs_gid = src->nfs_gid;
+
return g_steal_pointer();
}
@@ -2686,6 +2691,9 @@ virStorageSourceClear(virStorageSourcePtr def)
VIR_FREE(def->ssh_user);
+VIR_FREE(def->nfs_user);
+VIR_FREE(def->nfs_group);
+
virStorageSourceInitiatorClear(>initiator);
/* clear everything except the class header as the object APIs
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index c5d5f0233a..64fc519f87 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -385,6 +385,11 @@ struct _virStorageSource {
/* these must not be used apart from formatting the output JSON in the
qemu driver */
char *ssh_user;
bool ssh_host_key_check_disabled;
+
+char *nfs_user;
+char *nfs_group;
+uid_t nfs_uid;
+gid_t nfs_gid;
};
G_DEFINE_AUTOPTR_CLEANUP_FUNC(virStorageSource, virObjectUnref);
--
2.29.0
Re: [PATCH 6/5] lxc: skip the netdev autogenerated name counter past existing devices
On 12/16/20 4:27 PM, Michal Privoznik wrote:
On 12/16/20 9:13 PM, Laine Stump wrote:
the lxc driver uses virNetDevGenerateName() for its veth device names
since patch 2dd0fb492, so it should be using virNetDevReserveName()
during daemon restart/reconnect to skip over the device names that are
in use.
Signed-off-by: Laine Stump
---
I meant to mention this during review of the abovementioned patch, but
forgot.
(NB: a couple days ago I *removed* similar code from this same spot,
but it was trying to reserve the name of macvlan devices; a macvlan
device is moved into the container's namespace at startup, so it is
not visible to the host anyway. This new case is for the 1/2 of a veth
pair that does remain in the host's namespace
(type='bridge|network|ethernet' use a veth pair)
src/lxc/lxc_process.c | 24
1 file changed, 24 insertions(+)
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index 0f7c929535..a842ac91c5 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -1640,6 +1640,30 @@
virLXCProcessReconnectNotifyNets(virDomainDefPtr def)
for (i = 0; i < def->nnets; i++) {
virDomainNetDefPtr net = def->nets[i];
+ /* type='bridge|network|ethernet' interfaces may be using an
+ * autogenerated netdev name, so we should update the counter
+ * for autogenerated names to skip past this one.
+ */
+ switch (virDomainNetGetActualType(net)) {
+ case VIR_DOMAIN_NET_TYPE_BRIDGE:
+ case VIR_DOMAIN_NET_TYPE_NETWORK:
+ case VIR_DOMAIN_NET_TYPE_ETHERNET:
+ virNetDevReserveName(net->ifname);
+ break;
+ case VIR_DOMAIN_NET_TYPE_DIRECT:
+ case VIR_DOMAIN_NET_TYPE_USER:
+ case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
+ 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_HOSTDEV:
+ case VIR_DOMAIN_NET_TYPE_UDP:
+ case VIR_DOMAIN_NET_TYPE_VDPA:
+ case VIR_DOMAIN_NET_TYPE_LAST:
+ break;
+ }
+
I remember Peter being picky about switch()-es and (almost) always he
wanted me to add the default case with virReportEnumRangeError() despite
the variable passed to switch() being verified earlier. IIUC his
reasoning was that if we had a memory being overwritten somewhere it's
better to error out (I say it's better to crash), but since I don't care
that much, this could have:
default:
virReportEnumRangeError(virDomainNetType,
virDomainNetGetActualType(net));
return;
But if we add a default case to the switch, we won't get all the nice
compile-time errors when we add a new value to the enum and forget to
add it to every switch, will we?
At your discretion.
if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
if (!conn && !(conn = virGetConnectNetwork()))
continue;
Michal
Re: [PATCH 1/5] util: fix tap device name auto-generation for FreeBSD
On 12/16/20 2:27 AM, Laine Stump wrote:
The Linux implementation of virNetDevCreate() no longer requires a
template ifname (e.g. "vnet%d") when it is called, but just generates
a new name if ifname is empty. The FreeBSD implementation requires
that the caller actually fill in a template ifname, and will fail if
ifname is empty. Since we want to eliminate all the special code in
callers that is setting the template name, we need to make the
behavior of the FreeBSD virNetDevCreate() match the behavior of the
Linux virNetDevCreate().
The simplest way to do this is to use the new virNetDevGenerateName()
function - if ifname is empty it generates a new name with the proper
prefix, and if it's not empty, it leaves it alone.
Signed-off-by: Laine Stump
---
src/util/virnetdevtap.c | 35 ++-
1 file changed, 6 insertions(+), 29 deletions(-)
diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c
index 88ad321627..cca2f614fe 100644
--- a/src/util/virnetdevtap.c
+++ b/src/util/virnetdevtap.c
@@ -308,7 +308,6 @@ int virNetDevTapCreate(char **ifname,
int s;
struct ifreq ifr;
int ret = -1;
-char *newifname = NULL;
if (tapfdSize > 1) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
@@ -316,6 +315,12 @@ int virNetDevTapCreate(char **ifname,
goto cleanup;
}
+/* auto-generate an unused name for the new device (this
+ * is NOP if a name has been provided)
+ */
+if (virNetDevGenerateName(ifname, VIR_NET_DEV_GEN_NAME_VNET) < 0)
+return -1;
+
This line ^^ contains some trailing spaces.
/* As FreeBSD determines interface type by name,
* we have to create 'tap' interface first and
* then rename it to 'vnet'
Michal
Re: [PATCH 0/5] Followup to virNetDevGenerateName() patches
On 12/16/20 2:27 AM, Laine Stump wrote: A few issues came up during review of this series that were better fixed in cleanups rather than requiring yet another round of review. (A couple of things I noticed after the other patches were already pushed). Laine Stump (5): util: fix tap device name auto-generation for FreeBSD bhyve: remove redundant code that adds "template" netdev name qemu: remove redundant code that adds "template" netdev name util: simplify virNetDevMacVLanCreateWithVPortProfile() util: minor comment/formatting changes to virNetDevTapCreate() src/bhyve/bhyve_command.c | 7 src/qemu/qemu_interface.c | 18 +++ src/util/virnetdevmacvlan.c | 64 ++--- src/util/virnetdevtap.c | 46 +++--- 4 files changed, 25 insertions(+), 110 deletions(-) Reviewed-by: Michal Privoznik Michal
Re: [PATCH 6/5] lxc: skip the netdev autogenerated name counter past existing devices
On 12/16/20 9:13 PM, Laine Stump wrote:
the lxc driver uses virNetDevGenerateName() for its veth device names
since patch 2dd0fb492, so it should be using virNetDevReserveName()
during daemon restart/reconnect to skip over the device names that are
in use.
Signed-off-by: Laine Stump
---
I meant to mention this during review of the abovementioned patch, but forgot.
(NB: a couple days ago I *removed* similar code from this same spot,
but it was trying to reserve the name of macvlan devices; a macvlan
device is moved into the container's namespace at startup, so it is
not visible to the host anyway. This new case is for the 1/2 of a veth
pair that does remain in the host's namespace
(type='bridge|network|ethernet' use a veth pair)
src/lxc/lxc_process.c | 24
1 file changed, 24 insertions(+)
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index 0f7c929535..a842ac91c5 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -1640,6 +1640,30 @@ virLXCProcessReconnectNotifyNets(virDomainDefPtr def)
for (i = 0; i < def->nnets; i++) {
virDomainNetDefPtr net = def->nets[i];
+/* type='bridge|network|ethernet' interfaces may be using an
+ * autogenerated netdev name, so we should update the counter
+ * for autogenerated names to skip past this one.
+ */
+switch (virDomainNetGetActualType(net)) {
+case VIR_DOMAIN_NET_TYPE_BRIDGE:
+case VIR_DOMAIN_NET_TYPE_NETWORK:
+case VIR_DOMAIN_NET_TYPE_ETHERNET:
+virNetDevReserveName(net->ifname);
+break;
+case VIR_DOMAIN_NET_TYPE_DIRECT:
+case VIR_DOMAIN_NET_TYPE_USER:
+case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
+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_HOSTDEV:
+case VIR_DOMAIN_NET_TYPE_UDP:
+case VIR_DOMAIN_NET_TYPE_VDPA:
+case VIR_DOMAIN_NET_TYPE_LAST:
+break;
+}
+
I remember Peter being picky about switch()-es and (almost) always he
wanted me to add the default case with virReportEnumRangeError() despite
the variable passed to switch() being verified earlier. IIUC his
reasoning was that if we had a memory being overwritten somewhere it's
better to error out (I say it's better to crash), but since I don't care
that much, this could have:
default:
virReportEnumRangeError(virDomainNetType,
virDomainNetGetActualType(net));
return;
At your discretion.
if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
if (!conn && !(conn = virGetConnectNetwork()))
continue;
Michal
[PATCH 2/4] lxc: Allow NULL argument to lxcDomainDefNamespaceFree()
As all other free functions, NULL should be accepted. Even though
there currently is no caller that would pass NULL, there will be
in future patches.
Signed-off-by: Michal Privoznik
---
src/lxc/lxc_domain.c | 4
1 file changed, 4 insertions(+)
diff --git a/src/lxc/lxc_domain.c b/src/lxc/lxc_domain.c
index df60519fca..707262336b 100644
--- a/src/lxc/lxc_domain.c
+++ b/src/lxc/lxc_domain.c
@@ -195,6 +195,10 @@ lxcDomainDefNamespaceFree(void *nsdata)
{
size_t i;
lxcDomainDefPtr lxcDef = nsdata;
+
+if (!lxcDef)
+return;
+
for (i = 0; i < VIR_LXC_DOMAIN_NAMESPACE_LAST; i++)
g_free(lxcDef->ns_val[i]);
g_free(nsdata);
--
2.26.2
[PATCH 4/4] lxd_domain: Require that VIR_LXC_DOMAIN_NAMESPACE_SOURCE_NONE is zero
Our parser code relies on the fact that
VIR_LXC_DOMAIN_NAMESPACE_SOURCE_NONE has value of zero and thus
uses g_new0(). But strictly speaking, this is not mandated by
the enum typedef. Fix that.
Signed-off-by: Michal Privoznik
---
src/lxc/lxc_domain.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/lxc/lxc_domain.h b/src/lxc/lxc_domain.h
index 319f83338f..3b5adcbe1c 100644
--- a/src/lxc/lxc_domain.h
+++ b/src/lxc/lxc_domain.h
@@ -35,7 +35,7 @@ typedef enum {
} virLXCDomainNamespace;
typedef enum {
-VIR_LXC_DOMAIN_NAMESPACE_SOURCE_NONE,
+VIR_LXC_DOMAIN_NAMESPACE_SOURCE_NONE = 0,
VIR_LXC_DOMAIN_NAMESPACE_SOURCE_NAME,
VIR_LXC_DOMAIN_NAMESPACE_SOURCE_PID,
VIR_LXC_DOMAIN_NAMESPACE_SOURCE_NETNS,
--
2.26.2
[PATCH 3/4] lxc: Rework lxcDomainDefNamespaceParse()
While fixing our schema for I've looked into the
parser and realized it could use some treating.
Signed-off-by: Michal Privoznik
---
src/lxc/lxc_domain.c | 40 ++--
1 file changed, 22 insertions(+), 18 deletions(-)
diff --git a/src/lxc/lxc_domain.c b/src/lxc/lxc_domain.c
index 707262336b..255bfab495 100644
--- a/src/lxc/lxc_domain.c
+++ b/src/lxc/lxc_domain.c
@@ -208,26 +208,31 @@ static int
lxcDomainDefNamespaceParse(xmlXPathContextPtr ctxt,
void **data)
{
-lxcDomainDefPtr lxcDef = g_new0(lxcDomainDef, 1);
+lxcDomainDefPtr lxcDef = NULL;
g_autofree xmlNodePtr *nodes = NULL;
-bool uses_lxc_ns = false;
VIR_XPATH_NODE_AUTORESTORE(ctxt)
-int feature;
int n;
size_t i;
+int ret = -1;
if ((n = virXPathNodeSet("./lxc:namespace/*", ctxt, )) < 0)
-goto error;
-uses_lxc_ns |= n > 0;
+return -1;
+
+if (n == 0)
+return 0;
+
+lxcDef = g_new0(lxcDomainDef, 1);
for (i = 0; i < n; i++) {
g_autofree char *tmp = NULL;
+int feature;
+
if ((feature = virLXCDomainNamespaceTypeFromString(
(const char *)nodes[i]->name)) < 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-_("unsupported Namespace feature: %s"),
-nodes[i]->name);
-goto error;
+ _("unsupported Namespace feature: %s"),
+ nodes[i]->name);
+goto cleanup;
}
ctxt->node = nodes[i];
@@ -235,31 +240,30 @@ lxcDomainDefNamespaceParse(xmlXPathContextPtr ctxt,
if (!(tmp = virXMLPropString(nodes[i], "type"))) {
virReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("No lxc environment type specified"));
-goto error;
+goto cleanup;
}
if ((lxcDef->ns_source[feature] =
virLXCDomainNamespaceSourceTypeFromString(tmp)) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unknown LXC namespace source '%s'"),
tmp);
-goto error;
+goto cleanup;
}
if (!(lxcDef->ns_val[feature] =
virXMLPropString(nodes[i], "value"))) {
virReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("No lxc environment type specified"));
-goto error;
+goto cleanup;
}
}
-if (uses_lxc_ns)
-*data = lxcDef;
-else
-g_free(lxcDef);
-return 0;
- error:
+
+*data = g_steal_pointer();
+ret = 0;
+
+ cleanup:
lxcDomainDefNamespaceFree(lxcDef);
-return -1;
+return ret;
}
--
2.26.2
[PATCH 1/4] schema: Allow lxc:namepsace children to appear individually
Since its introduction in v1.2.19-rc1~8 our schema mandates that LXC domain namespace child elements appear either all three at once or not at all: This is not mandated by our parser though. Neither by code that later uses it (virLXCProcessSetupNamespaces()). Relax the schema. Signed-off-by: Michal Privoznik --- docs/schemas/domaincommon.rng | 66 +++ 1 file changed, 36 insertions(+), 30 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 795b654feb..17e25f14f2 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -6821,36 +6821,42 @@ --> http://libvirt.org/schemas/domain/lxc/1.0;> - - - - - netns - name - pid - - - - - - - - name - pid - - - - - - - - name - pid - - - - - + + + + + +netns +name +pid + + + + + + + + + +name +pid + + + + + + + + + +name +pid + + + + + + -- 2.26.2
[PATCH 0/4] lxc: Couple of improvements
I've noticed these thanks to question on the list: https://www.redhat.com/archives/libvir-list/2020-December/msg00699.html where I suggested to use namespace inheritance. Michal Prívozník (4): schema: Allow lxc:namepsace children to appear individually lxc: Allow NULL argument to lxcDomainDefNamespaceFree() lxc: Rework lxcDomainDefNamespaceParse() lxd_domain: Require that VIR_LXC_DOMAIN_NAMESPACE_SOURCE_NONE is zero docs/schemas/domaincommon.rng | 66 +++ src/lxc/lxc_domain.c | 44 +-- src/lxc/lxc_domain.h | 2 +- 3 files changed, 63 insertions(+), 49 deletions(-) -- 2.26.2
[PATCH 6/5] lxc: skip the netdev autogenerated name counter past existing devices
the lxc driver uses virNetDevGenerateName() for its veth device names
since patch 2dd0fb492, so it should be using virNetDevReserveName()
during daemon restart/reconnect to skip over the device names that are
in use.
Signed-off-by: Laine Stump
---
I meant to mention this during review of the abovementioned patch, but forgot.
(NB: a couple days ago I *removed* similar code from this same spot,
but it was trying to reserve the name of macvlan devices; a macvlan
device is moved into the container's namespace at startup, so it is
not visible to the host anyway. This new case is for the 1/2 of a veth
pair that does remain in the host's namespace
(type='bridge|network|ethernet' use a veth pair)
src/lxc/lxc_process.c | 24
1 file changed, 24 insertions(+)
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index 0f7c929535..a842ac91c5 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -1640,6 +1640,30 @@ virLXCProcessReconnectNotifyNets(virDomainDefPtr def)
for (i = 0; i < def->nnets; i++) {
virDomainNetDefPtr net = def->nets[i];
+/* type='bridge|network|ethernet' interfaces may be using an
+ * autogenerated netdev name, so we should update the counter
+ * for autogenerated names to skip past this one.
+ */
+switch (virDomainNetGetActualType(net)) {
+case VIR_DOMAIN_NET_TYPE_BRIDGE:
+case VIR_DOMAIN_NET_TYPE_NETWORK:
+case VIR_DOMAIN_NET_TYPE_ETHERNET:
+virNetDevReserveName(net->ifname);
+break;
+case VIR_DOMAIN_NET_TYPE_DIRECT:
+case VIR_DOMAIN_NET_TYPE_USER:
+case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
+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_HOSTDEV:
+case VIR_DOMAIN_NET_TYPE_UDP:
+case VIR_DOMAIN_NET_TYPE_VDPA:
+case VIR_DOMAIN_NET_TYPE_LAST:
+break;
+}
+
if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
if (!conn && !(conn = virGetConnectNetwork()))
continue;
--
2.29.2
Re: [PATCHv3 1/3] util:netlink: Enable virNetlinkNewLink to support veth
On 12/16/20 1:01 AM, Shi Lei wrote:
Signed-off-by: Shi Lei
---
src/util/virnetlink.c | 14 ++
src/util/virnetlink.h | 1 +
2 files changed, 15 insertions(+)
diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
index fdd3a6a4..8625b896 100644
--- a/src/util/virnetlink.c
+++ b/src/util/virnetlink.c
@@ -24,6 +24,7 @@
#include
#include
+#include
^^ This #include had to be moved down inside the #if defined(WITH_LIBNL)
to avoid build failures on mingw, MacOS, and FreeBSD. I squashed in that
change before pushing.
#include "virnetlink.h"
#include "virnetdev.h"
@@ -535,6 +536,19 @@ virNetlinkNewLink(const char *ifname,
NETLINK_MSG_NEST_END(nl_msg, infodata);
}
+if (STREQ(type, "veth") && extra_args && extra_args->veth_peer) {
+struct nlattr *infoveth = NULL;
+
+NETLINK_MSG_NEST_START(nl_msg, infodata, IFLA_INFO_DATA);
+NETLINK_MSG_NEST_START(nl_msg, infoveth, VETH_INFO_PEER);
+nlmsg_reserve(nl_msg, sizeof(struct ifinfomsg), 0);
+NETLINK_MSG_PUT(nl_msg, IFLA_IFNAME,
+(strlen(extra_args->veth_peer) + 1),
+extra_args->veth_peer);
+NETLINK_MSG_NEST_END(nl_msg, infoveth);
+NETLINK_MSG_NEST_END(nl_msg, infodata);
+}
+
NETLINK_MSG_NEST_END(nl_msg, linkinfo);
if (extra_args) {
diff --git a/src/util/virnetlink.h b/src/util/virnetlink.h
index 7121eac4..7c4ed202 100644
--- a/src/util/virnetlink.h
+++ b/src/util/virnetlink.h
@@ -84,6 +84,7 @@ struct _virNetlinkNewLinkData {
const int *ifindex; /* The index for the 'link' device */
const virMacAddr *mac; /* The MAC address of the device */
const uint32_t *macvlan_mode; /* The mode of macvlan */
+const char *veth_peer; /* The peer name for veth */
};
int virNetlinkNewLink(const char *ifname,
Re: [PATCHv3 0/3] Use netlink to create veth device pair when netlink is supported
On 12/16/20 1:01 AM, Shi Lei wrote: V2 here: https://www.redhat.com/archives/libvir-list/2020-November/msg01239.html Since V2: * Remove the argument 'status' of virNetDevVethCreateInternal * fix a memory leak in virLXCProcessSetupInterfaceTap V1 here: https://www.redhat.com/archives/libvir-list/2020-November/msg00898.html Since V1: * Include rather than introducing enum VETH_INFO_* * Have complete functions within an #ifdefs rather than putting #ifdefs in function Shi Lei (3): util:netlink: Enable virNetlinkNewLink to support veth util:veth: Create veth device pair by netlink lxc: fix a memory leak src/lxc/lxc_process.c| 4 +- src/util/virnetdevveth.c | 126 ++- src/util/virnetlink.c| 14 + src/util/virnetlink.h| 1 + 4 files changed, 89 insertions(+), 56 deletions(-) Reviewed-by: Laine Stump for all 3 (with one tiny change to patch 1 to fix build failure in mingw, MacOS and FreeBSD). They're pushed now. Thanks!
live migration is not using secondary interface
A naive 'virsh migrate --live domU xen+tcp://cross-over-ip' uses the ordinary uplink instead of the requested IP address. According to the documentation an additional option has to be specified to really use the other network interface. However, neither 'tcp://cross-over-ip' nor 'xenmigr:cross-over-ip/' works, the migration does not start. 'xenmigr' was removed with 1dac5f06a0341e8087dc33af75c8352d77a4 and 7ed598438687feaddaf0a653d7cbb8a1c1ad4933, but the docs were not updated. This leaves 'tcp://ip'. What change needs to be done in the driver to support a secondary interface? Olaf pgptGQ7HdJhyz.pgp Description: Digitale Signatur von OpenPGP
Re: [PATCH 2/2] virnetdevopenvswitch: Try to unescape ovs-vsctl reply in one specific case
On 12/16/20 1:45 PM, Michal Privoznik wrote:
During testing of my patch v6.10.0-rc1~221 it was found that
'ovs-vsctl get Interface $name name' or
'ovs-vsctl find Interface options:vhost-server-path=$path'
may return a string in double quotes, e.g. "vhost-user1". Later
investigation of openvswitch code showed, that early versions
(like 1.3.0) have somewhat restrictive set of safe characters
(isalpha() || '_' || '-' || '.'), which is then refined with
increasing version. For instance, version 2.11.4 has: isalnum()
|| '_' || '-' || '.'. If the string that ovs-vsctl wants to
output contains any other character it is escaped. You want to be
looking at ovsdb_atom_to_string() which handles outputting of a
single string and calls string_needs_quotes() and possibly
json_serialize_string() in openvswitch code base.
Since the interfaces are usually named "vhost-userN" we are
facing a problem where with one version we get the name in double
quotes and with another we get plain name without funny business.
Because of json involved I thought, let's make ovs-vsctl output
into JSON format and then use our JSON parser, but guess what -
ovs-vsctl ignores --format=json.
Yuck. Is this a known/reported problem? Is it true with current
ovs-vsctl, or just with old versions? (I guess in the end it doesn't
really matter, because we have to support old and new anyway :-/)
But with a little help of
g_strdup_printf() it can be turned into JSON.
Fixes: e4c29e2904197472919d050c67acfd59f0144bbc
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1767013
Signed-off-by: Michal Privoznik
---
src/libvirt_private.syms | 1 +
src/util/virnetdevopenvswitch.c | 47 +
src/util/virnetdevopenvswitch.h | 4 +++
tests/virnetdevopenvswitchtest.c | 52
4 files changed, 104 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c7c37d9689..583fc5800e 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2676,6 +2676,7 @@ virNetDevOpenvswitchGetVhostuserIfname;
virNetDevOpenvswitchInterfaceGetMaster;
virNetDevOpenvswitchInterfaceParseStats;
virNetDevOpenvswitchInterfaceStats;
+virNetDevOpenvswitchMaybeUnescapeReply;
virNetDevOpenvswitchRemovePort;
virNetDevOpenvswitchSetMigrateData;
virNetDevOpenvswitchSetTimeout;
diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c
index 7eabaa763d..14fa294ae1 100644
--- a/src/util/virnetdevopenvswitch.c
+++ b/src/util/virnetdevopenvswitch.c
@@ -460,6 +460,48 @@ virNetDevOpenvswitchInterfaceGetMaster(const char *ifname,
char **master)
}
+/**
+ * virNetDevOpenvswitchMaybeUnescapeReply:
+ * @reply: a string to unescape
+ *
+ * Depending on ovs-vsctl version a string might be escaped. For instance:
+ * -version 2.11.4 allows only is_alpha(), an underscore, a dash or a dot,
+ * -version 2.14.0 allows only is_alnum(), an underscore, a dash or a dot,
+ * any other character causes the string to be escaped.
+ *
+ * What this function does, is it checks whether @reply string consists solely
+ * from safe, not escaped characters (as defined by version 2.14.0) and if not
+ * an error is reported. If @reply is a string enclosed in double quotes, but
+ * otherwise safe those double quotes are removed.
+ *
+ * Returns: 0 on success,
+ * -1 otherwise (with error reported).
+ */
+int
+virNetDevOpenvswitchMaybeUnescapeReply(char *reply)
+{
+g_autoptr(virJSONValue) json = NULL;
+g_autofree char *jsonStr = NULL;
+const char *tmp = NULL;
+size_t replyLen = strlen(reply);
+
+if (*reply != '"')
+return 0;
+
+jsonStr = g_strdup_printf("{\"name\": %s}", reply);
+if (!(json = virJSONValueFromString(jsonStr)))
+return -1;
+
+if (!(tmp = virJSONValueObjectGetString(json, "name"))) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Malformed ovs-vsctl output"));
+return -1;
+}
Tricky!
There are other ways to unescape a string, but this one gets a vote for
novelty :-)
Reviewed-by: Laine Stump
+
+return virStrcpy(reply, tmp, replyLen);
+}
+
+
/**
* virNetDevOpenvswitchGetVhostuserIfname:
* @path: the path of the unix socket
@@ -522,6 +564,11 @@ virNetDevOpenvswitchGetVhostuserIfname(const char *path,
return 0;
}
+if (virNetDevOpenvswitchMaybeUnescapeReply(*ifname) < 0) {
+VIR_FREE(*ifname);
+return -1;
+}
+
return 1;
}
diff --git a/src/util/virnetdevopenvswitch.h b/src/util/virnetdevopenvswitch.h
index 8409aa92ac..3571708582 100644
--- a/src/util/virnetdevopenvswitch.h
+++ b/src/util/virnetdevopenvswitch.h
@@ -60,6 +60,10 @@ int virNetDevOpenvswitchInterfaceStats(const char *ifname,
int virNetDevOpenvswitchInterfaceGetMaster(const char *ifname, char **master)
ATTRIBUTE_NONNULL(1) G_GNUC_WARN_UNUSED_RESULT;
+int
+virNetDevOpenvswitchMaybeUnescapeReply(char
Re: [PATCH 1/2] virNetDevOpenvswitchGetVhostuserIfname: Actually use @path to lookup interface
On 12/16/20 1:45 PM, Michal Privoznik wrote:
In v6.10.0-rc1~221 I wanted to make virNetDevOpenvswitchGetVhostuserIfname()
lookup interface name even for vhostuser interfaces with mode='server'. For
these, were are given a socket path
s/were/we/
which is then created by QEMU and to which
OpenVSwitch connects to and creates an interface. Because of this, we don't
know the name of the interface upfront (when starting QEMU) and have to use
the path to query OpenVSwitch later (using ovs-vsctl). What I intended to use
was:
ovs-vsctl --no-headings --columns=name find Interface
options:vhost-server-path=$path
But what my code does is:
ovs-vsctl --no-headings --columns=name find Interface
options:vhost-server-path=path
and it's all because the argument to the function is named "path"
which I then enclosed in double quotes while it should have been
used as a variable.
Nice! :-)
Reviewed-by: Laine Stump
Fixes: e4c29e2904197472919d050c67acfd59f0144bbc
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1767013
Signed-off-by: Michal Privoznik
---
src/util/virnetdevopenvswitch.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c
index d380b0cf22..7eabaa763d 100644
--- a/src/util/virnetdevopenvswitch.c
+++ b/src/util/virnetdevopenvswitch.c
@@ -494,7 +494,7 @@ virNetDevOpenvswitchGetVhostuserIfname(const char *path,
if (server) {
virCommandAddArgList(cmd, "--no-headings", "--columns=name", "find",
"Interface", NULL);
-virCommandAddArgPair(cmd, "options:vhost-server-path", "path");
+virCommandAddArgPair(cmd, "options:vhost-server-path", path);
} else {
const char *tmpIfname = NULL;
[PATCH 2/2] virnetdevopenvswitch: Try to unescape ovs-vsctl reply in one specific case
During testing of my patch v6.10.0-rc1~221 it was found that
'ovs-vsctl get Interface $name name' or
'ovs-vsctl find Interface options:vhost-server-path=$path'
may return a string in double quotes, e.g. "vhost-user1". Later
investigation of openvswitch code showed, that early versions
(like 1.3.0) have somewhat restrictive set of safe characters
(isalpha() || '_' || '-' || '.'), which is then refined with
increasing version. For instance, version 2.11.4 has: isalnum()
|| '_' || '-' || '.'. If the string that ovs-vsctl wants to
output contains any other character it is escaped. You want to be
looking at ovsdb_atom_to_string() which handles outputting of a
single string and calls string_needs_quotes() and possibly
json_serialize_string() in openvswitch code base.
Since the interfaces are usually named "vhost-userN" we are
facing a problem where with one version we get the name in double
quotes and with another we get plain name without funny business.
Because of json involved I thought, let's make ovs-vsctl output
into JSON format and then use our JSON parser, but guess what -
ovs-vsctl ignores --format=json. But with a little help of
g_strdup_printf() it can be turned into JSON.
Fixes: e4c29e2904197472919d050c67acfd59f0144bbc
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1767013
Signed-off-by: Michal Privoznik
---
src/libvirt_private.syms | 1 +
src/util/virnetdevopenvswitch.c | 47 +
src/util/virnetdevopenvswitch.h | 4 +++
tests/virnetdevopenvswitchtest.c | 52
4 files changed, 104 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c7c37d9689..583fc5800e 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2676,6 +2676,7 @@ virNetDevOpenvswitchGetVhostuserIfname;
virNetDevOpenvswitchInterfaceGetMaster;
virNetDevOpenvswitchInterfaceParseStats;
virNetDevOpenvswitchInterfaceStats;
+virNetDevOpenvswitchMaybeUnescapeReply;
virNetDevOpenvswitchRemovePort;
virNetDevOpenvswitchSetMigrateData;
virNetDevOpenvswitchSetTimeout;
diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c
index 7eabaa763d..14fa294ae1 100644
--- a/src/util/virnetdevopenvswitch.c
+++ b/src/util/virnetdevopenvswitch.c
@@ -460,6 +460,48 @@ virNetDevOpenvswitchInterfaceGetMaster(const char *ifname,
char **master)
}
+/**
+ * virNetDevOpenvswitchMaybeUnescapeReply:
+ * @reply: a string to unescape
+ *
+ * Depending on ovs-vsctl version a string might be escaped. For instance:
+ * -version 2.11.4 allows only is_alpha(), an underscore, a dash or a dot,
+ * -version 2.14.0 allows only is_alnum(), an underscore, a dash or a dot,
+ * any other character causes the string to be escaped.
+ *
+ * What this function does, is it checks whether @reply string consists solely
+ * from safe, not escaped characters (as defined by version 2.14.0) and if not
+ * an error is reported. If @reply is a string enclosed in double quotes, but
+ * otherwise safe those double quotes are removed.
+ *
+ * Returns: 0 on success,
+ * -1 otherwise (with error reported).
+ */
+int
+virNetDevOpenvswitchMaybeUnescapeReply(char *reply)
+{
+g_autoptr(virJSONValue) json = NULL;
+g_autofree char *jsonStr = NULL;
+const char *tmp = NULL;
+size_t replyLen = strlen(reply);
+
+if (*reply != '"')
+return 0;
+
+jsonStr = g_strdup_printf("{\"name\": %s}", reply);
+if (!(json = virJSONValueFromString(jsonStr)))
+return -1;
+
+if (!(tmp = virJSONValueObjectGetString(json, "name"))) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Malformed ovs-vsctl output"));
+return -1;
+}
+
+return virStrcpy(reply, tmp, replyLen);
+}
+
+
/**
* virNetDevOpenvswitchGetVhostuserIfname:
* @path: the path of the unix socket
@@ -522,6 +564,11 @@ virNetDevOpenvswitchGetVhostuserIfname(const char *path,
return 0;
}
+if (virNetDevOpenvswitchMaybeUnescapeReply(*ifname) < 0) {
+VIR_FREE(*ifname);
+return -1;
+}
+
return 1;
}
diff --git a/src/util/virnetdevopenvswitch.h b/src/util/virnetdevopenvswitch.h
index 8409aa92ac..3571708582 100644
--- a/src/util/virnetdevopenvswitch.h
+++ b/src/util/virnetdevopenvswitch.h
@@ -60,6 +60,10 @@ int virNetDevOpenvswitchInterfaceStats(const char *ifname,
int virNetDevOpenvswitchInterfaceGetMaster(const char *ifname, char **master)
ATTRIBUTE_NONNULL(1) G_GNUC_WARN_UNUSED_RESULT;
+int
+virNetDevOpenvswitchMaybeUnescapeReply(char *reply)
+ATTRIBUTE_NONNULL(1) G_GNUC_WARN_UNUSED_RESULT;
+
int virNetDevOpenvswitchGetVhostuserIfname(const char *path,
bool server,
char **ifname)
diff --git a/tests/virnetdevopenvswitchtest.c b/tests/virnetdevopenvswitchtest.c
index fd47e927ea..46172dae90 100644
--- a/tests/virnetdevopenvswitchtest.c
+++
[PATCH 1/2] virNetDevOpenvswitchGetVhostuserIfname: Actually use @path to lookup interface
In v6.10.0-rc1~221 I wanted to make virNetDevOpenvswitchGetVhostuserIfname()
lookup interface name even for vhostuser interfaces with mode='server'. For
these, were are given a socket path which is then created by QEMU and to which
OpenVSwitch connects to and creates an interface. Because of this, we don't
know the name of the interface upfront (when starting QEMU) and have to use
the path to query OpenVSwitch later (using ovs-vsctl). What I intended to use
was:
ovs-vsctl --no-headings --columns=name find Interface
options:vhost-server-path=$path
But what my code does is:
ovs-vsctl --no-headings --columns=name find Interface
options:vhost-server-path=path
and it's all because the argument to the function is named "path"
which I then enclosed in double quotes while it should have been
used as a variable.
Fixes: e4c29e2904197472919d050c67acfd59f0144bbc
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1767013
Signed-off-by: Michal Privoznik
---
src/util/virnetdevopenvswitch.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c
index d380b0cf22..7eabaa763d 100644
--- a/src/util/virnetdevopenvswitch.c
+++ b/src/util/virnetdevopenvswitch.c
@@ -494,7 +494,7 @@ virNetDevOpenvswitchGetVhostuserIfname(const char *path,
if (server) {
virCommandAddArgList(cmd, "--no-headings", "--columns=name", "find",
"Interface", NULL);
-virCommandAddArgPair(cmd, "options:vhost-server-path", "path");
+virCommandAddArgPair(cmd, "options:vhost-server-path", path);
} else {
const char *tmpIfname = NULL;
--
2.26.2
[PATCH 0/2] virnetdevopenvswitch: Fir virNetDevOpenvswitchGetVhostuserIfname()
The first patch is a true fix of a mess I made earlier. The second is an improvement that was found by QE when testing patches with openvswitch older than I wrote patches with. Michal Prívozník (2): virNetDevOpenvswitchGetVhostuserIfname: Actually use @path to lookup interface virnetdevopenvswitch: Try to unescape ovs-vsctl reply in one specific case src/libvirt_private.syms | 1 + src/util/virnetdevopenvswitch.c | 49 +- src/util/virnetdevopenvswitch.h | 4 +++ tests/virnetdevopenvswitchtest.c | 52 4 files changed, 105 insertions(+), 1 deletion(-) -- 2.26.2
Re: [PATCH libvirt v1] tests: add capabilities for QEMU 5.1.0 on s390x
On 12/14/20 10:10 AM, Andrea Bolognani wrote: On Wed, 2020-11-18 at 17:18 +0100, Shalini Chellathurai Saroja wrote: Signed-off-by: Shalini Chellathurai Saroja --- The replies file is removed from this patch and is available in https://gitlab.com/shalinichellathurai/libvirt/-/commit/1c34c07c434560d7f44212ce0bbbc8bf92490622 tests/domaincapsdata/qemu_5.1.0.s390x.xml | 230 + .../caps_5.1.0.s390x.replies | 24617 .../qemucapabilitiesdata/caps_5.1.0.s390x.xml | 3278 ++ ...default-video-type-s390x.s390x-latest.args | 2 +- .../disk-error-policy-s390x.s390x-latest.args |12 +- .../fs9p-ccw.s390x-latest.args| 4 +- ...othreads-virtio-scsi-ccw.s390x-latest.args | 2 +- ...t-cpu-kvm-ccw-virtio-4.2.s390x-latest.args | 2 +- .../s390x-ccw-graphics.s390x-latest.args | 4 +- .../s390x-ccw-headless.s390x-latest.args | 4 +- .../vhost-vsock-ccw-auto.s390x-latest.args| 4 +- .../vhost-vsock-ccw.s390x-latest.args | 4 +- 12 files changed, 28144 insertions(+), 19 deletions(-) create mode 100644 tests/domaincapsdata/qemu_5.1.0.s390x.xml create mode 100644 tests/qemucapabilitiesdata/caps_5.1.0.s390x.replies create mode 100644 tests/qemucapabilitiesdata/caps_5.1.0.s390x.xml The patch looks good from a quick look; however, QEMU 5.2.0 was released a week ago and we have already merged capabilities for all architectures but s390x for that version so, unless there's something that is 5.1.0-specific that you need to test, can you please generate the capabilities for 5.2.0 instead? Thanks in advance! Hello Andrea, I have sent a patch with capabilities for QEMU 5.2.0 on s390x. (https://www.redhat.com/archives/libvir-list/2020-December/msg00754.html) Thank you. -- Kind regards Shalini Chellathurai Saroja Linux on Z and Virtualization Development Vorsitzende des Aufsichtsrats: Gregor Pillen Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
Re: [libvirt PATCH 0/3] Few fixes for migration over UNIX socket
On 12/16/20 12:19 PM, Martin Kletzander wrote: Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1638889 Martin Kletzander (3): qemu: Fix possible segfault when migrating disks docs: Slightly alter disks-uri description in virsh man qemu: Extra check for NBD URI being specified docs/manpages/virsh.rst | 22 +++--- src/qemu/qemu_driver.c| 21 + src/qemu/qemu_migration.c | 5 + 3 files changed, 37 insertions(+), 11 deletions(-) Reviewed-by: Michal Privoznik Michal
[libvirt PATCH 2/3] docs: Slightly alter disks-uri description in virsh man
It's more accurate this way.
https://bugzilla.redhat.com/show_bug.cgi?id=1638889
Signed-off-by: Martin Kletzander
---
docs/manpages/virsh.rst | 22 +++---
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index aa3a0095fe3d..4a1500e68628 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -3346,17 +3346,17 @@ bind to for incoming disks traffic. Currently it is
supported only by QEMU.
Optional *disks-uri* can also be specified (mutually exclusive with
*disks-port*) to specify what the remote hypervisor should bind/connect to when
migrating disks. This can be *tcp://address:port* to specify a listen address
-(which overrides *--listen-address* for the disk migration) and a port or
-*unix:///path/to/socket* in case you need the disk migration to happen over a
-UNIX socket with that specified path. In this case you need to make sure the
-same socket path is accessible to both source and destination hypervisors and
-connecting to the socket on the source (after hypervisor creates it on the
-destination) will actually connect to the destination. If you are using SELinux
-(at least on the source host) you need to make sure the socket on the source is
-accessible to libvirtd/QEMU for connection. Libvirt cannot change the context
-of the existing socket because it is different from the file representation of
-the socket and the context is chosen by its creator (usually by using
-*setsockcreatecon{,_raw}()* functions).
+(which overrides *--migrate-uri* and *--listen-address* for the disk migration)
+and a port or *unix:///path/to/socket* in case you need the disk migration to
+happen over a UNIX socket with that specified path. In this case you need to
+make sure the same socket path is accessible to both source and destination
+hypervisors and connecting to the socket on the source (after hypervisor
creates
+it on the destination) will actually connect to the destination. If you are
+using SELinux (at least on the source host) you need to make sure the socket on
+the source is accessible to libvirtd/QEMU for connection. Libvirt cannot
change
+the context of the existing socket because it is different from the file
+representation of the socket and the context is chosen by its creator (usually
+by using *setsockcreatecon{,_raw}()* functions).
migrate-compcache
--
2.29.2
[libvirt PATCH 3/3] qemu: Extra check for NBD URI being specified
It must be used when migration URI uses `unix:` transport because otherwise we
cannot just guess where to connect for disk migration.
https://bugzilla.redhat.com/show_bug.cgi?id=1638889
Signed-off-by: Martin Kletzander
---
src/qemu/qemu_driver.c | 21 +
1 file changed, 21 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5f0fb0a55fee..9caaa0723720 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11503,6 +11503,16 @@ qemuDomainMigratePrepare3Params(virConnectPtr dconn,
QEMU_MIGRATION_DESTINATION)))
return -1;
+if (flags & (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC) ||
+nmigrate_disks > 0) {
+if (uri_in && STRPREFIX(uri_in, "unix:") && !nbdURI) {
+virReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("NBD URI must be supplied when "
+ "migration URI uses UNIX transport method"));
+return -1;
+}
+}
+
if (nbdURI && nbdPort) {
virReportError(VIR_ERR_INVALID_ARG, "%s",
_("Both port and URI requested for disk migration "
@@ -11743,6 +11753,7 @@ qemuDomainMigratePerform3Params(virDomainPtr dom,
_xml) < 0)
goto cleanup;
+
if (nbdURI && nbdPort) {
virReportError(VIR_ERR_INVALID_ARG, "%s",
_("Both port and URI requested for disk migration "
@@ -11766,6 +11777,16 @@ qemuDomainMigratePerform3Params(virDomainPtr dom,
if (nmigrate_disks < 0)
goto cleanup;
+if (flags & (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC) ||
+nmigrate_disks > 0) {
+if (uri && STRPREFIX(uri, "unix:") && !nbdURI) {
+virReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("NBD URI must be supplied when "
+ "migration URI uses UNIX transport method"));
+return -1;
+}
+}
+
if (!(migParams = qemuMigrationParamsFromFlags(params, nparams, flags,
QEMU_MIGRATION_SOURCE)))
goto cleanup;
--
2.29.2
[libvirt PATCH 1/3] qemu: Fix possible segfault when migrating disks
Users can provide URI without a schema.
https://bugzilla.redhat.com/show_bug.cgi?id=1638889
Signed-off-by: Martin Kletzander
---
src/qemu/qemu_migration.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index fcb33d0364a1..90b0ec95e35a 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -408,6 +408,11 @@ qemuMigrationDstStartNBDServer(virQEMUDriverPtr driver,
if (!uri)
return -1;
+if (!uri->scheme) {
+virReportError(VIR_ERR_INVALID_ARG, _("No URI scheme specified:
%s"), nbdURI);
+return -1;
+}
+
if (STREQ(uri->scheme, "tcp")) {
server.transport = VIR_STORAGE_NET_HOST_TRANS_TCP;
if (!uri->server || STREQ(uri->server, "")) {
--
2.29.2
[libvirt PATCH 0/3] Few fixes for migration over UNIX socket
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1638889 Martin Kletzander (3): qemu: Fix possible segfault when migrating disks docs: Slightly alter disks-uri description in virsh man qemu: Extra check for NBD URI being specified docs/manpages/virsh.rst | 22 +++--- src/qemu/qemu_driver.c| 21 + src/qemu/qemu_migration.c | 5 + 3 files changed, 37 insertions(+), 11 deletions(-) -- 2.29.2
