"http://libvirt.org/schemas/domain/qemu/1.0" returns 404 error
Hello everyone, The XML namespace request website can not be accessed with 404 error: http://libvirt.org/schemas/domain/qemu/1.0 Could anyone help to fix it please? Thanks, Yan Fu
[PATCH v2.1 1/3] interface: introduce downscript element for interface
https://gitlab.com/libvirt/libvirt/-/issues/13
Add support for downscript:
Signed-off-by: Chen Hanxiao
---
docs/formatdomain.html.in | 6 +-
docs/schemas/domaincommon.rng | 8
src/conf/domain_conf.c| 9 +
src/conf/domain_conf.h| 1 +
4 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 23eb029234..4e2320d537 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -5879,7 +5879,10 @@
After creating/opening the tap device, an optional shell script
(given in the path attribute of
- the script element) will be run; this can
+ the script element) will be run;
+ Also, after detaching/closing the tap device, an optional shell
+ script (given in the path attribute of
+ the downscript element) will be run; this can
be used to do whatever extra host network integration is
required.
@@ -5889,6 +5892,7 @@
devices
interface type='ethernet'
script path='/etc/qemu-ifup-mynet'/
+downscript path='/etc/qemu-ifdown-mynet'/
/interface
...
interface type='ethernet'
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 9d60b090f3..6727cd743b 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3191,6 +3191,14 @@
+
+
+
+
+
+
+
+
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c201fc901d..1406cf079e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2520,6 +2520,7 @@ virDomainNetDefClear(virDomainNetDefPtr def)
VIR_FREE(def->teaming.persistent);
VIR_FREE(def->virtPortProfile);
VIR_FREE(def->script);
+VIR_FREE(def->downscript);
VIR_FREE(def->domain_name);
VIR_FREE(def->ifname);
VIR_FREE(def->ifname_guest);
@@ -11977,6 +11978,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
g_autofree char *ifname_guest = NULL;
g_autofree char *ifname_guest_actual = NULL;
g_autofree char *script = NULL;
+g_autofree char *downscript = NULL;
g_autofree char *address = NULL;
g_autofree char *port = NULL;
g_autofree char *localaddr = NULL;
@@ -12149,6 +12151,9 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
} else if (!script &&
virXMLNodeNameEqual(cur, "script")) {
script = virXMLPropString(cur, "path");
+} else if (!downscript &&
+ virXMLNodeNameEqual(cur, "downscript")) {
+downscript = virXMLPropString(cur, "path");
} else if (!domain_name &&
virXMLNodeNameEqual(cur, "backenddomain")) {
domain_name = virXMLPropString(cur, "name");
@@ -12482,6 +12487,8 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
if (script != NULL)
def->script = g_steal_pointer();
+if (downscript != NULL)
+def->downscript = g_steal_pointer();
if (domain_name != NULL)
def->domain_name = g_steal_pointer(_name);
if (ifname != NULL)
@@ -26567,6 +26574,8 @@ virDomainNetDefFormat(virBufferPtr buf,
virBufferEscapeString(buf, "\n",
def->script);
+virBufferEscapeString(buf, "\n",
+ def->downscript);
virBufferEscapeString(buf, "\n",
def->domain_name);
if (def->ifname &&
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index ddc75d8de2..e152c599ca 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1055,6 +1055,7 @@ struct _virDomainNetDef {
unsigned long sndbuf;
} tune;
char *script;
+char *downscript;
char *domain_name; /* backend domain name */
char *ifname; /* interface name on the host () */
int managed_tap; /* enum virTristateBool - ABSENT == YES */
--
2.23.0
[PATCH v2.1 3/3] news: add description about downscript
Signed-off-by: Chen Hanxiao --- docs/news.xml | 10 ++ 1 file changed, 10 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index 4cef804aac..67fb85377d 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -52,6 +52,16 @@ + + + qemu: support network interface downscript + + + QEMU has the ability to run a script when a NIC is brought up + and down. Libvirt only enables use of the up script. + Now add support for postscript when NIC is down/detached. + + qemu: support disabling hotplug/unplug of PCIe devices -- 2.23.0
[PATCH v2.1 2/3] downscript: add support for booting and hotplug interface
Support downscript for booting vm,
and hotunplug interface device.
Signed-off-by: Chen Hanxiao
---
src/qemu/qemu_extdevice.c | 4 ++
src/qemu/qemu_hotplug.c | 3 ++
tests/qemuxml2argvdata/downscript.xml | 60 +
tests/qemuxml2xmloutdata/downscript.xml | 60 +
tests/qemuxml2xmltest.c | 1 +
5 files changed, 128 insertions(+)
create mode 100644 tests/qemuxml2argvdata/downscript.xml
create mode 100644 tests/qemuxml2xmloutdata/downscript.xml
diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c
index 2096272761..4962521de4 100644
--- a/src/qemu/qemu_extdevice.c
+++ b/src/qemu/qemu_extdevice.c
@@ -213,6 +213,7 @@ qemuExtDevicesStop(virQEMUDriverPtr driver,
virDomainObjPtr vm)
{
virDomainDefPtr def = vm->def;
+virDomainNetType actualType;
size_t i;
if (qemuExtDevicesInitPaths(driver, def) < 0)
@@ -230,10 +231,13 @@ qemuExtDevicesStop(virQEMUDriverPtr driver,
for (i = 0; i < def->nnets; i++) {
virDomainNetDefPtr net = def->nets[i];
+actualType = virDomainNetGetActualType(net);
qemuSlirpPtr slirp = QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp;
if (slirp)
qemuSlirpStop(slirp, vm, driver, net);
+if (actualType == VIR_DOMAIN_NET_TYPE_ETHERNET && net->downscript)
+virNetDevRunEthernetScript(net->ifname, net->downscript);
}
for (i = 0; i < def->nfss; i++) {
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 5608566d69..f94f518f4d 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -4663,6 +4663,9 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver,
virDomainNetReleaseActualDevice(conn, vm->def, net);
else
VIR_WARN("Unable to release network device '%s'",
NULLSTR(net->ifname));
+} else if (net->type == VIR_DOMAIN_NET_TYPE_ETHERNET) {
+if (net->script)
+ virNetDevRunEthernetScript(net->ifname, net->downscript);
}
virDomainNetDefFree(net);
return 0;
diff --git a/tests/qemuxml2argvdata/downscript.xml
b/tests/qemuxml2argvdata/downscript.xml
new file mode 100644
index 00..4d0fb1beab
--- /dev/null
+++ b/tests/qemuxml2argvdata/downscript.xml
@@ -0,0 +1,60 @@
+
+ QEMUGuest1
+ c7a5fdbd-edaf-9455-926a-d65c16db1809
+ 219136
+ 219136
+ 1
+
+hvm
+
+
+
+qemu64
+
+
+ destroy
+ restart
+ destroy
+
+/usr/bin/qemu-system-x86_64
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/tests/qemuxml2xmloutdata/downscript.xml
b/tests/qemuxml2xmloutdata/downscript.xml
new file mode 100644
index 00..4d0fb1beab
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/downscript.xml
@@ -0,0 +1,60 @@
+
+ QEMUGuest1
+ c7a5fdbd-edaf-9455-926a-d65c16db1809
+ 219136
+ 219136
+ 1
+
+hvm
+
+
+
+qemu64
+
+
+ destroy
+ restart
+ destroy
+
+/usr/bin/qemu-system-x86_64
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 033f81013e..dcc7b29ded 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -1478,6 +1478,7 @@ mymain(void)
DO_TEST_CAPS_ARCH_LATEST("x86_64-default-cpu-tcg-q35-4.2", "x86_64");
DO_TEST_CAPS_LATEST("virtio-9p-multidevs");
+DO_TEST("downscript", NONE);
if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL)
virFileDeleteTree(fakerootdir);
--
2.23.0
[PATCH v2.1 0/3] Support network interface downscript
QEMU has the ability to run a script when a NIC is brought up and down. Libvirt only enables use of the up script at this time. This series add support for postscript when NIC is down/detached. Chen Hanxiao (3): interface: introduce downscript downscript: add support for booting and hotplug interface news: add description about downscript docs/formatdomain.html.in | 6 ++- docs/news.xml | 10 + docs/schemas/domaincommon.rng | 8 src/conf/domain_conf.c | 9 src/conf/domain_conf.h | 1 + src/qemu/qemu_extdevice.c | 4 ++ src/qemu/qemu_hotplug.c | 3 ++ tests/qemuxml2argvdata/downscript.xml | 60 + tests/qemuxml2xmloutdata/downscript.xml | 60 + tests/qemuxml2xmltest.c | 1 + 10 files changed, 161 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/downscript.xml create mode 100644 tests/qemuxml2xmloutdata/downscript.xml -- 2.23.0
Re: [PATCH v2 3/3] downscript: docs: add docs and news
On Mon, May 25, 2020 at 12:16:49 -0400, Chen Hanxiao wrote: > Signed-off-by: Chen Hanxiao > --- > docs/formatdomain.html.in | 6 +- > docs/news.xml | 10 ++ > 2 files changed, 15 insertions(+), 1 deletion(-) > > diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in > index 23eb029234..4e2320d537 100644 > --- a/docs/formatdomain.html.in > +++ b/docs/formatdomain.html.in [...] > diff --git a/docs/news.xml b/docs/news.xml > index 4cef804aac..67fb85377d 100644 > --- a/docs/news.xml > +++ b/docs/news.xml Changes to 'news.xml' must be in a separate commit. This is to decrease likelihood of conflicts when backporting patches.
[PATCH v2 0/3] Support network interface downscript
QEMU has the ability to run a script when a NIC is brought up and down. Libvirt only enables use of the up script at this time. This series add support for postscript when NIC is down/detached. Chen Hanxiao (3): interface: introduce downscript downscript: add support for booting and hotplug interface downscript: docs: add docs and news docs/formatdomain.html.in | 6 ++- docs/news.xml | 10 + docs/schemas/domaincommon.rng | 8 src/conf/domain_conf.c | 9 src/conf/domain_conf.h | 1 + src/qemu/qemu_extdevice.c | 4 ++ src/qemu/qemu_hotplug.c | 3 ++ tests/qemuxml2argvdata/downscript.xml | 60 + tests/qemuxml2xmloutdata/downscript.xml | 60 + tests/qemuxml2xmltest.c | 1 + 10 files changed, 161 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/downscript.xml create mode 100644 tests/qemuxml2xmloutdata/downscript.xml -- 2.23.0
[PATCH v2 3/3] downscript: docs: add docs and news
Signed-off-by: Chen Hanxiao --- docs/formatdomain.html.in | 6 +- docs/news.xml | 10 ++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 23eb029234..4e2320d537 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -5879,7 +5879,10 @@ After creating/opening the tap device, an optional shell script (given in the path attribute of - the script element) will be run; this can + the script element) will be run; + Also, after detaching/closing the tap device, an optional shell + script (given in the path attribute of + the downscript element) will be run; this can be used to do whatever extra host network integration is required. @@ -5889,6 +5892,7 @@ devices interface type='ethernet' script path='/etc/qemu-ifup-mynet'/ +downscript path='/etc/qemu-ifdown-mynet'/ /interface ... interface type='ethernet' diff --git a/docs/news.xml b/docs/news.xml index 4cef804aac..67fb85377d 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -52,6 +52,16 @@ + + + qemu: support network interface downscript + + + QEMU has the ability to run a script when a NIC is brought up + and down. Libvirt only enables use of the up script. + Now add support for postscript when NIC is down/detached. + + qemu: support disabling hotplug/unplug of PCIe devices -- 2.23.0
[PATCH v2 2/3] downscript: add support for booting and hotplug interface
Support downscript for booting vm,
and hotunplug interface device.
Signed-off-by: Chen Hanxiao
---
src/qemu/qemu_extdevice.c | 4 ++
src/qemu/qemu_hotplug.c | 3 ++
tests/qemuxml2argvdata/downscript.xml | 60 +
tests/qemuxml2xmloutdata/downscript.xml | 60 +
tests/qemuxml2xmltest.c | 1 +
5 files changed, 128 insertions(+)
create mode 100644 tests/qemuxml2argvdata/downscript.xml
create mode 100644 tests/qemuxml2xmloutdata/downscript.xml
diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c
index 2096272761..4962521de4 100644
--- a/src/qemu/qemu_extdevice.c
+++ b/src/qemu/qemu_extdevice.c
@@ -213,6 +213,7 @@ qemuExtDevicesStop(virQEMUDriverPtr driver,
virDomainObjPtr vm)
{
virDomainDefPtr def = vm->def;
+virDomainNetType actualType;
size_t i;
if (qemuExtDevicesInitPaths(driver, def) < 0)
@@ -230,10 +231,13 @@ qemuExtDevicesStop(virQEMUDriverPtr driver,
for (i = 0; i < def->nnets; i++) {
virDomainNetDefPtr net = def->nets[i];
+actualType = virDomainNetGetActualType(net);
qemuSlirpPtr slirp = QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp;
if (slirp)
qemuSlirpStop(slirp, vm, driver, net);
+if (actualType == VIR_DOMAIN_NET_TYPE_ETHERNET && net->downscript)
+virNetDevRunEthernetScript(net->ifname, net->downscript);
}
for (i = 0; i < def->nfss; i++) {
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 5608566d69..f94f518f4d 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -4663,6 +4663,9 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver,
virDomainNetReleaseActualDevice(conn, vm->def, net);
else
VIR_WARN("Unable to release network device '%s'",
NULLSTR(net->ifname));
+} else if (net->type == VIR_DOMAIN_NET_TYPE_ETHERNET) {
+if (net->script)
+ virNetDevRunEthernetScript(net->ifname, net->downscript);
}
virDomainNetDefFree(net);
return 0;
diff --git a/tests/qemuxml2argvdata/downscript.xml
b/tests/qemuxml2argvdata/downscript.xml
new file mode 100644
index 00..4d0fb1beab
--- /dev/null
+++ b/tests/qemuxml2argvdata/downscript.xml
@@ -0,0 +1,60 @@
+
+ QEMUGuest1
+ c7a5fdbd-edaf-9455-926a-d65c16db1809
+ 219136
+ 219136
+ 1
+
+hvm
+
+
+
+qemu64
+
+
+ destroy
+ restart
+ destroy
+
+/usr/bin/qemu-system-x86_64
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/tests/qemuxml2xmloutdata/downscript.xml
b/tests/qemuxml2xmloutdata/downscript.xml
new file mode 100644
index 00..4d0fb1beab
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/downscript.xml
@@ -0,0 +1,60 @@
+
+ QEMUGuest1
+ c7a5fdbd-edaf-9455-926a-d65c16db1809
+ 219136
+ 219136
+ 1
+
+hvm
+
+
+
+qemu64
+
+
+ destroy
+ restart
+ destroy
+
+/usr/bin/qemu-system-x86_64
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 033f81013e..dcc7b29ded 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -1478,6 +1478,7 @@ mymain(void)
DO_TEST_CAPS_ARCH_LATEST("x86_64-default-cpu-tcg-q35-4.2", "x86_64");
DO_TEST_CAPS_LATEST("virtio-9p-multidevs");
+DO_TEST("downscript", NONE);
if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL)
virFileDeleteTree(fakerootdir);
--
2.23.0
Re: [libvirt PATCH] scripts: Fix E741 that pycodesyle is pointing out during syntax-check
On a Monday in 2020, Michal Privoznik wrote:
On 5/25/20 2:56 PM, Erik Skultety wrote:
With newer pycodestyle 2.6.0 (which is part of flake8-3.8.2) reports
the following pep violation during syntax-check:
../scripts/check-remote-protocol.py:95:9: E741 ambiguous variable name 'l'
for l in err.strip().split("\n")
On all the distros we test on, this hasn't occurred yet, but with the
future update of flake8 it likely would. The fix is easy, just name the
variable appropriately.
Signed-off-by: Erik Skultety
---
The weird thing is that E741 checking has been in pycodestyle since 2.1.0 [1],
we now have 2.5.0 and yet only 2.6.0 is complaining about it
[1] https://pycodestyle.pycqa.org/en/latest/developer.html#id8
scripts/check-remote-protocol.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/scripts/check-remote-protocol.py b/scripts/check-remote-protocol.py
index 25caa19563..cf9e3f84a1 100644
--- a/scripts/check-remote-protocol.py
+++ b/scripts/check-remote-protocol.py
@@ -92,8 +92,8 @@ if out == "" or pdwtagsproc.returncode != 0:
else:
print("WARNING: exit code %d, pdwtags appears broken:" %
pdwtagsproc.returncode, file=sys.stderr)
-for l in err.strip().split("\n"):
-print("WARNING: %s" % l, file=sys.stderr)
+for line in err.strip().split("\n"):
+print("WARNING: %s" % line, file=sys.stderr)
print("WARNING: skipping the remote protocol test", file=sys.stderr)
sys.exit(0)
Ah, the error is about short variable name, it's about 'l' looking too
similar to 'I' (well, if this is somebody's case then I say use
better font if you want to code). But in order to shut the checker up:
Note that we can also shut it up by adding it to our FLAKE8_IGNORE
in build-aux/syntax-check.mk, but I don't care either way.
Jano
Reviewed-by: Michal Privoznik
Michal
signature.asc
Description: PGP signature
[PATCH v2 1/3] interface: introduce downscript
https://gitlab.com/libvirt/libvirt/-/issues/13
Add support for downscript:
Signed-off-by: Chen Hanxiao
---
docs/schemas/domaincommon.rng | 8
src/conf/domain_conf.c| 9 +
src/conf/domain_conf.h| 1 +
3 files changed, 18 insertions(+)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 9d60b090f3..6727cd743b 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3191,6 +3191,14 @@
+
+
+
+
+
+
+
+
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c201fc901d..1406cf079e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2520,6 +2520,7 @@ virDomainNetDefClear(virDomainNetDefPtr def)
VIR_FREE(def->teaming.persistent);
VIR_FREE(def->virtPortProfile);
VIR_FREE(def->script);
+VIR_FREE(def->downscript);
VIR_FREE(def->domain_name);
VIR_FREE(def->ifname);
VIR_FREE(def->ifname_guest);
@@ -11977,6 +11978,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
g_autofree char *ifname_guest = NULL;
g_autofree char *ifname_guest_actual = NULL;
g_autofree char *script = NULL;
+g_autofree char *downscript = NULL;
g_autofree char *address = NULL;
g_autofree char *port = NULL;
g_autofree char *localaddr = NULL;
@@ -12149,6 +12151,9 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
} else if (!script &&
virXMLNodeNameEqual(cur, "script")) {
script = virXMLPropString(cur, "path");
+} else if (!downscript &&
+ virXMLNodeNameEqual(cur, "downscript")) {
+downscript = virXMLPropString(cur, "path");
} else if (!domain_name &&
virXMLNodeNameEqual(cur, "backenddomain")) {
domain_name = virXMLPropString(cur, "name");
@@ -12482,6 +12487,8 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
if (script != NULL)
def->script = g_steal_pointer();
+if (downscript != NULL)
+def->downscript = g_steal_pointer();
if (domain_name != NULL)
def->domain_name = g_steal_pointer(_name);
if (ifname != NULL)
@@ -26567,6 +26574,8 @@ virDomainNetDefFormat(virBufferPtr buf,
virBufferEscapeString(buf, "\n",
def->script);
+virBufferEscapeString(buf, "\n",
+ def->downscript);
virBufferEscapeString(buf, "\n",
def->domain_name);
if (def->ifname &&
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index ddc75d8de2..e152c599ca 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1055,6 +1055,7 @@ struct _virDomainNetDef {
unsigned long sndbuf;
} tune;
char *script;
+char *downscript;
char *domain_name; /* backend domain name */
char *ifname; /* interface name on the host () */
int managed_tap; /* enum virTristateBool - ABSENT == YES */
--
2.23.0
Re: [libvirt PATCH] scripts: Fix E741 that pycodesyle is pointing out during syntax-check
On 5/25/20 2:56 PM, Erik Skultety wrote:
With newer pycodestyle 2.6.0 (which is part of flake8-3.8.2) reports
the following pep violation during syntax-check:
../scripts/check-remote-protocol.py:95:9: E741 ambiguous variable name 'l'
for l in err.strip().split("\n")
On all the distros we test on, this hasn't occurred yet, but with the
future update of flake8 it likely would. The fix is easy, just name the
variable appropriately.
Signed-off-by: Erik Skultety
---
The weird thing is that E741 checking has been in pycodestyle since 2.1.0 [1],
we now have 2.5.0 and yet only 2.6.0 is complaining about it
[1] https://pycodestyle.pycqa.org/en/latest/developer.html#id8
scripts/check-remote-protocol.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/scripts/check-remote-protocol.py b/scripts/check-remote-protocol.py
index 25caa19563..cf9e3f84a1 100644
--- a/scripts/check-remote-protocol.py
+++ b/scripts/check-remote-protocol.py
@@ -92,8 +92,8 @@ if out == "" or pdwtagsproc.returncode != 0:
else:
print("WARNING: exit code %d, pdwtags appears broken:" %
pdwtagsproc.returncode, file=sys.stderr)
-for l in err.strip().split("\n"):
-print("WARNING: %s" % l, file=sys.stderr)
+for line in err.strip().split("\n"):
+print("WARNING: %s" % line, file=sys.stderr)
print("WARNING: skipping the remote protocol test", file=sys.stderr)
sys.exit(0)
Ah, the error is about short variable name, it's about 'l' looking too
similar to 'I' (well, if this is somebody's case then I say use better
font if you want to code). But in order to shut the checker up:
Reviewed-by: Michal Privoznik
Michal
Re: [PATCH v2 1/1] qemuProcessRefreshCPU: skip 'host-model' logic for pSeries guests
On Mon, May 25, 2020 at 09:39:45 -0300, Daniel Henrique Barboza wrote:
> Commit v3.10.0-182-g237f045d9a ("qemu: Ignore fallback CPU attribute
> on reconnect") forced CPU 'fallback' to ALLOW, regardless of user
> choice. This fixed a situation in which guests created with older
> Libvirt versions, which used CPU mode 'host-model' in runtime, would
> fail to launch in a newer Libvirt if the fallback was set to FORBID.
> This would lead to a scenario where the CPU was translated to 'host-model'
> to 'custom', but then the FORBID setting would make the translation
> process fail.
>
> PSeries can operate with 'host-model' in runtime due to specific PPC64
> mechanics regarding compatibility mode. The update() implementation of
> the cpuDriverPPC64 driver is a NO-OP if CPU mode is 'host-model', and
> the driver does not implement translate(). The commit mentioned above
> is causing PSeries guests to get their 'fallback' setting to ALLOW,
> overwriting user choice, exposing a design problem in
> qemuProcessRefreshCPU() - for PSeries guests, handling 'host-model'
> as it is being done does not apply.
>
> All other cpuArchDrivers implements update() and changes guest mode
> to VIR_CPU_MODE_CUSTOM, meaning that PSeries is currently the only
> exception to this logic. Let's make it official.
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1660711
>
> Suggested-by: Jiri Denemark
> Signed-off-by: Daniel Henrique Barboza
> ---
> src/qemu/qemu_process.c | 9 +
> 1 file changed, 9 insertions(+)
Reviewed-by: Jiri Denemark
Thanks and pushed.
Re: [PATCH] Don't require secdrivers to implement .domainMoveImageMetadata
On 5/25/20 1:14 PM, Christian Ehrhardt wrote:
On Mon, May 18, 2020 at 10:07 AM Michal Privoznik wrote:
On 5/18/20 10:02 AM, Erik Skultety wrote:
Yes, I know, what I meant by "unrelated" here was just the fact that in order
to fix Apparmor, you only need the first hunk, I guess I'll be more explicit
next time :). It's true that with the first hunk the second becomes redundant,
but I still feel like the NOP driver should cover the full spectrum of
operations we support, but maybe I'm trying to be overly cautious here.
Well, it doesn't implement everything already. But okay, I have ACK for
the important hunk so I will push only that one.
Hi Michal,
while debugging a Ubuntu bug report I found that this fix will
mitigate the warning you wanted to fix.
But overall there still is an issue with the labeling introduced by
[1][2] in >=5.6.
The situation (related to this fix, hence replying in this context) is
the following.
Ubuntu (as an example) builds and has build with --without-attr.
That will make the code have !HAVE_LIBATTR which was fine in the past.
But with your code added the LP bug [3] identified an issue.
What happens is that in stacked security it will iterate on:
- virSecurityStackMoveImageMetadata (for the stacking itself)
- 0x0 (apparmor)
- virSecurityDACMoveImageMetadata
The fix discussed here fixes the warning emitted by the apparmor case like:
"this function is not supported by the connection driver"
But when iterating further on a build that has no attr support we
encounter the following (e.g. at guest shutdown):
libvirtd[6320]: internal error: child reported (status=125):
libvirtd[6320]: Unable to remove disk metadata on vm testguest from
/var/lib/uvtool/libvirt/images/testguest.qcow (disk target vda)
I found that this is due to:
qemuBlockRemoveImageMetadata (the one that emits the warning)
-> qemuSecurityMoveImageMetadata
-> virSecurityManagerMoveImageMetadata
-> virSecurityDACMoveImageMetadata
-> virSecurityDACMoveImageMetadataHelper
-> virProcessRunInFork (spawns subprocess)
-> virSecurityMoveRememberedLabel
Since this is built without HAVE_LIBATTR the following will happen
461 if (virFileGetXAttrQuiet(src, ref_name, _value) < 0) {
(gdb) n
462 if (errno == ENOSYS || errno == ENOTSUP) {
(gdb) p errno
$32 = 38
And that is due to !HAVE_LIBATTR which maps the implementation onto:
4412 #else /* !HAVE_LIBATTR */
4413
4414 int
4415 virFileGetXAttrQuiet(const char *path G_GNUC_UNUSED,
4416 const char *name G_GNUC_UNUSED,
4417 char **value G_GNUC_UNUSED)
4418 {
4419 errno = ENOSYS;
4420 return -1;
4421 }
Due to that we see the two messages reported above
a) internal errors -> for the subprocess that failed
b) "Unable to remove disk metadata" -> but this time due to DAC
instead of apparmor in the security stack
Thank you for your deep analysis.
I'm not sure what you'd prefer Michal, maybe an early RC=0 exit in
virSecurityMoveRememberedLabel in case of !HAVE_LIBATTR?
Con: That would still fork the process to do nothing then
Pro: It would but be a small change in just one place
Since you did all the related changes I thought I report the case and
leave it up to you Michal, what do you think?
Yes, a naive fix would be something like this:
diff --git i/src/security/security_dac.c w/src/security/security_dac.c
index bdc2d7edf3..7b95a6f86d 100644
--- i/src/security/security_dac.c
+++ w/src/security/security_dac.c
@@ -1117,6 +1117,12 @@ virSecurityDACMoveImageMetadataHelper(pid_t pid
G_GNUC_UNUSED,
ret = virSecurityMoveRememberedLabel(SECURITY_DAC_NAME, data->src,
data->dst);
virSecurityManagerMetadataUnlock(data->mgr, );
+
+if (ret == -2) {
+/* Libvirt built without XATTRS */
+ret = 0;
+}
+
return ret;
}
But as you correctly say, it will still lead to an unnecessary spawn of
a process that will do NOP (basically). I think a better fix would be to
not require .domainMoveImageMetadata callbacks (i.e. the one from NOP
driver could be then removed) and set the DAC/SELinux callbacks if and
only if HAVE_LIBATTR; alternatively, make those functions be NOP if
!HAVE_LIBATTR.
On the other hand, every time we relabel a path outside of /dev, we
technically spawn unnecessary because only /dev lives inside a private
NS. Everything else is from the parent NS. For instance, there is no
need to spawn only to chown("/var/lib/libvirt/images/fedora.qcow2").
Therefore I might have a slight preference for the naive fix, but I will
leave it up to you. Or do you want me to sent the patch?
Michal
Re: [libvirt PATCH v2 5/6] hostcpu: Implement virHostCPUGetSignature for s390
Thanks Jiri!
Reviewed-by: Boris Fiuczynski
On 5/22/20 11:15 AM, Jiri Denemark wrote:
Signed-off-by: Jiri Denemark
Reviewed-by: Ján Tomko
---
src/util/virhostcpu.c | 21 ++-
.../linux-s390x-with-frequency.signature | 1 +
2 files changed, 21 insertions(+), 1 deletion(-)
create mode 100644 tests/virhostcpudata/linux-s390x-with-frequency.signature
diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c
index a25a4fa333..39bbcf8ca8 100644
--- a/src/util/virhostcpu.c
+++ b/src/util/virhostcpu.c
@@ -1430,8 +1430,10 @@ virHostCPUReadSignature(virArch arch,
g_autofree char *model = NULL;
g_autofree char *stepping = NULL;
g_autofree char *revision = NULL;
+g_autofree char *proc = NULL;
+g_autofree char *facilities = NULL;
-if (!ARCH_IS_X86(arch) && !ARCH_IS_PPC64(arch))
+if (!ARCH_IS_X86(arch) && !ARCH_IS_PPC64(arch) && !ARCH_IS_S390(arch))
return 0;
while (fgets(line, lineLen, cpuinfo)) {
@@ -1479,6 +1481,23 @@ virHostCPUReadSignature(virArch arch,
*signature = g_strdup_printf("%s, rev %s", name, revision);
return 0;
}
+} else if (ARCH_IS_S390(arch)) {
+if (STREQ(parts[0], "vendor_id")) {
+if (!vendor)
+vendor = g_steal_pointer([1]);
+} else if (STREQ(parts[0], "processor 0")) {
+if (!proc)
+proc = g_steal_pointer([1]);
+} else if (STREQ(parts[0], "facilities")) {
+if (!facilities)
+facilities = g_steal_pointer([1]);
+}
+
+if (vendor && proc && facilities) {
+*signature = g_strdup_printf("%s, %s, facilities: %s",
+ vendor, proc, facilities);
+return 0;
+}
}
}
diff --git a/tests/virhostcpudata/linux-s390x-with-frequency.signature b/tests/virhostcpudata/linux-s390x-with-frequency.signature
new file mode 100644
index 00..70bb28a154
--- /dev/null
+++ b/tests/virhostcpudata/linux-s390x-with-frequency.signature
@@ -0,0 +1 @@
+IBM/S390, version = 00, identification = 145F07, machine = 2964, facilities:
0 1 2 3 4 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 30
31 32 33 34 35 36 37 40 41 42 43 44 45 46 47 48 49 50 51 52 53 55 57 64 65 66
67 68 69 70 71 72 73 75 76 77 78 80 128 129 131 132 142 143
\ No newline at end of file
--
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
[libvirt PATCH] scripts: Fix E741 that pycodesyle is pointing out during syntax-check
With newer pycodestyle 2.6.0 (which is part of flake8-3.8.2) reports
the following pep violation during syntax-check:
../scripts/check-remote-protocol.py:95:9: E741 ambiguous variable name 'l'
for l in err.strip().split("\n")
On all the distros we test on, this hasn't occurred yet, but with the
future update of flake8 it likely would. The fix is easy, just name the
variable appropriately.
Signed-off-by: Erik Skultety
---
The weird thing is that E741 checking has been in pycodestyle since 2.1.0 [1],
we now have 2.5.0 and yet only 2.6.0 is complaining about it
[1] https://pycodestyle.pycqa.org/en/latest/developer.html#id8
scripts/check-remote-protocol.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/scripts/check-remote-protocol.py b/scripts/check-remote-protocol.py
index 25caa19563..cf9e3f84a1 100644
--- a/scripts/check-remote-protocol.py
+++ b/scripts/check-remote-protocol.py
@@ -92,8 +92,8 @@ if out == "" or pdwtagsproc.returncode != 0:
else:
print("WARNING: exit code %d, pdwtags appears broken:" %
pdwtagsproc.returncode, file=sys.stderr)
-for l in err.strip().split("\n"):
-print("WARNING: %s" % l, file=sys.stderr)
+for line in err.strip().split("\n"):
+print("WARNING: %s" % line, file=sys.stderr)
print("WARNING: skipping the remote protocol test", file=sys.stderr)
sys.exit(0)
--
2.25.4
[PATCH v2 1/1] qemuProcessRefreshCPU: skip 'host-model' logic for pSeries guests
Commit v3.10.0-182-g237f045d9a ("qemu: Ignore fallback CPU attribute
on reconnect") forced CPU 'fallback' to ALLOW, regardless of user
choice. This fixed a situation in which guests created with older
Libvirt versions, which used CPU mode 'host-model' in runtime, would
fail to launch in a newer Libvirt if the fallback was set to FORBID.
This would lead to a scenario where the CPU was translated to 'host-model'
to 'custom', but then the FORBID setting would make the translation
process fail.
PSeries can operate with 'host-model' in runtime due to specific PPC64
mechanics regarding compatibility mode. The update() implementation of
the cpuDriverPPC64 driver is a NO-OP if CPU mode is 'host-model', and
the driver does not implement translate(). The commit mentioned above
is causing PSeries guests to get their 'fallback' setting to ALLOW,
overwriting user choice, exposing a design problem in
qemuProcessRefreshCPU() - for PSeries guests, handling 'host-model'
as it is being done does not apply.
All other cpuArchDrivers implements update() and changes guest mode
to VIR_CPU_MODE_CUSTOM, meaning that PSeries is currently the only
exception to this logic. Let's make it official.
https://bugzilla.redhat.com/show_bug.cgi?id=1660711
Suggested-by: Jiri Denemark
Signed-off-by: Daniel Henrique Barboza
---
src/qemu/qemu_process.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index a1ef1d42b0..999e576e5b 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -7775,6 +7775,15 @@ qemuProcessRefreshCPU(virQEMUDriverPtr driver,
* running domain.
*/
if (vm->def->cpu->mode == VIR_CPU_MODE_HOST_MODEL) {
+/*
+ * PSeries domains are able to run with host-model CPU by design,
+ * even on Libvirt newer than 2.3, never replacing host-model with
+ * custom in the virCPUUpdate() call. It is not needed to call
+ * virCPUUpdate() and qemuProcessUpdateCPU() in this case.
+ */
+if (qemuDomainIsPSeries(vm->def))
+return 0;
+
if (!(hostmig = virCPUCopyMigratable(host->arch, host)))
return -1;
--
2.26.2
[PATCH v2 0/1] qemuProcessRefreshCPU: skip 'host-model' code
changes in v2: - resending only patch 5/5 - changed the 'if' placement to qemuProcessRefreshCPU as suggested by Jiri Denemark - changed the commit msg to reflect the new intention of the patch link to v1: https://www.redhat.com/archives/libvir-list/2020-May/msg01049.html Daniel Henrique Barboza (1): qemuProcessRefreshCPU: skip 'host-model' logic for pSeries guests src/qemu/qemu_process.c | 9 + 1 file changed, 9 insertions(+) -- 2.26.2
Re: [PATCH 5/5] qemuProcessUpdateCPU: do not change 'fallback' to ALLOW for pSeries guests
On 5/25/20 7:38 AM, Jiri Denemark wrote:
On Fri, May 22, 2020 at 16:56:20 -0300, Daniel Henrique Barboza wrote:
Commit v3.10.0-182-g237f045d9a ("qemu: Ignore fallback CPU attribute
on reconnect") forced CPU 'fallback' to ALLOW, regardless of user
choice. This fixed a situation in which guests created with older
Libvirt versions, which used CPU mode 'host-model' in runtime, would
fail to launch in a newer Libvirt if the fallback was set to FORBID.
This would lead to a scenario where the CPU was translated to 'host-model'
to 'custom', but then the FORBID setting would make the translation
process fail.
This fix has a side effect for PSeries guests. PSeries can operate
with 'host-model' in runtime due to specific PPC64 mechanics regarding
compatibility mode. In fact, the update() implementation of the
cpuDriverPPC64 driver is a NO-OP if CPU mode is 'host-model', and
the driver does not implement translate(). The result is that PSeries
guests aren't affected by the problem, but they are being affected by
the fix - users are seeing 'fallback' mode being changed without
necessity during daemon restart.
All other cpuArchDrivers implements update() and changes guest mode
to VIR_CPU_MODE_CUSTOM, meaning that PSeries is currently the only
exception to this logic. Let's make it official.
https://bugzilla.redhat.com/show_bug.cgi?id=1660711
CC: Jiri Denemark
Signed-off-by: Daniel Henrique Barboza
---
src/qemu/qemu_process.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index a1ef1d42b0..fec1720f33 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4347,8 +4347,14 @@ qemuProcessUpdateCPU(virQEMUDriverPtr driver,
/* The host CPU model comes from host caps rather than QEMU caps so
* fallback must be allowed no matter what the user specified in the XML.
+ *
+ * Note: PSeries domains are able to run with host-model CPU by design,
+ * even on Libvirt newer than 2.3, never replacing host-model with
+ * custom in the virCPUUpdate() call prior to this function. It is not
+ * needed to change the user defined 'fallback' attribute in this case.
*/
-vm->def->cpu->fallback = VIR_CPU_FALLBACK_ALLOW;
+if (!qemuDomainIsPSeries(vm->def))
+vm->def->cpu->fallback = VIR_CPU_FALLBACK_ALLOW;
if (qemuProcessFetchGuestCPU(driver, vm, asyncJob, , ) < 0)
return -1;
qemuProcessUpdateCPU should not be called at all in this case. The
caller (qemuProcessRefreshCPU) is supposed to decide whether the guest
CPU needs to be changed:
/* If the domain with a host-model CPU was started by an old libvirt
* (< 2.3) which didn't replace the CPU with a custom one, let's do it now
* since the rest of our code does not really expect a host-model CPU in a
* running domain.
*/
if (vm->def->cpu->mode == VIR_CPU_MODE_HOST_MODEL) {
if (!(hostmig = virCPUCopyMigratable(host->arch, host)))
return -1;
if (!(cpu = virCPUDefCopyWithoutModel(hostmig)) ||
virCPUDefCopyModelFilter(cpu, hostmig, false,
virQEMUCapsCPUFilterFeatures,
>arch) < 0)
return -1;
if (virCPUUpdate(vm->def->os.arch, vm->def->cpu, cpu) < 0)
return -1;
if (qemuProcessUpdateCPU(driver, vm, QEMU_ASYNC_JOB_NONE) < 0)
return -1;
So better solution would be to move your new comment and check just
after the check for VIR_CPU_MODE_HOST_MODEL:
if (vm->def->cpu->mode == VIR_CPU_MODE_HOST_MODEL) {
/* PSeries... */
if (qemuDomainIsPSeries(vm->def))
return 0;
Got it. I'll also update the commit msg to mention that the problem resides in
doing the proper handling in qemuProcessRefreshCPU(), and that your patch in
qemuProcessUpdateCPU() I mentioned there simply exposed the problem.
Thanks,
DHB
...
Jirka
Re: [PATCH] Don't require secdrivers to implement .domainMoveImageMetadata
On Mon, May 18, 2020 at 10:07 AM Michal Privoznik wrote:
>
> On 5/18/20 10:02 AM, Erik Skultety wrote:
>
> > Yes, I know, what I meant by "unrelated" here was just the fact that in
> > order
> > to fix Apparmor, you only need the first hunk, I guess I'll be more explicit
> > next time :). It's true that with the first hunk the second becomes
> > redundant,
> > but I still feel like the NOP driver should cover the full spectrum of
> > operations we support, but maybe I'm trying to be overly cautious here.
> >
>
> Well, it doesn't implement everything already. But okay, I have ACK for
> the important hunk so I will push only that one.
Hi Michal,
while debugging a Ubuntu bug report I found that this fix will
mitigate the warning you wanted to fix.
But overall there still is an issue with the labeling introduced by
[1][2] in >=5.6.
The situation (related to this fix, hence replying in this context) is
the following.
Ubuntu (as an example) builds and has build with --without-attr.
That will make the code have !HAVE_LIBATTR which was fine in the past.
But with your code added the LP bug [3] identified an issue.
What happens is that in stacked security it will iterate on:
- virSecurityStackMoveImageMetadata (for the stacking itself)
- 0x0 (apparmor)
- virSecurityDACMoveImageMetadata
The fix discussed here fixes the warning emitted by the apparmor case like:
"this function is not supported by the connection driver"
But when iterating further on a build that has no attr support we
encounter the following (e.g. at guest shutdown):
libvirtd[6320]: internal error: child reported (status=125):
libvirtd[6320]: Unable to remove disk metadata on vm testguest from
/var/lib/uvtool/libvirt/images/testguest.qcow (disk target vda)
I found that this is due to:
qemuBlockRemoveImageMetadata (the one that emits the warning)
-> qemuSecurityMoveImageMetadata
-> virSecurityManagerMoveImageMetadata
-> virSecurityDACMoveImageMetadata
-> virSecurityDACMoveImageMetadataHelper
-> virProcessRunInFork (spawns subprocess)
-> virSecurityMoveRememberedLabel
Since this is built without HAVE_LIBATTR the following will happen
461 if (virFileGetXAttrQuiet(src, ref_name, _value) < 0) {
(gdb) n
462 if (errno == ENOSYS || errno == ENOTSUP) {
(gdb) p errno
$32 = 38
And that is due to !HAVE_LIBATTR which maps the implementation onto:
4412 #else /* !HAVE_LIBATTR */
4413
4414 int
4415 virFileGetXAttrQuiet(const char *path G_GNUC_UNUSED,
4416 const char *name G_GNUC_UNUSED,
4417 char **value G_GNUC_UNUSED)
4418 {
4419 errno = ENOSYS;
4420 return -1;
4421 }
Due to that we see the two messages reported above
a) internal errors -> for the subprocess that failed
b) "Unable to remove disk metadata" -> but this time due to DAC
instead of apparmor in the security stack
I'm not sure what you'd prefer Michal, maybe an early RC=0 exit in
virSecurityMoveRememberedLabel in case of !HAVE_LIBATTR?
Con: That would still fork the process to do nothing then
Pro: It would but be a small change in just one place
Since you did all the related changes I thought I report the case and
leave it up to you Michal, what do you think?
[1]: https://gitlab.com/libvirt/libvirt/-/commit/706e68237f5
[2]: https://gitlab.com/libvirt/libvirt/-/commit/d73f3f58360
[3]: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1879325
> Thanks!
> Michal
>
--
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd
Re: [libvirt PATCH 0/3] cpu_map: Add Cooperlake x86 CPU model
On Sun, May 24, 2020 at 08:10:18PM +0200, Jiri Denemark wrote: > Jiri Denemark (3): > cputest: Add data for Cooperlake CPU > cpu_map: Add pschange-mc-no bit in IA32_ARCH_CAPABILITIES MSR > cpu_map: Add Cooperlake x86 CPU model Reviewed-by: Pavel Hrdina signature.asc Description: PGP signature
Re: [PATCH 5/5] qemuProcessUpdateCPU: do not change 'fallback' to ALLOW for pSeries guests
On Fri, May 22, 2020 at 16:56:20 -0300, Daniel Henrique Barboza wrote:
> Commit v3.10.0-182-g237f045d9a ("qemu: Ignore fallback CPU attribute
> on reconnect") forced CPU 'fallback' to ALLOW, regardless of user
> choice. This fixed a situation in which guests created with older
> Libvirt versions, which used CPU mode 'host-model' in runtime, would
> fail to launch in a newer Libvirt if the fallback was set to FORBID.
> This would lead to a scenario where the CPU was translated to 'host-model'
> to 'custom', but then the FORBID setting would make the translation
> process fail.
>
> This fix has a side effect for PSeries guests. PSeries can operate
> with 'host-model' in runtime due to specific PPC64 mechanics regarding
> compatibility mode. In fact, the update() implementation of the
> cpuDriverPPC64 driver is a NO-OP if CPU mode is 'host-model', and
> the driver does not implement translate(). The result is that PSeries
> guests aren't affected by the problem, but they are being affected by
> the fix - users are seeing 'fallback' mode being changed without
> necessity during daemon restart.
>
> All other cpuArchDrivers implements update() and changes guest mode
> to VIR_CPU_MODE_CUSTOM, meaning that PSeries is currently the only
> exception to this logic. Let's make it official.
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1660711
>
> CC: Jiri Denemark
> Signed-off-by: Daniel Henrique Barboza
> ---
> src/qemu/qemu_process.c | 8 +++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index a1ef1d42b0..fec1720f33 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -4347,8 +4347,14 @@ qemuProcessUpdateCPU(virQEMUDriverPtr driver,
>
> /* The host CPU model comes from host caps rather than QEMU caps so
> * fallback must be allowed no matter what the user specified in the XML.
> + *
> + * Note: PSeries domains are able to run with host-model CPU by design,
> + * even on Libvirt newer than 2.3, never replacing host-model with
> + * custom in the virCPUUpdate() call prior to this function. It is not
> + * needed to change the user defined 'fallback' attribute in this case.
> */
> -vm->def->cpu->fallback = VIR_CPU_FALLBACK_ALLOW;
> +if (!qemuDomainIsPSeries(vm->def))
> +vm->def->cpu->fallback = VIR_CPU_FALLBACK_ALLOW;
>
> if (qemuProcessFetchGuestCPU(driver, vm, asyncJob, , ) < 0)
> return -1;
qemuProcessUpdateCPU should not be called at all in this case. The
caller (qemuProcessRefreshCPU) is supposed to decide whether the guest
CPU needs to be changed:
/* If the domain with a host-model CPU was started by an old libvirt
* (< 2.3) which didn't replace the CPU with a custom one, let's do it now
* since the rest of our code does not really expect a host-model CPU in a
* running domain.
*/
if (vm->def->cpu->mode == VIR_CPU_MODE_HOST_MODEL) {
if (!(hostmig = virCPUCopyMigratable(host->arch, host)))
return -1;
if (!(cpu = virCPUDefCopyWithoutModel(hostmig)) ||
virCPUDefCopyModelFilter(cpu, hostmig, false,
virQEMUCapsCPUFilterFeatures,
>arch) < 0)
return -1;
if (virCPUUpdate(vm->def->os.arch, vm->def->cpu, cpu) < 0)
return -1;
if (qemuProcessUpdateCPU(driver, vm, QEMU_ASYNC_JOB_NONE) < 0)
return -1;
So better solution would be to move your new comment and check just
after the check for VIR_CPU_MODE_HOST_MODEL:
if (vm->def->cpu->mode == VIR_CPU_MODE_HOST_MODEL) {
/* PSeries... */
if (qemuDomainIsPSeries(vm->def))
return 0;
...
Jirka
Re: [PATCH 4/5] qemu_process.c: modernize qemuProcessUpdateCPU code path
On Fri, May 22, 2020 at 16:56:19 -0300, Daniel Henrique Barboza wrote: > Use automatic cleanup on qemuProcessUpdateCPU and the functions called > by it. > > Signed-off-by: Daniel Henrique Barboza > --- > src/qemu/qemu_process.c | 50 ++--- > 1 file changed, 17 insertions(+), 33 deletions(-) Reviewed-by: Jiri Denemark I pushed patches 1-4.
Re: [PATCH 3/5] cpu_s390.c: modernize virCPUs390Update
On Fri, May 22, 2020 at 16:56:18 -0300, Daniel Henrique Barboza wrote: > Use automatic cleanup of variables. > > Signed-off-by: Daniel Henrique Barboza > --- > src/cpu/cpu_s390.c | 18 +++--- > 1 file changed, 7 insertions(+), 11 deletions(-) Reviewed-by: Jiri Denemark
Re: [PATCH 2/5] cpu_arm.c: modernize virCPUarmUpdate
On Fri, May 22, 2020 at 16:56:17 -0300, Daniel Henrique Barboza wrote: > Use automatic cleanup of variables. > > Signed-off-by: Daniel Henrique Barboza > --- > src/cpu/cpu_arm.c | 14 +- > 1 file changed, 5 insertions(+), 9 deletions(-) Reviewed-by: Jiri Denemark
Re: [PATCH 1/5] cpu_conf.c: modernize virCPUDefCopyWithoutModel and virCPUDefCopy
On Fri, May 22, 2020 at 16:56:16 -0300, Daniel Henrique Barboza wrote: > Use automatic cleanup of variables. > > Signed-off-by: Daniel Henrique Barboza > --- > src/conf/cpu_conf.c | 22 +++--- > 1 file changed, 7 insertions(+), 15 deletions(-) Reviewed-by: Jiri Denemark
Re: [PATCH 4/5] qemu: Prefer -numa cpu over -numa node,cpus=
On 5/22/20 7:18 PM, Igor Mammedov wrote: On Fri, 22 May 2020 18:28:31 +0200 Michal Privoznik wrote: On 5/22/20 6:07 PM, Igor Mammedov wrote: On Fri, 22 May 2020 16:14:14 +0200 Michal Privoznik wrote: QEMU is trying to obsolete -numa node,cpus= because that uses ambiguous vCPU id to [socket, die, core, thread] mapping. The new form is: -numa cpu,node-id=N,socket-id=S,die-id=D,core-id=C,thread-id=T which is repeated for every vCPU and places it at [S, D, C, T] into guest NUMA node N. While in general this is magic mapping, we can deal with it. Firstly, with QEMU 2.7 or newer, libvirt ensures that if topology is given then maxvcpus must be sockets * dies * cores * threads (i.e. there are no 'holes'). Secondly, if no topology is given then libvirt itself places each vCPU into a different socket (basically, it fakes topology of: [maxvcpus, 1, 1, 1]) Thirdly, we can copy whatever QEMU is doing when mapping vCPUs onto topology, to make sure vCPUs don't start to move around. Note, migration from old to new cmd line works and therefore doesn't need any special handling. Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1678085 Signed-off-by: Michal Privoznik --- src/qemu/qemu_command.c | 108 +- .../hugepages-nvdimm.x86_64-latest.args | 4 +- ...memory-default-hugepage.x86_64-latest.args | 10 +- .../memfd-memory-numa.x86_64-latest.args | 10 +- ...y-hotplug-nvdimm-access.x86_64-latest.args | 4 +- ...ry-hotplug-nvdimm-align.x86_64-latest.args | 4 +- ...ry-hotplug-nvdimm-label.x86_64-latest.args | 4 +- ...ory-hotplug-nvdimm-pmem.x86_64-latest.args | 4 +- ...ory-hotplug-nvdimm-ppc64.ppc64-latest.args | 4 +- ...hotplug-nvdimm-readonly.x86_64-latest.args | 4 +- .../memory-hotplug-nvdimm.x86_64-latest.args | 4 +- ...vhost-user-fs-fd-memory.x86_64-latest.args | 4 +- ...vhost-user-fs-hugepages.x86_64-latest.args | 4 +- ...host-user-gpu-secondary.x86_64-latest.args | 3 +- .../vhost-user-vga.x86_64-latest.args | 3 +- 15 files changed, 158 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 7d84fd8b5e..0de4fe4905 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7079,6 +7079,91 @@ qemuBuildNumaOldCPUs(virBufferPtr buf, } +/** + * qemuTranlsatevCPUID: + * + * For given vCPU @id and vCPU topology (@cpu) compute corresponding + * @socket, @die, @core and @thread). This assumes linear topology, + * that is every [socket, die, core, thread] combination is valid vCPU + * ID and there are no 'holes'. This is ensured by + * qemuValidateDomainDef() if QEMU_CAPS_QUERY_HOTPLUGGABLE_CPUS is + * set. I wouldn't make this assumption, each machine can have (and has) it's own layout, and now it's not hard to change that per machine version if necessary. I'd suppose one could pull the list of possible CPUs from QEMU started in preconfig mode with desired -smp x,y,z using QUERY_HOTPLUGGABLE_CPUS and then continue to configure numa with QMP commands using provided CPUs layout. Continue where? At the 'preconfig mode' the guest is already started, isn't it? Are you suggesting that libvirt starts a dummy QEMU process, fetches the CPU topology from it an then starts if for real? Libvirt QEMU started but it's very far from starting guest, at that time it's possible configure numa mapping at runtime and continue to -S or running state without restarting QEMU. For the follow up starts, used topology and numa options can be cached and reused at CLI time as long as machine/-smp combination stays the same. This is a problem. The domain XML that is provided can't be changed, mostly because mgmt apps construct it on the fly and then just pass it as a RO string to libvirt. While libvirt could create a separate cache, there has to be a better way. I mean, I can add some more code that once the guest is running preserves the mapping during migration. But that assumes a running QEMU. When starting a domain from scratch, is it acceptable it vCPU topology changes? I suspect it is not. tries to avoid that as much as it can. How to present it to libvirt user I'm not sure (give them that list perhaps and let select from it???) This is what I am trying to figure out in the cover letter. Maybe we need to let users configure the topology (well, vCPU id to [socket, die, core, thread] mapping), but then again, in my testing the guest ignored that and displayed different topology (true, I was testing with -cpu host, so maybe that's why). there is ongiong issue with EPYC VCPUs topology, but I otherwise it should work. Just report bug to qemu-devel, if it's broken. But it's irrelevant, to the patch, magical IDs for socket/core/...whatever should not be generated by libvirt anymore, but rather taken from QEMU for given machine + -smp combination. Taken when? We can do this for running machines, but not for
Re: [PATCH 0/3] Support network interface downscript
On 5/22/20 7:16 PM, Chen Hanxiao wrote: At 2020-05-22 23:09:21, "Michal Privoznik" wrote: On 5/21/20 3:59 PM, Chen Hanxiao wrote: QEMU has the ability to run a script when a NIC is brought up and down. Libvirt only enables use of the up script at this time. This series add support for postscript when NIC is down/detached. Chen Hanxiao (3): downscript: Support network interface downscript downscript: add test case doc: downscript: updating the documentation docs/formatdomain.html.in | 6 ++- docs/schemas/domaincommon.rng | 8 src/conf/domain_conf.c | 9 src/conf/domain_conf.h | 1 + src/qemu/qemu_extdevice.c | 4 ++ src/qemu/qemu_hotplug.c | 6 +++ tests/qemuxml2argvdata/downscript.xml | 60 + tests/qemuxml2xmloutdata/downscript.xml | 60 + tests/qemuxml2xmltest.c | 1 + 9 files changed, 154 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/downscript.xml create mode 100644 tests/qemuxml2xmloutdata/downscript.xml I don't mean to be picky, especially with new contributors (well, you have one contribution already, exactly one month ago). Anyway, we usually structure patches differently. In the first patch we add XML parsing, formatting, RNG change, documentation and xml2xml test case. In the second the feature is implemented in the driver and xml2argv test case is introduced. In the third patch the news.xml is adjusted. The reason is that this way it is easier for downstream maintainers to backport patches. Mind posting a v2 which follows that? Apart from a small nit in 1/3 the rest looks good. I've contributed to libvirt for years(from another email), but did not send patched for years either :). Ah sorry about that. I've done quick 'git log --author=' and found only one commit. BTW: you can update .mailmap in the top most directory to join authorship of older commits. https://git-scm.com/docs/git-shortlog#_mapping_authors I'll post a v2 to address all the comments aroud the weekends. Okay, I will review them. Michal
