Re: [libvirt PATCH 3/9] qemu: add -display dbus capability check (to update to 6.3)
On Fri, Nov 05, 2021 at 14:51:13 +0400, marcandre.lur...@redhat.com wrote:
> From: Marc-André Lureau
>
> Signed-off-by: Marc-André Lureau
> ---
> src/qemu/qemu_capabilities.c | 2 ++
> src/qemu/qemu_capabilities.h | 1 +
> tests/qemucapabilitiesdata/caps_6.2.0.x86_64.replies | 10 +-
> tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml | 1 +
> 4 files changed, 13 insertions(+), 1 deletion(-)
[...]
> diff --git a/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.replies
> b/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.replies
> index 69d3b1b12a6e..e823ad1188b8 100644
> --- a/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.replies
> +++ b/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.replies
> @@ -5000,6 +5000,10 @@
> {
>"case": "spice-app",
>"type": "0"
> +},
> +{
> + "case": "dbus",
> + "type": "0"
> }
>],
>"members": [
> @@ -11456,6 +11460,9 @@
> },
> {
>"name": "spice-app"
> +},
> +{
> + "name": "dbus"
> }
>],
>"meta-type": "enum",
> @@ -11465,7 +11472,8 @@
> "sdl",
> "egl-headless",
> "curses",
> -"spice-app"
> +"spice-app",
> +"dbus"
I didn't find this one in my latest re-generation of the capabilities
from the current master. Do I need to enable/add some dependencies, or
is this something that is not upstream yet? In case if it isn't upstream,
these hunks are obviously not acceptable.
Re: [libvirt PATCH] vz: fix vzCapsAddGuestDomain
On 11/29/21 17:16, Ján Tomko wrote: > There is a stray 'return -1' executed on all code paths. > > Fixes: c18d9e23fafabcfbb80481e0705931036b8e7331 > Signed-off-by: Ján Tomko > --- > src/vz/vz_driver.c | 2 -- > 1 file changed, 2 deletions(-) > Ooops. Reviewed-by: Michal Privoznik Michal
Re: [libvirt PATCH 4/9] qemu: add -display dbus support
On 11/5/21 11:51, marcandre.lur...@redhat.com wrote: > From: Marc-André Lureau > > Without any extra option, libvirt will start and tell QEMU to connect to > the private VM bus. > > Instead, a D-Bus "address" to connect to can be specified. Or the p2p > mode enabled. > > D-Bus display best works with GL & a rendernode, which can be specified > with child element. > > Signed-off-by: Marc-André Lureau > --- > docs/schemas/basictypes.rng | 7 ++ > docs/schemas/domaincommon.rng | 33 ++ > src/conf/domain_conf.c| 65 ++- > src/conf/domain_conf.h| 7 ++ > src/libxl/libxl_conf.c| 1 + > src/qemu/qemu_capabilities.c | 2 + > src/qemu/qemu_command.c | 57 ++-- > src/qemu/qemu_domain.c| 1 + > src/qemu/qemu_driver.c| 10 ++- > src/qemu/qemu_hotplug.c | 1 + > src/qemu/qemu_process.c | 11 +++- > src/qemu/qemu_validate.c | 1 + > src/vmx/vmx.c | 1 + > .../domaincapsdata/qemu_6.2.0-q35.x86_64.xml | 1 + > .../domaincapsdata/qemu_6.2.0-tcg.x86_64.xml | 1 + > tests/domaincapsdata/qemu_6.2.0.x86_64.xml| 1 + > .../graphics-dbus-address.args| 31 + > .../graphics-dbus-address.xml | 34 ++ > tests/qemuxml2argvdata/graphics-dbus-p2p.args | 31 + > tests/qemuxml2argvdata/graphics-dbus-p2p.xml | 36 ++ > tests/qemuxml2argvdata/graphics-dbus.args | 31 + > tests/qemuxml2argvdata/graphics-dbus.xml | 34 ++ > tests/qemuxml2argvtest.c | 7 ++ > .../graphics-dbus-address.xml | 39 +++ > .../qemuxml2xmloutdata/graphics-dbus-p2p.xml | 39 +++ > tests/qemuxml2xmloutdata/graphics-dbus.xml| 39 +++ > tests/qemuxml2xmltest.c | 10 +++ > 27 files changed, 523 insertions(+), 8 deletions(-) > create mode 100644 tests/qemuxml2argvdata/graphics-dbus-address.args > create mode 100644 tests/qemuxml2argvdata/graphics-dbus-address.xml > create mode 100644 tests/qemuxml2argvdata/graphics-dbus-p2p.args > create mode 100644 tests/qemuxml2argvdata/graphics-dbus-p2p.xml > create mode 100644 tests/qemuxml2argvdata/graphics-dbus.args > create mode 100644 tests/qemuxml2argvdata/graphics-dbus.xml > create mode 100644 tests/qemuxml2xmloutdata/graphics-dbus-address.xml > create mode 100644 tests/qemuxml2xmloutdata/graphics-dbus-p2p.xml > create mode 100644 tests/qemuxml2xmloutdata/graphics-dbus.xml We like to separate changes to public structures (like XML and/or APIs) and changes to drviers. IOW, this should be split into two patches, in the first you include all XML related changes (parsing, formatting) can even introduce those .args files so that xml2xmltest can check XML parsing & formatting. In here, you can put 'case VIR_DOMAIN_GRAPHICS_TYPE_DBUS:' just next to 'VIR_DOMAIN_GRAPHICS_TYPE_LAST' so that using dbus type errors out. And then in the second patch you provide the qemu implementation. Oh, and don't forget to document your changes in docs/formatdomain.rst so that users know there's a new type and what attributes they can set. > > diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng > index a221ff6295c0..ae4d5682229c 100644 > --- a/docs/schemas/basictypes.rng > +++ b/docs/schemas/basictypes.rng > @@ -220,6 +220,13 @@ > > > > > + > + > + > + .+ > + > + Or use "filePath" type. They are the same. > + > > > > diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng > index f01b7a64704b..1dba199db7ec 100644 > --- a/docs/schemas/domaincommon.rng > +++ b/docs/schemas/domaincommon.rng > @@ -3996,6 +3996,39 @@ > > > > + > + > +dbus > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > > > rdp > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index da0c64b46009..8a2f3c4115e0 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -911,6 +911,7 @@ VIR_ENUM_IMPL(virDomainGraphics, >"desktop", >"spice", >"egl-headless", > + "dbus", > ); > > VIR_ENUM_IMPL(virDo
Re: [libvirt PATCH 1/9] qemu: start the D-Bus daemon as needed
On 11/5/21 11:51, marcandre.lur...@redhat.com wrote:
> From: Marc-André Lureau
>
> The daemon is started on daemon in external devices and hotplug code.
> Add a mechanism to start it before qemu, if qemu itself needs it. (it is
> already stopped in qemuProcessStop)
>
> Signed-off-by: Marc-André Lureau
> ---
> src/qemu/qemu_domain.h | 1 +
> src/qemu/qemu_process.c | 4
> 2 files changed, 5 insertions(+)
>
> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
> index 6728ab047ed0..575fb8393b83 100644
> --- a/src/qemu/qemu_domain.h
> +++ b/src/qemu/qemu_domain.h
> @@ -237,6 +237,7 @@ struct _qemuDomainObjPrivate {
> /* running backup job */
> virDomainBackupDef *backup;
>
> +bool dbusDaemonWanted;
> bool dbusDaemonRunning;
>
> /* list of Ids to migrate */
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index d5f8a47ac293..4ca9b100a802 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -7284,6 +7284,10 @@ qemuProcessLaunch(virConnectPtr conn,
> &nnicindexes, &nicindexes, 0)))
> goto cleanup;
>
> +if (QEMU_DOMAIN_PRIVATE(vm)->dbusDaemonWanted &&
> +qemuDBusStart(driver, vm) < 0)
> +goto cleanup;
> +
We already have priv variable, so s/QEMU_DOMAIN_PRIVATE(vm)/priv/.
However, we already have qemuExtDevicesStart() which I believe can be
used to check whether dbus graphics was defined and thus whether dbus
daemon needs to be started.
> if (incoming && incoming->fd != -1)
> virCommandPassFD(cmd, incoming->fd, 0);
>
>
Michal
Re: [PATCH 2/3] virsh: Add '--full-seclabels' option for dominfo
On Thu, Sep 02, 2021 at 08:29:35PM +0800, Luke Yue wrote:
There is no virsh command uses virDomainGetSecurityLabelList API, so add
an option for dominfo to call it and print full list of security labels.
Also realign some outputs as it's now "Security labels:" instead of
"Security label:".
This should go into separate patch as that makes it nicer to review and
the separation of changes makes it more clear.
Signed-off-by: Luke Yue
---
docs/manpages/virsh.rst | 5 +-
tests/virsh-undefine | 8 ++--
tests/virshtest.c| 70 ++--
tools/virsh-domain-monitor.c | 89
4 files changed, 101 insertions(+), 71 deletions(-)
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index f7cf82acdf..2b2746e713 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -1299,29 +1304,53 @@ cmdDominfo(vshControl *ctl, const vshCmd *cmd)
} else {
/* Only print something if a security model is active */
if (secmodel.model[0] != '\0') {
-vshPrint(ctl, "%-15s %s\n", _("Security model:"), secmodel.model);
-vshPrint(ctl, "%-15s %s\n", _("Security DOI:"), secmodel.doi);
-
-/* Security labels are only valid for active domains */
-seclabel = g_new0(virSecurityLabel, 1);
+vshPrint(ctl, "%-16s %s\n", _("Security model:"), secmodel.model);
+vshPrint(ctl, "%-16s %s\n", _("Security DOI:"), secmodel.doi);
+
+if (fullseclabels) {
+int len;
+size_t i;
+
+if ((len = virDomainGetSecurityLabelList(dom, &seclabel)) < 0)
{
+g_clear_pointer(&(seclabel), g_free);
+return false;
+} else {
No need for else branch when the first branch returns.
+for (i = 0; i < len; i++)
+if (seclabel[i].label[0] != '\0')
+vshPrint(ctl, "%-16s %s (%s)\n",
+ i == 0 ? _("Security labels:") : "",
+ seclabel[i].label,
+ seclabel[i].enforcing ?
+ "enforcing" :
+ "permissive");
+}
-if (virDomainGetSecurityLabel(dom, seclabel) == -1) {
-VIR_FREE(seclabel);
-return false;
+g_clear_pointer(&seclabel, g_free);
} else {
-if (seclabel->label[0] != '\0')
-vshPrint(ctl, "%-15s %s (%s)\n", _("Security label:"),
- seclabel->label, seclabel->enforcing ? "enforcing" :
"permissive");
-}
+/* Security labels are only valid for active domains */
+seclabel = g_new0(virSecurityLabel, 1);
+
+if (virDomainGetSecurityLabel(dom, seclabel) == -1) {
You properly used `< 0` before, this should do the same then.
+g_clear_pointer(&seclabel, g_free);
+return false;
+} else {
Again, no need for an else branch when if branch returns.
Otherwise it looks good.
signature.asc
Description: PGP signature
Re: [PATCH v7 0/2] Dirty Ring support (Libvirt)
Ping Any corrections and suggetions are welcome. Please review, thanks! 在 2021/11/23 22:36, huang...@chinatelecom.cn 写道: From: "Hyman Huang(黄勇)" v7 - rebase on master - modify the following points according to the advice given by Peter 1. skip the -accel switch and reuse the existing commit d20ebdda2 'qemu: Switch to -accel' 2. remove the post-parse function and do the parse work in virDomainFeaturesKVMDefParse once for all 3. throw an error if "size" not specified when kvm-dirty-ring feature enabled in xml 4. fix the memory leak when parsing xml 5. use macro VIR_ROUND_UP_POWER_OF_TWO to check power-of-two 6. put error messages in one line 7. squash the last 2 commit into 1 8. add test for kvm-dirty-ring feature Thanks for the careful reviews made by Peter. Please review, Thanks! Hyman Ping for this series. I still keep thinking the dirty ring feature is something good to have for libvirt. qemu-6.1 has supported dirty ring feature and followed up with the commit 0e21bf24 "support dirtyrate at the granualrity of vcpu", which is a typical usage scenario of dirty ring. another usage scenario may be the implementation of per-vcpu auto-converge during live migration which is already being reviewed. so we can make full use of dirty ring feature if libvirt supports. and any corrections and comments about this series would be very appreciated. Please review, Thanks! Hyman v6 - rebase on master v5,v4: blank, just make v6 be the the latest version. v3 - rebase master and fix the confilict when apply "conf: introduce dirty_ring_size in struct "_virDomainDef" to current master. v2 - split patchset into 4 patches - leave out the tcg case when building commandline. - handle the VIR_DOMAIN_KVM_DIRTY_RING case independently in , virDomainFeatureDefParse and virDomainDefFeaturesCheckABIStability, do not integrate it with other cases... - add dirty ring size check in virDomainDefFeaturesCheckABIStability - modify zero checks on integers of dirty ring size in a explicit way. - set the default value of dirty ring size in a post-parser callback. - check the absence of kvm_feature in a explicit way. - code clean of virTristateSwitchTypeToString function. this version's modification base on Peter's advices mostly, thanks a lot, please review ! v1 since qemu has introduced a dirty ring feature in 6.1.0, may be it's the right time to introduce dirty ring in libvirt meanwhile. this patch add feature named 'dirty-ring', which enable dirty ring feature when starting vm. to try this out, three things has done in this patchset: - introduce QEMU_CAPS_ACCEL so the the libvirt can use it to select the right option when specifying the accelerator type. - switch the option "-machine accel=xxx" to "-accel xxx" when specifying accelerator type once libvirt build QEMU command line, so that dirty-ring-size property can be passed to qemu when start vm. - introduce dirty_ring_size to hold the ring size configured by user and pass dirty_ring_size when building qemu commandline if dirty ring feature enabled. though dirty ring is per-cpu logically, the size of dirty ring is registered by 'struct kvm' in QEMU. so we would like to place the dirty_ring_size as a property of vm in Libvirt as the QEMU do. the dirty ring feature is disabled by default, and if enabled, the default value of ring size if 4096 if size not configured. for more details about dirty ring and "-accel" option, please refer to: https://lore.kernel.org/qemu-devel/20210108165050.406906-10-pet...@redhat.com/ https://lore.kernel.org/qemu-devel/3aa73987-40e8-3619-0723-9f17f7385...@redhat.com/ please review, Thanks! Best Regards ! Hyman Huang(黄勇) (2): qemu: support dirty ring feature tests: add test for kvm-dirty-ring feature docs/formatdomain.rst | 18 --- docs/schemas/domaincommon.rng | 10 src/conf/domain_conf.c| 54 +++ src/conf/domain_conf.h| 4 ++ src/qemu/qemu_command.c | 12 + tests/qemuxml2argvdata/kvm-features-off.xml | 1 + tests/qemuxml2argvdata/kvm-features.args | 2 +- tests/qemuxml2argvdata/kvm-features.xml | 1 + tests/qemuxml2xmloutdata/kvm-features-off.xml | 1 + tests/qemuxml2xmloutdata/kvm-features.xml | 1 + 10 files changed, 95 insertions(+), 9 deletions(-)
[libvirt PATCH] vz: fix vzCapsAddGuestDomain
There is a stray 'return -1' executed on all code paths. Fixes: c18d9e23fafabcfbb80481e0705931036b8e7331 Signed-off-by: Ján Tomko --- src/vz/vz_driver.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index b47266290c..23b7795035 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -85,8 +85,6 @@ vzCapsAddGuestDomain(virCaps *caps, guest = virCapabilitiesAddGuest(caps, ostype, arch, emulator, NULL, 0, NULL); -return -1; - virCapabilitiesAddGuestDomain(guest, virt_type, NULL, NULL, 0, NULL); -- 2.31.1
Re: [PATCH 1/3] test_driver: Implement virDomainGetSecurityLabelList
On Thu, Sep 02, 2021 at 08:29:34PM +0800, Luke Yue wrote:
Signed-off-by: Luke Yue
---
src/test/test_driver.c | 42 ++
1 file changed, 42 insertions(+)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 2f19b7c520..1b5914c890 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -137,6 +137,7 @@ VIR_ONCE_GLOBAL_INIT(testDriver);
#define TEST_MODEL "i686"
#define TEST_EMULATOR "/usr/bin/test-hv"
+#define TEST_SECURITY_LABEL_LIST_LENGTH 2
static const virNodeInfo defaultNodeInfo = {
TEST_MODEL,
@@ -5261,6 +5262,46 @@ testDomainGetSecurityLabel(virDomainPtr dom,
return ret;
}
+static int
+testDomainGetSecurityLabelList(virDomainPtr dom,
+ virSecurityLabelPtr* seclabels)
+{
+virDomainObj *vm;
+size_t i;
+int ret = -1;
+
+if (!(vm = testDomObjFromDomain(dom)))
+return -1;
+
+if (!virDomainObjIsActive(vm)) {
+/* No seclabels */
+*seclabels = NULL;
+ret = 0;
+} else {
+int len = TEST_SECURITY_LABEL_LIST_LENGTH;
+
+(*seclabels) = g_new0(virSecurityLabel, len);
+memset(*seclabels, 0, sizeof(**seclabels) * len);
+
+/* Fill the array */
+for (i = 0; i < len; i++) {
+if (virStrcpyStatic((*seclabels)[i].label, "libvirt-test") < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("security label exceeds maximum: %zu"),
+ sizeof((*seclabels)[i].label) - 1);
+g_clear_pointer(seclabels, g_free);
+goto cleanup;
+}
+(*seclabels)[i].enforcing = 1;
There could be some variety here, like:
if (i == 0)
(*seclabels)[i].enforcing = 1
But that's just a nitpick.
+}
+ret = len;
+}
+
+ cleanup:
+virDomainObjEndAPI(&vm);
+return ret;
+}
+
static int
testNodeGetSecurityModel(virConnectPtr conn,
virSecurityModelPtr secmodel)
@@ -9615,6 +9656,7 @@ static virHypervisorDriver testHypervisorDriver = {
.domainGetVcpuPinInfo = testDomainGetVcpuPinInfo, /* 1.2.18 */
.domainGetMaxVcpus = testDomainGetMaxVcpus, /* 0.7.3 */
.domainGetSecurityLabel = testDomainGetSecurityLabel, /* 7.5.0 */
+.domainGetSecurityLabelList = testDomainGetSecurityLabelList, /* 7.8.0 */
.nodeGetSecurityModel = testNodeGetSecurityModel, /* 7.5.0 */
.domainGetXMLDesc = testDomainGetXMLDesc, /* 0.1.4 */
.domainSetMemoryParameters = testDomainSetMemoryParameters, /* 5.6.0 */
--
2.33.0
signature.asc
Description: PGP signature
Re: [libvirt PATCH 00/10] Enable hyperv-passthrough
On a Friday in 2021, Tim Wiederhake wrote: This series enables "hv-passthrough" in libvirt. See https://bugzilla.redhat.com/show_bug.cgi?id=1851249. Example usage in VM definition: Tim Wiederhake (10): schema: Wrap hyperv element in choice and group schema: Add optional "mode" attribute to hyperv conf: domain: Define enum for HyperV mode virDomainFeaturesHyperVDefParse: Read attribute "mode" of element "hyperv" virDomainDefFormatFeatures: Write attribute "mode" of element "hyperv" docs: domain: Add documentation for "hyperv"'s new "mode" attribute conf: domain: Add hyperv passthrough mode schema: hyperv: Add mode "passthrough" tests: Add tests for hyperv-passthrough docs: domain: Add documentation for hyperv passthrough mode docs/formatdomain.rst | 13 +- docs/schemas/domaincommon.rng | 172 ++ src/conf/domain_conf.c| 23 ++- src/conf/domain_conf.h| 8 + src/qemu/qemu_command.c | 18 +- src/qemu/qemu_validate.c | 2 +- .../hyperv-passthrough.x86_64-6.1.0.args | 32 .../hyperv-passthrough.x86_64-latest.args | 32 tests/qemuxml2argvdata/hyperv-passthrough.xml | 27 +++ tests/qemuxml2argvtest.c | 2 + tests/qemuxml2xmloutdata/hyperv-off.xml | 2 +- .../qemuxml2xmloutdata/hyperv-passthrough.xml | 31 .../hyperv-stimer-direct.xml | 2 +- tests/qemuxml2xmloutdata/hyperv.xml | 2 +- tests/qemuxml2xmltest.c | 1 + 15 files changed, 277 insertions(+), 90 deletions(-) create mode 100644 tests/qemuxml2argvdata/hyperv-passthrough.x86_64-6.1.0.args create mode 100644 tests/qemuxml2argvdata/hyperv-passthrough.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/hyperv-passthrough.xml create mode 100644 tests/qemuxml2xmloutdata/hyperv-passthrough.xml Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [libvirt PATCH 06/10] docs: domain: Add documentation for "hyperv"'s new "mode" attribute
On a Friday in 2021, Tim Wiederhake wrote: Signed-off-by: Tim Wiederhake --- docs/formatdomain.rst | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index eb8c973cf1..95ef2e0d05 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -1820,7 +1820,7 @@ Hypervisors may allow certain CPU / machine features to be toggled on/off. - + @@ -1918,6 +1918,14 @@ are: evmcs Enable Enlightened VMCS on, off :since:`4.10.0 (QEMU 3.1)` === == === + :since:`Since 7.11.0` , the hypervisor can be configured further by setting The next libvirt release will be 8.0.0 mid-January. https://libvirt.org/downloads.html#numbering Jano signature.asc Description: PGP signature
Re: [PATCH v2] tools: fix iterating over argv when recovering xattr
On Fri, Nov 26, 2021 at 03:46:56PM +, Daniel P. Berrangé wrote:
The libvirt_recover_xattrs.sh tool hangs when run. When not flags
s/not/no/
are provided OPTIND is 1, so the loop expands to 'shift 0' which
has not effect. Rewrite to just loop over $@ instead which involves
less cleverness.
Signed-off-by: Daniel P. Berrangé
Reviewed-by: Martin Kletzander
---
tools/libvirt_recover_xattrs.sh | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/libvirt_recover_xattrs.sh b/tools/libvirt_recover_xattrs.sh
index be6ee84b5f..35c164a9c7 100755
--- a/tools/libvirt_recover_xattrs.sh
+++ b/tools/libvirt_recover_xattrs.sh
@@ -106,9 +106,9 @@ fix_xattrs() {
shift $((OPTIND - 1))
if [ $# -gt 0 ]; then
-while [ $# -gt 0 ]; do
-fix_xattrs "$1"
-shift $((OPTIND - 1))
+for arg in "$@"
+do
+fix_xattrs "$arg"
done
else
if [ ${UNSAFE} -eq 1 ]; then
--
2.33.1
signature.asc
Description: PGP signature
Re: [PATCH v3 12/12] tests: Test detach-device and detach-device-alias for test driver
On Mon, 2021-11-29 at 15:50 +0100, Martin Kletzander wrote:
> On Mon, Nov 29, 2021 at 10:41:51PM +0800, Luke Yue wrote:
> > On Fri, 2021-11-26 at 16:44 +0100, Martin Kletzander wrote:
> > > On Wed, Nov 10, 2021 at 10:24:31PM +0800, Luke Yue wrote:
> > > > Signed-off-by: Luke Yue
> > > > ---
> > > > tests/virshtest.c | 96
> > > > +++
> > > > 1 file changed, 96 insertions(+)
> > > >
> > >
> > > This makes the tests very fragile. It would be even easier to do
> > > the
> > > test using the internal APIs, similarly to how we do it with the
> > > qemuhotplugtest.
> > >
> >
> > Thanks for the explanation, I will look into how qemuhotplugtest is
> > implemented.
> >
> > > > diff --git a/tests/virshtest.c b/tests/virshtest.c
> > > > index af2a70f5fb..8e5b397420 100644
> > > > --- a/tests/virshtest.c
> > > > +++ b/tests/virshtest.c
> > > > @@ -160,6 +160,8 @@ static char *custom_uri;
> > > > "--connect", \
> > > > custom_uri
> > > >
> > > > +# define TEST_XML_PATH abs_top_builddir
> > > > "/../examples/xml/test"
> > > > +
> > > > static int testCompareListDefault(const void *data
> > > > G_GNUC_UNUSED)
> > > > {
> > > > const char *const argv[] = { VIRSH_DEFAULT, "list", NULL };
> > > > @@ -437,6 +439,88 @@ static int testIOThreadPin(const void
> > > > *data
> > > > G_GNUC_UNUSED)
> > > > return testCompareOutputLit(exp, "", NULL, argv);
> > > > }
> > > >
> > > > +static int testCompareDetachDevice(const void *data
> > > > G_GNUC_UNUSED)
> > > > +{
> > > > + const char *const argv[] = { VIRSH_CUSTOM, "detach-device
> > > > fc5\
> > > > + " TEST_XML_PATH
> > > > "/testdevif.xml;\
> > > > + detach-device fc5\
> > > > + " TEST_XML_PATH
> > > > "/testdevdiskcdrom.xml;\
> > > > + detach-device fc5\
> > > > + " TEST_XML_PATH
> > > > "/testdevsound.xml;\
> > > > + detach-device fc5\
> > > > + " TEST_XML_PATH
> > > > "/testdevhostdev.xml;\
> > > > + detach-device fc5\
> > > > + " TEST_XML_PATH
> > > > "/testdevlease.xml;\
> > > > + detach-device fc5\
> > > > + " TEST_XML_PATH
> > > > "/testdevcontroller.xml;\
> > > > + detach-device fc5\
> > > > + " TEST_XML_PATH
> > > > "/testdevfs.xml;\
> > > > + detach-device fc5\
> > > > + " TEST_XML_PATH
> > > > "/testdevrng.xml;\
> > > > + detach-device fc5\
> > > > + " TEST_XML_PATH
> > > > "/testdevmem.xml;\
> > > > + detach-device fc5\
> > > > + " TEST_XML_PATH
> > > > "/testdevshmem.xml;\
> > > > + detach-device fc5\
> > > > + " TEST_XML_PATH
> > > > "/testdevwatchdog.xml;\
> > > > + detach-device fc5\
> > > > + " TEST_XML_PATH
> > > > "/testdevinput.xml;\
> > > > + detach-device fc5\
> > > > + " TEST_XML_PATH
> > > > "/testdevvsock.xml;\
> > > > + detach-device fc5\
> > > > + " TEST_XML_PATH
> > > > "/testdevtpm.xml",
> > > > + NULL };
> > > > + const char *exp =
> > > > +"Device detached successfully\n\n\
> > > > +Device detached successfully\n\n\
> > > > +Device detached successfully\n\n\
> > > > +Device detached successfully\n\n\
> > > > +Device detached successfully\n\n\
> > > > +Device detached successfully\n\n\
> > > > +Device detached successfully\n\n\
> > > > +Device detached successfully\n\n\
> > > > +Device detached successfully\n\n\
> > > > +Device detached successfully\n\n\
> > > > +Device detached successfully\n\n\
> > > > +Device detached successfully\n\n\
> > > > +Device detached successfully\n\n\
> > > > +Device detached successfully\n\n";
> > >
> > > If any of these fails it will be a pain to figure out which one
> > > it
> > > was
> > > if the error message does not include the name. Splitting these
> > > is
> > > definitely the way to go.
> > >
> > > > + return testCompareOutputLit(exp, "", NULL, argv);
> > > > +}
> > > > +
> > > > +static int testCompareDetachDeviceError(const void *data
> > > > G_GNUC_UNUSED)
> > > > +{
> > > > + const char *const argv[] = { VIRSH_CUSTOM, "detach-device
> > > > fc5\
> > > > + " TEST_XML_PATH
> > > > "/testdevtpm.xml;\
> > > > + detach-device fc5\
> > > > + " TEST_XML_PA
Re: [PATCH v3 02/12] domain_driver: extract DetachXXXDeviceConfig related functions and use them
On Mon, 2021-11-29 at 15:33 +0100, Martin Kletzander wrote:
> On Mon, Nov 29, 2021 at 09:06:57PM +0800, Luke Yue wrote:
> > On Fri, 2021-11-26 at 16:43 +0100, Martin Kletzander wrote:
> > > On Wed, Nov 10, 2021 at 10:24:21PM +0800, Luke Yue wrote:
> > > > libxl / lxc / qemu drivers share some common codes in their
> > > > DomainDetachDeviceConfig functions, so extract them to
> > > > domain_driver and
> > > > reuse them.
> > > >
> > >
> > > I would argue that these specific functions are actually dealing
> > > just
> > > with the domain definition which is done mostly in domain_conf.c,
> > > so
> > > I
> > > would pick that place.
> > >
> >
> > Thanks for the review! No problem, I would move the code to
> > domain_conf.c in next version.
> >
> > > > At the same time, this will enable test driver to test these
> > > > functions
> > > > with virshtest in the future.
> > > >
> > > > Signed-off-by: Luke Yue
> > > > ---
> > > > Not pretty sure whether this is a proper way to make these
> > > > functions
> > > > reusable, maybe there is a more elegant choice.
> > >
> > > The patch does a good job in deduplicating some of the code and
> > > except
> > > the file placement I think the overall approach is fine.
> > >
> > > However unfortunately the functions are just copy-paste from
> > > various
> > > places and they could be cleaned up to look the same. For
> > > example...
> > >
> > > > ---
> > > > src/hypervisor/domain_driver.c | 266
> > > > +
> > > > src/hypervisor/domain_driver.h | 41 +
> > > > src/libvirt_private.syms | 14 ++
> > > > src/libxl/libxl_driver.c | 41 +
> > > > src/lxc/lxc_driver.c | 37 +
> > > > src/qemu/qemu_driver.c | 123 +++
> > > > 6 files changed, 356 insertions(+), 166 deletions(-)
> > > >
> > > > diff --git a/src/hypervisor/domain_driver.c
> > > > b/src/hypervisor/domain_driver.c
> > > > index 31737b0f4a..01ecb4e30e 100644
> > > > --- a/src/hypervisor/domain_driver.c
> > > > +++ b/src/hypervisor/domain_driver.c
> > > > @@ -644,3 +644,269 @@
> > > > virDomainDriverGetIOThreadsConfig(virDomainDef *targetDef,
> > > >
> > > > return ret;
> > > > }
> > > > +
> > > > +
> > > > +int
> > > > +virDomainDriverDetachDiskDeviceConfig(virDomainDef *vmdef,
> > > > + virDomainDeviceDef *dev)
> > > > +{
> > > > + virDomainDiskDef *disk;
> > > > + virDomainDiskDef *det_disk;
> > > > +
> > > > + disk = dev->data.disk;
> > > > + if (!(det_disk = virDomainDiskRemoveByName(vmdef, disk-
> > > > >dst)))
> > > > {
> > > > + virReportError(VIR_ERR_DEVICE_MISSING, _("no target
> > > > device
> > > > %s"),
> > > > + disk->dst);
> > > > + return -1;
> > > > + }
> > > > + virDomainDiskDefFree(det_disk);
> > > > +
> > > > + return 0;
> > >
> > > This function uses temporary variables for everything and adds an
> > > error
> > > message (which could be moved to the virDomainDiskRemoveByName()
> > > as
> > > all
> > > callers do report the same error message when it is not found.
> > > Other
> > > functions do essentially the same job, but have random comments,
> > > some
> > > have extra helper variables. All that could be nicely unified and
> > > it
> > > would read so much nicer.
> > >
> > > What I really like about this change is that apart from roughly
> > > two
> > > device types, like video and chardev, this function could be
> > > de-duplicated as well. Adding a new functionality to that de-
> > > duplicated
> > > function would then add that functionality into all the drivers
> > > using
> > > this function. Of course the special devices would need some
> > > xmlopt
> > > callbacks or something that drivers can modify themselves for
> > > such
> > > reasons, but such change would be really, really helpful,
> > > especially
> > > in
> > > the long term.
> >
> > So if I understand you correctly, I will unified all these
> > functions
> > into one,
>
> Not really into one, although if you figure out a way to make that
> easily possible feel free to go on, what I meant make them look and
> read
> the same.
>
> > just like the original ones for QEMU / lxc / libxl, but this
> > one is for all, and maybe I should create a new enum for flags of
> > detachable device that different hypervisor support, and xmlopt etc
> > for
> > special devices. I will do that in next version.
> >
>
> Deduplicating the bigger function would be nice to have for the
> future,
> it does not need to happen together in this one. It will require
> some
> more work to make it "modular" enough to be used by various drivers.
OK, thanks, now I see.
Re: [PATCH v3 12/12] tests: Test detach-device and detach-device-alias for test driver
On Mon, Nov 29, 2021 at 10:41:51PM +0800, Luke Yue wrote:
On Fri, 2021-11-26 at 16:44 +0100, Martin Kletzander wrote:
On Wed, Nov 10, 2021 at 10:24:31PM +0800, Luke Yue wrote:
> Signed-off-by: Luke Yue
> ---
> tests/virshtest.c | 96
> +++
> 1 file changed, 96 insertions(+)
>
This makes the tests very fragile. It would be even easier to do the
test using the internal APIs, similarly to how we do it with the
qemuhotplugtest.
Thanks for the explanation, I will look into how qemuhotplugtest is
implemented.
> diff --git a/tests/virshtest.c b/tests/virshtest.c
> index af2a70f5fb..8e5b397420 100644
> --- a/tests/virshtest.c
> +++ b/tests/virshtest.c
> @@ -160,6 +160,8 @@ static char *custom_uri;
> "--connect", \
> custom_uri
>
> +# define TEST_XML_PATH abs_top_builddir "/../examples/xml/test"
> +
> static int testCompareListDefault(const void *data G_GNUC_UNUSED)
> {
> const char *const argv[] = { VIRSH_DEFAULT, "list", NULL };
> @@ -437,6 +439,88 @@ static int testIOThreadPin(const void *data
> G_GNUC_UNUSED)
> return testCompareOutputLit(exp, "", NULL, argv);
> }
>
> +static int testCompareDetachDevice(const void *data G_GNUC_UNUSED)
> +{
> + const char *const argv[] = { VIRSH_CUSTOM, "detach-device fc5\
> + " TEST_XML_PATH "/testdevif.xml;\
> + detach-device fc5\
> + " TEST_XML_PATH
> "/testdevdiskcdrom.xml;\
> + detach-device fc5\
> + " TEST_XML_PATH
> "/testdevsound.xml;\
> + detach-device fc5\
> + " TEST_XML_PATH
> "/testdevhostdev.xml;\
> + detach-device fc5\
> + " TEST_XML_PATH
> "/testdevlease.xml;\
> + detach-device fc5\
> + " TEST_XML_PATH
> "/testdevcontroller.xml;\
> + detach-device fc5\
> + " TEST_XML_PATH "/testdevfs.xml;\
> + detach-device fc5\
> + " TEST_XML_PATH
> "/testdevrng.xml;\
> + detach-device fc5\
> + " TEST_XML_PATH
> "/testdevmem.xml;\
> + detach-device fc5\
> + " TEST_XML_PATH
> "/testdevshmem.xml;\
> + detach-device fc5\
> + " TEST_XML_PATH
> "/testdevwatchdog.xml;\
> + detach-device fc5\
> + " TEST_XML_PATH
> "/testdevinput.xml;\
> + detach-device fc5\
> + " TEST_XML_PATH
> "/testdevvsock.xml;\
> + detach-device fc5\
> + " TEST_XML_PATH
> "/testdevtpm.xml",
> + NULL };
> + const char *exp =
> +"Device detached successfully\n\n\
> +Device detached successfully\n\n\
> +Device detached successfully\n\n\
> +Device detached successfully\n\n\
> +Device detached successfully\n\n\
> +Device detached successfully\n\n\
> +Device detached successfully\n\n\
> +Device detached successfully\n\n\
> +Device detached successfully\n\n\
> +Device detached successfully\n\n\
> +Device detached successfully\n\n\
> +Device detached successfully\n\n\
> +Device detached successfully\n\n\
> +Device detached successfully\n\n";
If any of these fails it will be a pain to figure out which one it
was
if the error message does not include the name. Splitting these is
definitely the way to go.
> + return testCompareOutputLit(exp, "", NULL, argv);
> +}
> +
> +static int testCompareDetachDeviceError(const void *data
> G_GNUC_UNUSED)
> +{
> + const char *const argv[] = { VIRSH_CUSTOM, "detach-device fc5\
> + " TEST_XML_PATH
> "/testdevtpm.xml;\
> + detach-device fc5\
> + " TEST_XML_PATH
> "/testdevtpm.xml;\
> + detach-device fc5 --live\
> + " TEST_XML_PATH
> "/testdevmemballoon.xml",
> + NULL };
> + const char *exp =
> +"Device detached successfully\n\n\n\n";
> + const char *error_msg =
> +"error: Failed to detach device from " TEST_XML_PATH
> "/testdevtpm.xml\n\
> +error: device not found: matching tpm device not found\n\
> +error: Failed to detach device from " TEST_XML_PATH
> "/testdevmemballoon.xml\n\
> +error: Operation not supported: detach of device 'memballoon' on
> running domain is not supported\n";
It'd be also nicer to read and write these tests if they did not rely
on
the output just like this and instead used the internal APIs.
Should I create
Re: [PATCH 2/2] virStorageSourceIsSameLocation: Special-case storage sources of type 'volume'
On Mon, Nov 29, 2021 at 02:31:55PM +0100, Peter Krempa wrote:
On Fri, Nov 26, 2021 at 14:50:37 +0100, Martin Kletzander wrote:
On Fri, Nov 26, 2021 at 02:07:54PM +0100, Peter Krempa wrote:
> The function is used also to compare virStorageSource which may not be
> resolved to the image at that point in which case the 'path' is not yet
> populated and the actual type is not yet set. This means that the
> function fails to consider two identical volume-based disks as pointing
> to the same thing.
>
> Add a special case for both images being type=volume in which case we
> compare only the pool/volume names.
>
> Closes: https://gitlab.com/libvirt/libvirt/-/issues/240
> Signed-off-by: Peter Krempa
> ---
> src/conf/storage_source_conf.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/src/conf/storage_source_conf.c b/src/conf/storage_source_conf.c
> index 44944e1dbd..c0acee189a 100644
> --- a/src/conf/storage_source_conf.c
> +++ b/src/conf/storage_source_conf.c
> @@ -920,6 +920,13 @@ virStorageSourceIsSameLocation(virStorageSource *a,
> virStorageSourceIsEmpty(b))
> return true;
>
> +/* for disk type=volume we must check just pool/volume names as they
might
> + * not yet be resolved if e.g. we are comparing against the persistent
def */
> +if (a->type == VIR_STORAGE_TYPE_VOLUME && b->type ==
VIR_STORAGE_TYPE_VOLUME) {
> +return STREQ(a->srcpool->pool, b->srcpool->pool) &&
> + STREQ(a->srcpool->volume, b->srcpool->volume);
> +}
> +
Does this hold true even for disk sources that differ in the `mode`
attribute? I'm just asking for clarity. If that was taken into
consideration and the difference does not matter here, then this is
The idea is that the function should return true if both are pointing to
the same backing, in those terms the 'mode' is irrelevant as it's still
the same image.
This code is used in blockjob finalization, where it doesn't matter at
all and in cdrom image changing code, where it very theoretically could
be used to change the mode, but it's questionable whether that would
even make sense in any real usage.
OK, I had to ask because I read the documentation around 10 times and
wasn't sure whether it can change where the source points to, but it
looks like it does no, just changes how to represent the path to the
device. Sorry for the noise.
signature.asc
Description: PGP signature
Re: [PATCH v3 12/12] tests: Test detach-device and detach-device-alias for test driver
On Fri, 2021-11-26 at 16:44 +0100, Martin Kletzander wrote:
> On Wed, Nov 10, 2021 at 10:24:31PM +0800, Luke Yue wrote:
> > Signed-off-by: Luke Yue
> > ---
> > tests/virshtest.c | 96
> > +++
> > 1 file changed, 96 insertions(+)
> >
>
> This makes the tests very fragile. It would be even easier to do the
> test using the internal APIs, similarly to how we do it with the
> qemuhotplugtest.
>
Thanks for the explanation, I will look into how qemuhotplugtest is
implemented.
> > diff --git a/tests/virshtest.c b/tests/virshtest.c
> > index af2a70f5fb..8e5b397420 100644
> > --- a/tests/virshtest.c
> > +++ b/tests/virshtest.c
> > @@ -160,6 +160,8 @@ static char *custom_uri;
> > "--connect", \
> > custom_uri
> >
> > +# define TEST_XML_PATH abs_top_builddir "/../examples/xml/test"
> > +
> > static int testCompareListDefault(const void *data G_GNUC_UNUSED)
> > {
> > const char *const argv[] = { VIRSH_DEFAULT, "list", NULL };
> > @@ -437,6 +439,88 @@ static int testIOThreadPin(const void *data
> > G_GNUC_UNUSED)
> > return testCompareOutputLit(exp, "", NULL, argv);
> > }
> >
> > +static int testCompareDetachDevice(const void *data G_GNUC_UNUSED)
> > +{
> > + const char *const argv[] = { VIRSH_CUSTOM, "detach-device fc5\
> > + " TEST_XML_PATH "/testdevif.xml;\
> > + detach-device fc5\
> > + " TEST_XML_PATH
> > "/testdevdiskcdrom.xml;\
> > + detach-device fc5\
> > + " TEST_XML_PATH
> > "/testdevsound.xml;\
> > + detach-device fc5\
> > + " TEST_XML_PATH
> > "/testdevhostdev.xml;\
> > + detach-device fc5\
> > + " TEST_XML_PATH
> > "/testdevlease.xml;\
> > + detach-device fc5\
> > + " TEST_XML_PATH
> > "/testdevcontroller.xml;\
> > + detach-device fc5\
> > + " TEST_XML_PATH "/testdevfs.xml;\
> > + detach-device fc5\
> > + " TEST_XML_PATH
> > "/testdevrng.xml;\
> > + detach-device fc5\
> > + " TEST_XML_PATH
> > "/testdevmem.xml;\
> > + detach-device fc5\
> > + " TEST_XML_PATH
> > "/testdevshmem.xml;\
> > + detach-device fc5\
> > + " TEST_XML_PATH
> > "/testdevwatchdog.xml;\
> > + detach-device fc5\
> > + " TEST_XML_PATH
> > "/testdevinput.xml;\
> > + detach-device fc5\
> > + " TEST_XML_PATH
> > "/testdevvsock.xml;\
> > + detach-device fc5\
> > + " TEST_XML_PATH
> > "/testdevtpm.xml",
> > + NULL };
> > + const char *exp =
> > +"Device detached successfully\n\n\
> > +Device detached successfully\n\n\
> > +Device detached successfully\n\n\
> > +Device detached successfully\n\n\
> > +Device detached successfully\n\n\
> > +Device detached successfully\n\n\
> > +Device detached successfully\n\n\
> > +Device detached successfully\n\n\
> > +Device detached successfully\n\n\
> > +Device detached successfully\n\n\
> > +Device detached successfully\n\n\
> > +Device detached successfully\n\n\
> > +Device detached successfully\n\n\
> > +Device detached successfully\n\n";
>
> If any of these fails it will be a pain to figure out which one it
> was
> if the error message does not include the name. Splitting these is
> definitely the way to go.
>
> > + return testCompareOutputLit(exp, "", NULL, argv);
> > +}
> > +
> > +static int testCompareDetachDeviceError(const void *data
> > G_GNUC_UNUSED)
> > +{
> > + const char *const argv[] = { VIRSH_CUSTOM, "detach-device fc5\
> > + " TEST_XML_PATH
> > "/testdevtpm.xml;\
> > + detach-device fc5\
> > + " TEST_XML_PATH
> > "/testdevtpm.xml;\
> > + detach-device fc5 --live\
> > + " TEST_XML_PATH
> > "/testdevmemballoon.xml",
> > + NULL };
> > + const char *exp =
> > +"Device detached successfully\n\n\n\n";
> > + const char *error_msg =
> > +"error: Failed to detach device from " TEST_XML_PATH
> > "/testdevtpm.xml\n\
> > +error: device not found: matching tpm device not found\n\
> > +error: Failed to detach device from " TEST_XML_PATH
> > "/testdevmemballoon.xml\n\
> > +error: Operation not supported: detach of device 'memballoon' on
> > running do
Re: [PATCH v3 02/12] domain_driver: extract DetachXXXDeviceConfig related functions and use them
On Mon, Nov 29, 2021 at 09:06:57PM +0800, Luke Yue wrote:
On Fri, 2021-11-26 at 16:43 +0100, Martin Kletzander wrote:
On Wed, Nov 10, 2021 at 10:24:21PM +0800, Luke Yue wrote:
> libxl / lxc / qemu drivers share some common codes in their
> DomainDetachDeviceConfig functions, so extract them to
> domain_driver and
> reuse them.
>
I would argue that these specific functions are actually dealing just
with the domain definition which is done mostly in domain_conf.c, so
I
would pick that place.
Thanks for the review! No problem, I would move the code to
domain_conf.c in next version.
> At the same time, this will enable test driver to test these
> functions
> with virshtest in the future.
>
> Signed-off-by: Luke Yue
> ---
> Not pretty sure whether this is a proper way to make these
> functions
> reusable, maybe there is a more elegant choice.
The patch does a good job in deduplicating some of the code and
except
the file placement I think the overall approach is fine.
However unfortunately the functions are just copy-paste from various
places and they could be cleaned up to look the same. For example...
> ---
> src/hypervisor/domain_driver.c | 266
> +
> src/hypervisor/domain_driver.h | 41 +
> src/libvirt_private.syms | 14 ++
> src/libxl/libxl_driver.c | 41 +
> src/lxc/lxc_driver.c | 37 +
> src/qemu/qemu_driver.c | 123 +++
> 6 files changed, 356 insertions(+), 166 deletions(-)
>
> diff --git a/src/hypervisor/domain_driver.c
> b/src/hypervisor/domain_driver.c
> index 31737b0f4a..01ecb4e30e 100644
> --- a/src/hypervisor/domain_driver.c
> +++ b/src/hypervisor/domain_driver.c
> @@ -644,3 +644,269 @@
> virDomainDriverGetIOThreadsConfig(virDomainDef *targetDef,
>
> return ret;
> }
> +
> +
> +int
> +virDomainDriverDetachDiskDeviceConfig(virDomainDef *vmdef,
> + virDomainDeviceDef *dev)
> +{
> + virDomainDiskDef *disk;
> + virDomainDiskDef *det_disk;
> +
> + disk = dev->data.disk;
> + if (!(det_disk = virDomainDiskRemoveByName(vmdef, disk->dst)))
> {
> + virReportError(VIR_ERR_DEVICE_MISSING, _("no target device
> %s"),
> + disk->dst);
> + return -1;
> + }
> + virDomainDiskDefFree(det_disk);
> +
> + return 0;
This function uses temporary variables for everything and adds an
error
message (which could be moved to the virDomainDiskRemoveByName() as
all
callers do report the same error message when it is not found. Other
functions do essentially the same job, but have random comments, some
have extra helper variables. All that could be nicely unified and it
would read so much nicer.
What I really like about this change is that apart from roughly two
device types, like video and chardev, this function could be
de-duplicated as well. Adding a new functionality to that de-
duplicated
function would then add that functionality into all the drivers using
this function. Of course the special devices would need some xmlopt
callbacks or something that drivers can modify themselves for such
reasons, but such change would be really, really helpful, especially
in
the long term.
So if I understand you correctly, I will unified all these functions
into one,
Not really into one, although if you figure out a way to make that
easily possible feel free to go on, what I meant make them look and read
the same.
just like the original ones for QEMU / lxc / libxl, but this
one is for all, and maybe I should create a new enum for flags of
detachable device that different hypervisor support, and xmlopt etc for
special devices. I will do that in next version.
Deduplicating the bigger function would be nice to have for the future,
it does not need to happen together in this one. It will require some
more work to make it "modular" enough to be used by various drivers.
signature.asc
Description: PGP signature
Re: [PATCH v3 08/12] conf: Add a memballoon helper for future use
On Fri, 2021-11-26 at 16:44 +0100, Martin Kletzander wrote: > On Wed, Nov 10, 2021 at 10:24:27PM +0800, Luke Yue wrote: > > Currently it will only be used in test driver. > > > > Signed-off-by: Luke Yue > > --- > > src/conf/domain_conf.c | 24 > > src/conf/domain_conf.h | 4 > > src/libvirt_private.syms | 1 + > > 3 files changed, 29 insertions(+) > > > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > > index 512bfab9e9..92a8bd63f3 100644 > > --- a/src/conf/domain_conf.c > > +++ b/src/conf/domain_conf.c > > @@ -16998,6 +16998,30 @@ virDomainTPMDefRemove(virDomainDef *def, > > } > > > > > > +bool > > +virDomainMemballoonDefEquals(const virDomainMemballoonDef *a, > > + const virDomainMemballoonDef *b) > > It is really weird that you picked the membaloon as that particular > one > does not really make sense to be present multiple times, it is > similar > to the watchdog device. It is not a problem in this case, but I feel > like this function is a bit useless. Oh, sorry, I didn't realize that, I forgot that the device would just present once, so that we can just remove the device without comparison, I will remove the comparison function. Thanks, Luke
Re: [PATCH v3 06/12] conf: Add tpm helpers for future use
On Fri, 2021-11-26 at 16:43 +0100, Martin Kletzander wrote:
> On Wed, Nov 10, 2021 at 10:24:25PM +0800, Luke Yue wrote:
> > Currently it will only be used in the test driver.
> >
> > Signed-off-by: Luke Yue
> > ---
> > src/conf/domain_conf.c | 67
> >
> > src/conf/domain_conf.h | 6
> > src/libvirt_private.syms | 2 ++
> > 3 files changed, 75 insertions(+)
> >
> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > index 3193120b79..512bfab9e9 100644
> > --- a/src/conf/domain_conf.c
> > +++ b/src/conf/domain_conf.c
> > @@ -16931,6 +16931,73 @@ virDomainVsockDefEquals(const
> > virDomainVsockDef *a,
> > }
> >
> >
> > +static bool
> > +virDomainTPMDefEquals(const virDomainTPMDef *a,
> > + const virDomainTPMDef *b)
> > +{
> > + if (a->type != b->type)
> > + return false;
> > +
> > + if (a->model != b->model)
> > + return false;
> > +
> > + if (a->version != b->version)
> > + return false;
> > +
> > + if (a->type == VIR_DOMAIN_TPM_TYPE_PASSTHROUGH) {
> > + if (STRNEQ_NULLABLE(a-
> > >data.passthrough.source.data.file.path,
> > + b-
> > >data.passthrough.source.data.file.path))
>
> This will not compile as `source` is a pointer here.
>
I just noticed in commit 42b, the `source` was changed to be a
pointer, will fix in next version.
> > + return false;
> > + } else {
> > + if (a->data.emulator.hassecretuuid != b-
> > >data.emulator.hassecretuuid)
> > + return false;
> > +
> > + if (a->data.emulator.hassecretuuid == true &&
> > + memcmp(a->data.emulator.secretuuid,
> > + b->data.emulator.secretuuid,
> > + VIR_UUID_BUFLEN))
> > + return false;
> > +
> > + if (a->data.emulator.persistent_state !=
> > + b->data.emulator.persistent_state)
>
> Also it would make sense to try to detach a device without all the
> information, e.g. those that libvirt itself adds by default if they
> are
> missing, or in this case when persistent_state is set to the
> specified
> default (in formatdomain.html this is said to be "no" currently, but
> not
> filled in). Luckily the test driver itself does not do any of this,
> so
> I guess it's fine here.
Hmm, I will look into how libvirt adds these default devices and refine
this function in next version or in the future.
Thanks,
Luke
Re: [PATCH 2/2] virStorageSourceIsSameLocation: Special-case storage sources of type 'volume'
On Fri, Nov 26, 2021 at 14:50:37 +0100, Martin Kletzander wrote:
> On Fri, Nov 26, 2021 at 02:07:54PM +0100, Peter Krempa wrote:
> > The function is used also to compare virStorageSource which may not be
> > resolved to the image at that point in which case the 'path' is not yet
> > populated and the actual type is not yet set. This means that the
> > function fails to consider two identical volume-based disks as pointing
> > to the same thing.
> >
> > Add a special case for both images being type=volume in which case we
> > compare only the pool/volume names.
> >
> > Closes: https://gitlab.com/libvirt/libvirt/-/issues/240
> > Signed-off-by: Peter Krempa
> > ---
> > src/conf/storage_source_conf.c | 7 +++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/src/conf/storage_source_conf.c b/src/conf/storage_source_conf.c
> > index 44944e1dbd..c0acee189a 100644
> > --- a/src/conf/storage_source_conf.c
> > +++ b/src/conf/storage_source_conf.c
> > @@ -920,6 +920,13 @@ virStorageSourceIsSameLocation(virStorageSource *a,
> > virStorageSourceIsEmpty(b))
> > return true;
> >
> > +/* for disk type=volume we must check just pool/volume names as they
> > might
> > + * not yet be resolved if e.g. we are comparing against the persistent
> > def */
> > +if (a->type == VIR_STORAGE_TYPE_VOLUME && b->type ==
> > VIR_STORAGE_TYPE_VOLUME) {
> > +return STREQ(a->srcpool->pool, b->srcpool->pool) &&
> > + STREQ(a->srcpool->volume, b->srcpool->volume);
> > +}
> > +
>
> Does this hold true even for disk sources that differ in the `mode`
> attribute? I'm just asking for clarity. If that was taken into
> consideration and the difference does not matter here, then this is
The idea is that the function should return true if both are pointing to
the same backing, in those terms the 'mode' is irrelevant as it's still
the same image.
This code is used in blockjob finalization, where it doesn't matter at
all and in cdrom image changing code, where it very theoretically could
be used to change the mode, but it's questionable whether that would
even make sense in any real usage.
Re: [PATCH v3 03/12] test_driver: Implement virDomainDetachDeviceFlags
On Fri, 2021-11-26 at 16:43 +0100, Martin Kletzander wrote:
> On Wed, Nov 10, 2021 at 10:24:22PM +0800, Luke Yue wrote:
> > Introduce testDomainChgDevice for further development (just like
> > what we
> > did for IOThread). And introduce
> > testDomainDetachDeviceLiveAndConfig for
> > detaching devices.
> >
> > Signed-off-by: Luke Yue
> > ---
> > src/test/test_driver.c | 202
> > +
> > 1 file changed, 202 insertions(+)
> >
> > diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> > index ea474d55ac..6a7eb12f77 100644
> > --- a/src/test/test_driver.c
> > +++ b/src/test/test_driver.c
> > @@ -10051,6 +10051,207 @@
> > testConnectGetAllDomainStats(virConnectPtr conn,
> > return ret;
> > }
> >
> > +static int
> > +testDomainDetachDeviceLiveAndConfig(virDomainDef *vmdef,
> > + virDomainDeviceDef *dev)
>
> My comment from the previous patch would be nicely usable here as we
> could easily just make use of it.
>
> Also I see no reason for splitting the next two patches from this
> one.
>
OK, I will squash them in next version.
> [...]
>
> > +
> > +static int
> > +testDomainChgDevice(virDomainPtr dom,
> > + virDomainDeviceAction action,
> > + const char *xml,
> > + const char *alias,
> > + unsigned int flags)
> > +{
> > + testDriver *driver = dom->conn->privateData;
> > + virDomainObj *vm = NULL;
> > + virDomainDef *def;
> > + virDomainDeviceDef *dev = NULL;
> > + unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE;
> > + int ret = -1;
> > +
> > + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
> > + VIR_DOMAIN_AFFECT_CONFIG, -1);
>
> So here you check for both live and config, saying both of them are
> supported, ...
>
> > +
> > + if (!(vm = testDomObjFromDomain(dom)))
> > + goto cleanup;
> > +
> > + if (virDomainObjUpdateModificationImpact(vm, &flags) < 0)
> > + goto cleanup;
> > +
> > + if (!(def = virDomainObjGetOneDef(vm, flags)))
> > + goto cleanup;
> > +
>
> But here it would fail with both since you are explicitly saying you
> want just one definition.
>
> > + if (action == VIR_DOMAIN_DEVICE_ACTION_DETACH)
> > + parse_flags |= VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE;
> > +
> > + if (xml) {
> > + if (!(dev = virDomainDeviceDefParse(xml, def, driver-
> > >xmlopt,
> > + driver->caps,
> > parse_flags)))
> > + goto cleanup;
> > + } else if (alias) {
> > + dev = g_new0(virDomainDeviceDef, 1);
> > + if (virDomainDefFindDevice(def, alias, dev, true) < 0)
> > + goto cleanup;
> > + }
> > +
> > + if (dev == NULL)
> > + goto cleanup;
> > +
> > + switch (action) {
> > + case VIR_DOMAIN_DEVICE_ACTION_ATTACH:
> > + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> > + _("attaching devices is not supported"));
> > + goto cleanup;
> > + break;
> > +
> > + case VIR_DOMAIN_DEVICE_ACTION_DETACH:
> > + if (testDomainDetachDeviceLiveAndConfig(def, dev) < 0)
>
> And I kind of see now why you call the function "LiveAndConfig" since
> it
> can do both based on what DomainDef you call it with. This function
> could be very easily modified to do both live and config properly.
Sorry, I missed that situation, will fix it in next version.
Thanks,
Luke
Re: [PATCH v3 02/12] domain_driver: extract DetachXXXDeviceConfig related functions and use them
On Fri, 2021-11-26 at 16:43 +0100, Martin Kletzander wrote:
> On Wed, Nov 10, 2021 at 10:24:21PM +0800, Luke Yue wrote:
> > libxl / lxc / qemu drivers share some common codes in their
> > DomainDetachDeviceConfig functions, so extract them to
> > domain_driver and
> > reuse them.
> >
>
> I would argue that these specific functions are actually dealing just
> with the domain definition which is done mostly in domain_conf.c, so
> I
> would pick that place.
>
Thanks for the review! No problem, I would move the code to
domain_conf.c in next version.
> > At the same time, this will enable test driver to test these
> > functions
> > with virshtest in the future.
> >
> > Signed-off-by: Luke Yue
> > ---
> > Not pretty sure whether this is a proper way to make these
> > functions
> > reusable, maybe there is a more elegant choice.
>
> The patch does a good job in deduplicating some of the code and
> except
> the file placement I think the overall approach is fine.
>
> However unfortunately the functions are just copy-paste from various
> places and they could be cleaned up to look the same. For example...
>
> > ---
> > src/hypervisor/domain_driver.c | 266
> > +
> > src/hypervisor/domain_driver.h | 41 +
> > src/libvirt_private.syms | 14 ++
> > src/libxl/libxl_driver.c | 41 +
> > src/lxc/lxc_driver.c | 37 +
> > src/qemu/qemu_driver.c | 123 +++
> > 6 files changed, 356 insertions(+), 166 deletions(-)
> >
> > diff --git a/src/hypervisor/domain_driver.c
> > b/src/hypervisor/domain_driver.c
> > index 31737b0f4a..01ecb4e30e 100644
> > --- a/src/hypervisor/domain_driver.c
> > +++ b/src/hypervisor/domain_driver.c
> > @@ -644,3 +644,269 @@
> > virDomainDriverGetIOThreadsConfig(virDomainDef *targetDef,
> >
> > return ret;
> > }
> > +
> > +
> > +int
> > +virDomainDriverDetachDiskDeviceConfig(virDomainDef *vmdef,
> > + virDomainDeviceDef *dev)
> > +{
> > + virDomainDiskDef *disk;
> > + virDomainDiskDef *det_disk;
> > +
> > + disk = dev->data.disk;
> > + if (!(det_disk = virDomainDiskRemoveByName(vmdef, disk->dst)))
> > {
> > + virReportError(VIR_ERR_DEVICE_MISSING, _("no target device
> > %s"),
> > + disk->dst);
> > + return -1;
> > + }
> > + virDomainDiskDefFree(det_disk);
> > +
> > + return 0;
>
> This function uses temporary variables for everything and adds an
> error
> message (which could be moved to the virDomainDiskRemoveByName() as
> all
> callers do report the same error message when it is not found. Other
> functions do essentially the same job, but have random comments, some
> have extra helper variables. All that could be nicely unified and it
> would read so much nicer.
>
> What I really like about this change is that apart from roughly two
> device types, like video and chardev, this function could be
> de-duplicated as well. Adding a new functionality to that de-
> duplicated
> function would then add that functionality into all the drivers using
> this function. Of course the special devices would need some xmlopt
> callbacks or something that drivers can modify themselves for such
> reasons, but such change would be really, really helpful, especially
> in
> the long term.
So if I understand you correctly, I will unified all these functions
into one, just like the original ones for QEMU / lxc / libxl, but this
one is for all, and maybe I should create a new enum for flags of
detachable device that different hypervisor support, and xmlopt etc for
special devices. I will do that in next version.
Thanks,
Luke
Re: [libvirt PATCH 0/9] WIP/RFC: add QEMU "-display dbus" support
Hi On Fri, Nov 5, 2021 at 2:51 PM wrote: > > From: Marc-André Lureau > > Hi, > > This series implements supports for the upcoming QEMU "-display dbus" support. > Development is still in progress, but I hope to land the QEMU support early in > 6.3 (last version posted: > https://patchew.org/QEMU/20211009210838.2219430-1-marcandre.lur...@redhat.com/). > > By default, libvirt will start a private VM bus (sharing and reusing the > existing "vmstate" code). A client will need the address of the bus and access > to it. Since D-Bus addresses are not URIs unfortunately, I am not sure yet > what > should "virsh domdisplay" return. > > The feature set should cover the needs to replace Spice as local client of > choice, > including 3daccel/dmabuf, audio, clipboard sharing, usb redirection, and > arbitrary > chardev/channels (for serial etc). > > The test Gtk4 client is also in progress, currently in development at > https://gitlab.com/marcandre.lureau/qemu-display/. A few dependencies, such as > zbus, require an upcoming release. virt-viewer & boxes will need a port to > Gtk4 > to make use of the shared widget. > > Comments welcome, as we can still adjust the QEMU side etc. No comments? thanks > > thanks > > Marc-André Lureau (9): > qemu: start the D-Bus daemon as needed > qemu: add chardev-vdagent capability check > qemu: add -display dbus capability check (to update to 6.3) > qemu: add -display dbus support > qemu: add audio type 'dbus' > qemu: add dbus clipboard sharing > qemu: add -chardev dbus > qemu: add usbredir type 'dbus' > docs: document type dbus > > docs/formatdomain.rst | 24 > docs/schemas/basictypes.rng | 7 ++ > docs/schemas/domaincommon.rng | 71 +++ > src/bhyve/bhyve_command.c | 1 + > src/conf/domain_conf.c| 116 +- > src/conf/domain_conf.h| 14 +++ > src/conf/domain_validate.c| 32 +++-- > src/libxl/libxl_conf.c| 1 + > src/qemu/qemu_capabilities.c | 6 + > src/qemu/qemu_capabilities.h | 2 + > src/qemu/qemu_command.c | 94 +- > src/qemu/qemu_domain.c| 1 + > src/qemu/qemu_domain.h| 1 + > src/qemu/qemu_driver.c| 10 +- > src/qemu/qemu_hotplug.c | 1 + > src/qemu/qemu_monitor_json.c | 1 + > src/qemu/qemu_process.c | 15 ++- > src/qemu/qemu_validate.c | 33 + > src/security/security_dac.c | 2 + > src/vmx/vmx.c | 1 + > .../domaincapsdata/qemu_6.2.0-q35.x86_64.xml | 1 + > .../domaincapsdata/qemu_6.2.0-tcg.x86_64.xml | 1 + > tests/domaincapsdata/qemu_6.2.0.x86_64.xml| 1 + > .../caps_6.1.0.x86_64.xml | 1 + > .../caps_6.2.0.aarch64.xml| 1 + > .../caps_6.2.0.x86_64.replies | 10 +- > .../caps_6.2.0.x86_64.xml | 2 + > .../graphics-dbus-address.args| 31 + > .../graphics-dbus-address.xml | 34 + > .../qemuxml2argvdata/graphics-dbus-audio.args | 32 + > .../qemuxml2argvdata/graphics-dbus-audio.xml | 40 ++ > .../graphics-dbus-chardev.args| 33 + > .../graphics-dbus-chardev.xml | 38 ++ > .../graphics-dbus-clipboard.args | 32 + > .../graphics-dbus-clipboard.xml | 36 ++ > tests/qemuxml2argvdata/graphics-dbus-p2p.args | 31 + > tests/qemuxml2argvdata/graphics-dbus-p2p.xml | 36 ++ > .../graphics-dbus-usbredir.args | 35 ++ > .../graphics-dbus-usbredir.xml| 36 ++ > tests/qemuxml2argvdata/graphics-dbus.args | 31 + > tests/qemuxml2argvdata/graphics-dbus.xml | 34 + > tests/qemuxml2argvtest.c | 21 > .../graphics-dbus-address.xml | 39 ++ > .../graphics-dbus-audio.xml | 43 +++ > .../qemuxml2xmloutdata/graphics-dbus-p2p.xml | 39 ++ > tests/qemuxml2xmloutdata/graphics-dbus.xml| 39 ++ > tests/qemuxml2xmltest.c | 14 +++ > 47 files changed, 1106 insertions(+), 18 deletions(-) > create mode 100644 tests/qemuxml2argvdata/graphics-dbus-address.args > create mode 100644 tests/qemuxml2argvdata/graphics-dbus-address.xml > create mode 100644 tests/qemuxml2argvdata/graphics-dbus-audio.args > create mode 100644 tests/qemuxml2argvdata/graphics-dbus-audio.xml > create mode 100644 tests/qemuxml2argvdata/graphics-dbus-chardev.args > create mode 100644 tests/qemuxml2argvdata/graphics-dbus-chardev.xml > create mode 100644 tests/qemuxml2argvdata/graphics-dbus-clipboard.args > create mode
Re: [PATCH 0/2] wireshark: Fix build with newer wireshark
On Mon, Nov 29, 2021 at 11:35:48 +0100, Michal Privoznik wrote: > I've started seeing a build failure on rawhide because of new wireshark > (3.6.0). The problem is that wmem_alloc() function which we use was > removed from public symbols starting with the 3.6.0 release in favor of > wmem_new(). However, as I was looking at the code I realized that > wireshark already has a function for what our code does and decided to > drop our implementation in favor of wireshark's. > > Michal Prívozník (2): > wireshark: Switch to tvb_bytes_to_str() > wireshark: Drop needless comment in dissect_xdr_bytes() > > tools/wireshark/src/packet-libvirt.c | 32 +++- > 1 file changed, 8 insertions(+), 24 deletions(-) > Reviewed-by: Jiri Denemark
[PATCH 1/2] wireshark: Switch to tvb_bytes_to_str()
When the dissector sees a byte sequence that is either an opaque
data (xdr_opaque) or a byte sequence (xdr_bytes) it formats the
bytes as a hex numbers using our own implementation. But
wireshark already provides a function for it: tvb_bytes_to_str().
NB, the reason why it returns a const string is so that callers
don't try to free it - the string is allocated using an allocator
which will decide when to free it.
The wireshark formatter was introduced in wireshark commit of
v1.99.2~479 and thus is present in the version we require at
least (2.6.0).
Signed-off-by: Michal Privoznik
---
tools/wireshark/src/packet-libvirt.c | 30
1 file changed, 8 insertions(+), 22 deletions(-)
diff --git a/tools/wireshark/src/packet-libvirt.c
b/tools/wireshark/src/packet-libvirt.c
index f43919b05d..cb922b8070 100644
--- a/tools/wireshark/src/packet-libvirt.c
+++ b/tools/wireshark/src/packet-libvirt.c
@@ -158,24 +158,6 @@ dissect_xdr_string(tvbuff_t *tvb, proto_tree *tree, XDR
*xdrs, int hf,
}
}
-static const gchar *
-format_xdr_bytes(guint8 *bytes, guint32 length)
-{
-gchar *buf;
-guint32 i;
-
-if (length == 0)
-return "";
-buf = wmem_alloc(wmem_packet_scope(), length*2 + 1);
-for (i = 0; i < length; i++) {
-/* We know that buf has enough size to contain
- 2 * length + '\0' characters. */
-g_snprintf(buf, 2*(length - i) + 1, "%02x", bytes[i]);
-buf += 2;
-}
-return buf - length*2;
-}
-
static gboolean
dissect_xdr_opaque(tvbuff_t *tvb, proto_tree *tree, XDR *xdrs, int hf,
guint32 size)
@@ -187,8 +169,10 @@ dissect_xdr_opaque(tvbuff_t *tvb, proto_tree *tree, XDR
*xdrs, int hf,
val = g_malloc(size);
start = xdr_getpos(xdrs);
if ((rc = xdr_opaque(xdrs, (caddr_t)val, size))) {
-proto_tree_add_bytes_format_value(tree, hf, tvb, start,
xdr_getpos(xdrs) - start,
- NULL, "%s", format_xdr_bytes(val,
size));
+gint len = xdr_getpos(xdrs) - start;
+const char *s = tvb_bytes_to_str(wmem_packet_scope(), tvb, start, len);
+
+proto_tree_add_bytes_format_value(tree, hf, tvb, start, len, NULL,
"%s", s);
} else {
proto_tree_add_item(tree, hf_libvirt_unknown, tvb, start, -1, ENC_NA);
}
@@ -207,8 +191,10 @@ dissect_xdr_bytes(tvbuff_t *tvb, proto_tree *tree, XDR
*xdrs, int hf,
start = xdr_getpos(xdrs);
if (xdr_bytes(xdrs, (char **)&val, &length, maxlen)) {
-proto_tree_add_bytes_format_value(tree, hf, tvb, start,
xdr_getpos(xdrs) - start,
- NULL, "%s", format_xdr_bytes(val,
length));
+gint len = xdr_getpos(xdrs) - start;
+const char *s = tvb_bytes_to_str(wmem_packet_scope(), tvb, start, len);
+
+proto_tree_add_bytes_format_value(tree, hf, tvb, start, len, NULL,
"%s", s);
/* Seems I can't call xdr_free() for this case.
It will raises SEGV by referencing out of bounds call stack */
free(val);
--
2.32.0
[PATCH 2/2] wireshark: Drop needless comment in dissect_xdr_bytes()
In the dissect_xdr_bytes() there's a comment that the string
allocated by xdr_bytes() can't be freed using xdr_free(). Well,
that is expected because xdr_bytes() used plain calloc() AND the
string is not an XDR struct but plain 'char *' type. Passing it
to xdr_free() must result in weird things happening.
Signed-off-by: Michal Privoznik
---
tools/wireshark/src/packet-libvirt.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/tools/wireshark/src/packet-libvirt.c
b/tools/wireshark/src/packet-libvirt.c
index cb922b8070..eeacbcdf0e 100644
--- a/tools/wireshark/src/packet-libvirt.c
+++ b/tools/wireshark/src/packet-libvirt.c
@@ -195,8 +195,6 @@ dissect_xdr_bytes(tvbuff_t *tvb, proto_tree *tree, XDR
*xdrs, int hf,
const char *s = tvb_bytes_to_str(wmem_packet_scope(), tvb, start, len);
proto_tree_add_bytes_format_value(tree, hf, tvb, start, len, NULL,
"%s", s);
-/* Seems I can't call xdr_free() for this case.
- It will raises SEGV by referencing out of bounds call stack */
free(val);
return TRUE;
} else {
--
2.32.0
[PATCH 0/2] wireshark: Fix build with newer wireshark
I've started seeing a build failure on rawhide because of new wireshark (3.6.0). The problem is that wmem_alloc() function which we use was removed from public symbols starting with the 3.6.0 release in favor of wmem_new(). However, as I was looking at the code I realized that wireshark already has a function for what our code does and decided to drop our implementation in favor of wireshark's. Michal Prívozník (2): wireshark: Switch to tvb_bytes_to_str() wireshark: Drop needless comment in dissect_xdr_bytes() tools/wireshark/src/packet-libvirt.c | 32 +++- 1 file changed, 8 insertions(+), 24 deletions(-) -- 2.32.0
libvirt-7.10.0 release candidate 2
I have just tagged v7.10.0-rc2 in the repository and pushed signed tarballs and source RPMs to https://libvirt.org/sources/ Please give the release candidate some testing and in case you find a serious issue which should have a fix in the upcoming release, feel free to reply to this thread to make sure the issue is more visible. If you have not done so yet, please update NEWS.rst to document any significant change you made since the last release. Thanks, Jirka
