Re: [libvirt] [PATCH 1/4] qemu: Escape commas for qemuBuildChrChardevStr
On 06/19/2018 10:19 AM, Anya Harter wrote: > > > On 06/18/2018 07:37 PM, John Ferlan wrote: >> >> >> On 06/18/2018 01:57 PM, Anya Harter wrote: >>> Add comma escaping for dev->data.file.path in cases >>> VIR_DOMAIN_CHR_TYPE_DEV and VIR_DOMAIN_CHR_TYPE_PIPE. >>> >>> Signed-off-by: Anya Harter >>> --- >>> src/qemu/qemu_command.c | 9 + >>> tests/qemuxml2argvdata/name-escape.args | 4 >>> tests/qemuxml2argvdata/name-escape.xml | 7 +++ >>> tests/qemuxml2argvtest.c| 3 ++- >>> 4 files changed, 18 insertions(+), 5 deletions(-) >>> >> >> Having tests is awesome! Thanks! >> >> Not sure why the bite size tasks omitted VIR_DOMAIN_CHR_TYPE_FILE too. >> If you want to investigate the FILE case that'd be good - just to make >> sure we aren't missing any! I'll still push this as is since it's fine, >> but if the FILE needs something it can be added afterwards. > > VIR_DOMAIN_CHR_TYPE_FILE calls the function qemuBuildChrChardevFileStr > and passes the dev->data.file.path into the parameter named fileval > which I escape in the second patch. > > Please let me know if I am missing something here. > Nope I didn't dig far enough... John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/5] qemu: Promote start_flags in qemuDomainRevertToSnapshot
Promote the @start_flags to the top of the function, a
subsequent patch needs to use it.
Signed-off-by: John Ferlan
---
src/qemu/qemu_driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 3abbe41895..971f485226 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15990,6 +15990,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr
snapshot,
bool was_stopped = false;
qemuDomainSaveCookiePtr cookie;
virCPUDefPtr origCPU = NULL;
+unsigned int start_flags = VIR_QEMU_PROCESS_START_GEN_VMID;
virCheckFlags(VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING |
VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED |
@@ -16307,7 +16308,6 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr
snapshot,
VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED)) {
/* Flush first event, now do transition 2 or 3 */
bool paused = (flags & VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED) != 0;
-unsigned int start_flags = VIR_QEMU_PROCESS_START_GEN_VMID;
start_flags |= paused ? VIR_QEMU_PROCESS_START_PAUSED : 0;
--
2.14.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/5] qemu: Don't use asyncJob after stop during snapshot revert
Attempting to use the FORCE flag for snapshot-revert was resulting in failures because qemuProcessStart and qemuProcessStartCPUs were using QEMU_ASYNC_JOB_START after a qemuProcessStop resulting in an error when entering the monitor: error: internal error: unexpected async job 6 type expected 0 So create a local @jobType, initialize to QEMU_ASYNC_JOB_START, and change to QEMU_ASYNC_JOB_NONE if we end up in the --force path where the qemuProcessStop is run before a Start and StartCPUs. https://bugzilla.redhat.com/show_bug.cgi?id=1591628 Signed-off-by: John Ferlan --- src/qemu/qemu_driver.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f737f4d350..71d2bb357c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15991,6 +15991,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, qemuDomainSaveCookiePtr cookie; virCPUDefPtr origCPU = NULL; unsigned int start_flags = VIR_QEMU_PROCESS_START_GEN_VMID; +qemuDomainAsyncJob jobType = QEMU_ASYNC_JOB_START; virCheckFlags(VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING | VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED | @@ -16166,6 +16167,9 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, VIR_DOMAIN_EVENT_STOPPED, detail); virObjectEventStateQueue(driver->domainEventState, event); +/* Start after stop won't be an async start job, so + * reset to none */ +jobType = QEMU_ASYNC_JOB_NONE; goto load; } } @@ -16224,7 +16228,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, rc = qemuProcessStart(snapshot->domain->conn, driver, vm, cookie ? cookie->cpu : NULL, - QEMU_ASYNC_JOB_START, NULL, -1, NULL, snap, + jobType, NULL, -1, NULL, snap, VIR_NETDEV_VPORT_PROFILE_OP_CREATE, start_flags); virDomainAuditStart(vm, "from-snapshot", rc >= 0); @@ -16259,7 +16263,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, } rc = qemuProcessStartCPUs(driver, vm, VIR_DOMAIN_RUNNING_FROM_SNAPSHOT, - QEMU_ASYNC_JOB_START); + jobType); if (rc < 0) goto endjob; virObjectUnref(event); -- 2.14.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/5] qemu: Unset the genid start change flag for revert force
If the the snapshot revert involves a forced revert option, then
let's not cause startup to change the genid flag in order to signify
that we're still running the same/previous guest and not some
snapshot reversion.
https://bugzilla.redhat.com/show_bug.cgi?id=1149445
Signed-off-by: John Ferlan
---
src/qemu/qemu_driver.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 01011906d1..f737f4d350 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16136,12 +16136,14 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr
snapshot,
}
/* If using VM GenID, there is no way currently to change
- * the genid for the running guest, so set an error and
- * mark as incompatible. */
+ * the genid for the running guest, so set an error,
+ * mark as incompatible, and don't allow change of genid
+ * if the revert force flag would start the guest again. */
if (compatible && config->genidRequested) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("domain genid update requires restart"));
compatible = false;
+start_flags &= ~VIR_QEMU_PROCESS_START_GEN_VMID;
}
if (!compatible) {
--
2.14.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/5] qemu: Use start_flags for RUNNING and PAUSED transitions
Use and set the @start_flags at the top of the RUNNING and PAUSED
transitions to GEN_VMID | PAUSED.
Signed-off-by: John Ferlan
---
src/qemu/qemu_driver.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 971f485226..01011906d1 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16102,6 +16102,9 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr
snapshot,
switch ((virDomainState) snap->def->state) {
case VIR_DOMAIN_RUNNING:
case VIR_DOMAIN_PAUSED:
+
+start_flags |= VIR_QEMU_PROCESS_START_PAUSED;
+
/* Transitions 2, 3, 5, 6, 8, 9 */
/* When using the loadvm monitor command, qemu does not know
* whether to pause or run the reverted domain, and just stays
@@ -16221,8 +16224,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr
snapshot,
cookie ? cookie->cpu : NULL,
QEMU_ASYNC_JOB_START, NULL, -1, NULL, snap,
VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
- VIR_QEMU_PROCESS_START_PAUSED |
- VIR_QEMU_PROCESS_START_GEN_VMID);
+ start_flags);
virDomainAuditStart(vm, "from-snapshot", rc >= 0);
detail = VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT;
event = virDomainEventLifecycleNewFromObj(vm,
--
2.14.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/5] qemu: Adjust async job failure message
Make it clearer what asyncJob type was passed and what was expected.
Signed-off-by: John Ferlan
---
src/qemu/qemu_domain.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 8a2d4750a5..939d64542c 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -6581,7 +6581,8 @@ qemuDomainObjBeginNestedJob(virQEMUDriverPtr driver,
if (asyncJob != priv->job.asyncJob) {
virReportError(VIR_ERR_INTERNAL_ERROR,
- _("unexpected async job %d"), asyncJob);
+ _("unexpected async job %d type expected %d"),
+ asyncJob, priv->job.asyncJob);
return -1;
}
--
2.14.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/5] Fix a couple of force snapshot revert issues
Well, at least I hope so. There's a couple of related bz's in the last two patches with gobs more details, but the long and short of it is that when using the force flag, the domain is Stop'd and re-Start'd without applying any snapshot. For both issues I've seen, the "error: internal error: unexpected async job 6" occurs on the Start and thus the CPU's aren't started. For one bz, when running managedsave the call never returns. For the other bz, one cannot restart the paused domain. If there's a better way to do this - I'm sure someone will let me know. John Ferlan (5): qemu: Adjust async job failure message qemu: Promote start_flags in qemuDomainRevertToSnapshot qemu: Use start_flags for RUNNING and PAUSED transitions qemu: Unset the genid start change flag for revert force qemu: Don't use asyncJob after stop during snapshot revert src/qemu/qemu_domain.c | 3 ++- src/qemu/qemu_driver.c | 22 +++--- 2 files changed, 17 insertions(+), 8 deletions(-) -- 2.14.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v7 1/3] qmp: adding 'wakeup-suspend-support' in query-target
Hi,
Sorry for the delay. I'll summarize what I've understood from the discussion
so far:
- query-target is the wrong place for this flag. query-machines is
(less) wrong
because it is not a static property of the machine object
- a new "query-current-machine" can be created to host these dynamic
properties that belongs to the current instance of the VM
- there are machines in which the suspend support may vary with a
"-no-acpi" option that would disable both the suspend and wake-up
support. In this case, I see no problem into counting this flag into
the logic (assuming it is possible, of course) and setting it as "false"
if there is -no-acpi present (or even making the API returning "yes",
"no" or "acpi" like Markus suggested) somewhere.
Based on the last email from Eduardo, apparently there is a handful
of other machine properties that can be hosted in either this new
query-current-machine API or query-machines. I believe that this is
more of a long term goal, but this new query-current-machine API
would be a good kick-off and we should go for it.
Is this a fair understanding? Did I miss something?
Thanks,
Daniel
On 05/29/2018 11:55 AM, Eduardo Habkost wrote:
On Mon, May 28, 2018 at 09:23:54AM +0200, Markus Armbruster wrote:
Eduardo Habkost writes:
[...]
[1] Doing a:
$ git grep 'STR.*machine, "'
on libvirt source is enough to find some code demonstrating where
query-machines is already lacking today:
[...]
How can we get from this grep to a list of static or dynamic machine
type capabilties?
Let's look at the code:
$ git grep -W 'STR.*machine, "'
src/libxl/libxl_capabilities.c=libxlMakeDomainOSCaps(const char *machine,
src/libxl/libxl_capabilities.c- virDomainCapsOSPtr os,
src/libxl/libxl_capabilities.c- virFirmwarePtr *firmwares,
src/libxl/libxl_capabilities.c- size_t nfirmwares)
src/libxl/libxl_capabilities.c-{
src/libxl/libxl_capabilities.c-virDomainCapsLoaderPtr capsLoader =
>loader;
src/libxl/libxl_capabilities.c-size_t i;
src/libxl/libxl_capabilities.c-
src/libxl/libxl_capabilities.c-os->supported = true;
src/libxl/libxl_capabilities.c-
src/libxl/libxl_capabilities.c:if (STREQ(machine, "xenpv"))
src/libxl/libxl_capabilities.c-return 0;
I don't understand why this one is here, but we can find out what
we could add to query-machines to make this unnecessary.
[...]
--
src/libxl/libxl_capabilities.c=libxlMakeDomainCapabilities(virDomainCapsPtr
domCaps,
src/libxl/libxl_capabilities.c-virFirmwarePtr
*firmwares,
src/libxl/libxl_capabilities.c-size_t nfirmwares)
src/libxl/libxl_capabilities.c-{
src/libxl/libxl_capabilities.c-virDomainCapsOSPtr os = >os;
src/libxl/libxl_capabilities.c-virDomainCapsDeviceDiskPtr disk =
>disk;
src/libxl/libxl_capabilities.c-virDomainCapsDeviceGraphicsPtr graphics =
>graphics;
src/libxl/libxl_capabilities.c-virDomainCapsDeviceVideoPtr video =
>video;
src/libxl/libxl_capabilities.c-virDomainCapsDeviceHostdevPtr hostdev =
>hostdev;
src/libxl/libxl_capabilities.c-
src/libxl/libxl_capabilities.c:if (STREQ(domCaps->machine, "xenfv"))
src/libxl/libxl_capabilities.c-domCaps->maxvcpus = HVM_MAX_VCPUS;
src/libxl/libxl_capabilities.c-else
src/libxl/libxl_capabilities.c-domCaps->maxvcpus = PV_MAX_VCPUS;
Looks like libvirt isn't using MachineInfo::cpu-max. libvirt
bug, or workaround for QEMU limitation?
[...]
--
src/libxl/libxl_driver.c=libxlConnectGetDomainCapabilities(virConnectPtr conn,
src/libxl/libxl_driver.c- const char
*emulatorbin,
src/libxl/libxl_driver.c- const char *arch_str,
src/libxl/libxl_driver.c- const char *machine,
src/libxl/libxl_driver.c- const char
*virttype_str,
src/libxl/libxl_driver.c- unsigned int flags)
src/libxl/libxl_driver.c-{
[...]
src/libxl/libxl_driver.c-if (machine) {
src/libxl/libxl_driver.c:if (STRNEQ(machine, "xenpv") && STRNEQ(machine,
"xenfv")) {
src/libxl/libxl_driver.c-virReportError(VIR_ERR_INVALID_ARG, "%s",
src/libxl/libxl_driver.c- _("Xen only supports 'xenpv' and
'xenfv' machines"));
Not sure if this should be encoded in QEMU. accel=xen works with
other PC machines, doesn't it?
[...]
--
src/qemu/qemu_capabilities.c=bool virQEMUCapsHasPCIMultiBus(virQEMUCapsPtr
qemuCaps,
src/qemu/qemu_capabilities.c- const virDomainDef
*def)
src/qemu/qemu_capabilities.c-{
src/qemu/qemu_capabilities.c-/* x86_64 and i686 support PCI-multibus on all
machine types
src/qemu/qemu_capabilities.c- * since forever */
src/qemu/qemu_capabilities.c-if (ARCH_IS_X86(def->os.arch))
src/qemu/qemu_capabilities.c-return true;
src/qemu/qemu_capabilities.c-
Re: [libvirt] [PATCH] cpu: add 'amd-ssbd' and 'amd-no-ssb' CPU features (CVE-2018-3639)
On Thu, Jun 14, 2018 at 11:48:41 +0100, Daniel P. Berrangé wrote: > AMD x86 CPUs have two separate ways to mitigate the Speculative Store > Bypass hardware flaw. In current processors only non-architectural MSRs > are available, and so hypervisors must expose a virtualized MSR and CPU > flag "virt-ssbd" (CPUID Function 8000_0008, EBX[25]=1). > > In future processors AMD will provide an architectural MSR, indicated by > existance of the CPUID Function 8000_0008, EBX[24]=1, to which QEMU has > given the name "amd-ssbd". > > The "amd-ssbd" flag should be used in preference to "virt-ssbd", if it > is available, since it provides improved performance. For virtual > machine configuration, both should be exposed when available, to allow > for maximal guest OS compatibility as not all guests yet support both. > > If future processes are not vulnerable to the flaw, this will be > indicated by the existance of CPUID Function 8000_0008, EBX[26]=1, > to which QEMU has given the name "amd-no-ssb". > > See also 124441_AMD64_SpeculativeStoreBypassDisable_Whitepaper_final.pdf > from: > > https://bugzilla.kernel.org/show_bug.cgi?id=199889 > > Note that neither amd-ssbd or amd-no-ssb will be reported by the kernel > in /proc/cpuinfo. It knows about these CPUID bits and does the right thing, > but doesn't report their existance as distinct flags in /proc/cpuinfo. > > Signed-off-by: Daniel P. Berrangé Eduardo pushed the QEMU part into his x86-next queue, but he didn't send a pull request yet. I think it's a good idea to wait until the patch lands in QEMU master before pushing this patch. Reviewed-by: Jiri Denemark -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Fix domain resume after failed migration
* Peter Krempa (pkre...@redhat.com) wrote: > On Mon, Jun 04, 2018 at 16:51:18 +0200, Jiri Denemark wrote: > > Libvirt relies on being able to kill the destination domain and resume > > the source one during migration until we called "cont" on the > > destination. Unfortunately, QEMU automatically activates block devices > > at the end of migration even when it's called with -S. This wasn't a big > > issue in the past since the guest is not running and thus no data are > > written to the block devices. However, when QEMU introduced its internal > > block device locks, we can no longer resume the source domain once the > > destination domain already activated the block devices (and thus > > acquired all locks) unless the destination domain is killed first. > > > > Since it's impossible to synchronize the destination and the source > > libvirt daemons after a failed migration, QEMU introduced a new > > migration capability called "late-block-activat" which ensures QEMU > > won't activate block devices until it gets "cont". The only thing we > > need to do is to enable this capability whenever QEMU supports it. > > I'm wondering when this new feature should _not_ be used. I did not get > the information from the qemu commit message so I've cc'd David to shed > some light. > > If it's desired to always pass it then I'm failing to see why they've > added it in the first place. There was some worry that doing it by default would be a subtle API change; personally I didn't really see it as a problem, but since people were worried I made it switchable. See: https://lists.gnu.org/archive/html/qemu-devel/2018-04/msg01300.html Dave -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] conf: Remove unused virDomainDefNewFull
The last usages were removed with the xend driver in 1dac5f0
Signed-off-by: Cole Robinson
---
src/conf/domain_conf.c | 22 --
src/conf/domain_conf.h | 3 ---
src/libvirt_private.syms | 1 -
3 files changed, 26 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 522e0c2bf3..f0b604e125 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3241,28 +3241,6 @@ virDomainDefNew(void)
}
-virDomainDefPtr
-virDomainDefNewFull(const char *name,
-const unsigned char *uuid,
-int id)
-{
-virDomainDefPtr def;
-
-if (!(def = virDomainDefNew()))
-return NULL;
-
-if (VIR_STRDUP(def->name, name) < 0) {
-VIR_FREE(def);
-return NULL;
-}
-
-memcpy(def->uuid, uuid, VIR_UUID_BUFLEN);
-def->id = id;
-
-return def;
-}
-
-
void virDomainObjAssignDef(virDomainObjPtr domain,
virDomainDefPtr def,
bool live,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 6344c02d1c..8cefef535a 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2919,9 +2919,6 @@ virDomainChrSourceDefNew(virDomainXMLOptionPtr xmlopt);
virDomainChrDefPtr virDomainChrDefNew(virDomainXMLOptionPtr xmlopt);
virDomainDefPtr virDomainDefNew(void);
-virDomainDefPtr virDomainDefNewFull(const char *name,
-const unsigned char *uuid,
-int id);
void virDomainObjAssignDef(virDomainObjPtr domain,
virDomainDefPtr def,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index ea24f2847c..98913a577a 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -287,7 +287,6 @@ virDomainDefMaybeAddController;
virDomainDefMaybeAddInput;
virDomainDefNeedsPlacementAdvice;
virDomainDefNew;
-virDomainDefNewFull;
virDomainDefParseFile;
virDomainDefParseNode;
virDomainDefParseString;
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] conf: Remove unused virDomainDefNewFull
The last usages were removed with the xend driver in 1dac5f0
Signed-off-by: Cole Robinson
---
src/conf/domain_conf.c | 22 --
src/conf/domain_conf.h | 3 ---
src/libvirt_private.syms | 1 -
3 files changed, 26 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 522e0c2bf3..f0b604e125 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3241,28 +3241,6 @@ virDomainDefNew(void)
}
-virDomainDefPtr
-virDomainDefNewFull(const char *name,
-const unsigned char *uuid,
-int id)
-{
-virDomainDefPtr def;
-
-if (!(def = virDomainDefNew()))
-return NULL;
-
-if (VIR_STRDUP(def->name, name) < 0) {
-VIR_FREE(def);
-return NULL;
-}
-
-memcpy(def->uuid, uuid, VIR_UUID_BUFLEN);
-def->id = id;
-
-return def;
-}
-
-
void virDomainObjAssignDef(virDomainObjPtr domain,
virDomainDefPtr def,
bool live,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 6344c02d1c..8cefef535a 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2919,9 +2919,6 @@ virDomainChrSourceDefNew(virDomainXMLOptionPtr xmlopt);
virDomainChrDefPtr virDomainChrDefNew(virDomainXMLOptionPtr xmlopt);
virDomainDefPtr virDomainDefNew(void);
-virDomainDefPtr virDomainDefNewFull(const char *name,
-const unsigned char *uuid,
-int id);
void virDomainObjAssignDef(virDomainObjPtr domain,
virDomainDefPtr def,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index ea24f2847c..98913a577a 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -287,7 +287,6 @@ virDomainDefMaybeAddController;
virDomainDefMaybeAddInput;
virDomainDefNeedsPlacementAdvice;
virDomainDefNew;
-virDomainDefNewFull;
virDomainDefParseFile;
virDomainDefParseNode;
virDomainDefParseString;
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [v2 RESEND 3/3] qemu: Switch code to use new agent job APIs
On Tue, Jun 19, 2018 at 19:03:24 +0200, Michal Prívozník wrote: > On 06/19/2018 05:05 PM, Jiri Denemark wrote: > > On Tue, Jun 19, 2018 at 08:38:02 +0200, Michal Privoznik wrote: > >> There are two sets of functions here: > >> 1) some functions talk on both monitor and agent monitor, > >> 2) some functions only talk on agent monitor. > >> > >> For functions from set 1) we need to use > >> qemuDomainObjBeginJobWithAgent() and for functions from set 2) we > >> need to use qemuDomainObjBeginAgentJob() only. > >> > >> Signed-off-by: Michal Privoznik > >> --- > >> src/qemu/qemu_driver.c | 91 > >> -- > >> 1 file changed, 58 insertions(+), 33 deletions(-) > >> > >> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > >> index 3abbe41895..cffd4c928a 100644 > >> --- a/src/qemu/qemu_driver.c > >> +++ b/src/qemu/qemu_driver.c ... > >> @@ -4949,6 +4967,7 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, > >> virDomainDefPtr def; > >> virDomainDefPtr persistentDef; > >> bool hotpluggable = !!(flags & VIR_DOMAIN_VCPU_HOTPLUGGABLE); > >> +bool useAgent = !!(flags & VIR_DOMAIN_VCPU_GUEST); > >> int ret = -1; > >> > >> virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | > >> @@ -4963,13 +4982,14 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, > >> if (virDomainSetVcpusFlagsEnsureACL(dom->conn, vm->def, flags) < 0) > >> goto cleanup; > >> > >> -if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) > >> +if ((!useAgent && qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) > >> < 0) || > >> +(useAgent && qemuDomainObjBeginAgentJob(driver, vm, > >> QEMU_AGENT_JOB_MODIFY) < 0)) > >> goto cleanup; > > > > And here. > > Actually no. This one is different to the previous two places. This one > is either grab domain job OR agent job but not both at the same time > (which is what previous places do). Ah right, I misread qemuDomainObjBeginAgentJob as qemuDomainObjBeginJobWithAgent. In this case, I'd just modify the code a bit to make it clearer the two cases are mutually exclusive, e.g.: int rc; if (useAgent) rc = qemuDomainObjBeginAgentJob(); else rc = qemuDomainObjBeginJob(); if (rc < 0) goto cleanup; Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [v2 RESEND 3/3] qemu: Switch code to use new agent job APIs
On 06/19/2018 05:05 PM, Jiri Denemark wrote:
> On Tue, Jun 19, 2018 at 08:38:02 +0200, Michal Privoznik wrote:
>> There are two sets of functions here:
>> 1) some functions talk on both monitor and agent monitor,
>> 2) some functions only talk on agent monitor.
>>
>> For functions from set 1) we need to use
>> qemuDomainObjBeginJobWithAgent() and for functions from set 2) we
>> need to use qemuDomainObjBeginAgentJob() only.
>>
>> Signed-off-by: Michal Privoznik
>> ---
>> src/qemu/qemu_driver.c | 91
>> --
>> 1 file changed, 58 insertions(+), 33 deletions(-)
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index 3abbe41895..cffd4c928a 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -1954,6 +1954,7 @@ static int qemuDomainShutdownFlags(virDomainPtr dom,
>> unsigned int flags)
>> bool useAgent = false, agentRequested, acpiRequested;
>> bool isReboot = false;
>> bool agentForced;
>> +bool agentJob = false;
>> int agentFlag = QEMU_AGENT_SHUTDOWN_POWERDOWN;
>>
>> virCheckFlags(VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN |
>> @@ -1980,9 +1981,14 @@ static int qemuDomainShutdownFlags(virDomainPtr dom,
>> unsigned int flags)
>> if (virDomainShutdownFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
>> goto cleanup;
>>
>> -if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
>> +if ((!useAgent && qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) <
>> 0) ||
>> +(useAgent && qemuDomainObjBeginJobWithAgent(driver, vm,
>> +QEMU_JOB_MODIFY,
>> +QEMU_AGENT_JOB_MODIFY)
>> < 0))
>
> Looks a bit hard to parse, it would be easier to just call
> qemuDomainObjBeginJobWithAgent:
>
> qemuDomainAgentJob agentJob = QEMU_AGENT_JOB_NONE;
>
> if (useAgent)
> agentJob = QEMU_AGENT_JOB_MODIFY;
>
> if (qemuDomainObjBeginJobWithAgent(driver, vm, QEMU_JOB_MODIFY,
>agentJob) < 0)
> goto cleanup;
>
> Perhaps it would even make sense to document this usage if the caller
> does not always need to talk to the agent.
>
>> @@ -2026,7 +2032,10 @@ static int qemuDomainShutdownFlags(virDomainPtr dom,
>> unsigned int flags)
>> }
>>
>> endjob:
>> -qemuDomainObjEndJob(driver, vm);
>> +if (agentJob)
>> +qemuDomainObjEndJobWithAgent(driver, vm);
>> +else
>> +qemuDomainObjEndJob(driver, vm);
>
> And this would still work even with the suggested changes.
Okay.
>
>>
>> cleanup:
>> virDomainObjEndAPI();
>> @@ -2049,6 +2058,7 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags)
>> bool useAgent = false, agentRequested, acpiRequested;
>> bool isReboot = true;
>> bool agentForced;
>> +bool agentJob = false;
>> int agentFlag = QEMU_AGENT_SHUTDOWN_REBOOT;
>>
>> virCheckFlags(VIR_DOMAIN_REBOOT_ACPI_POWER_BTN |
>> @@ -2075,9 +2085,14 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags)
>> if (virDomainRebootEnsureACL(dom->conn, vm->def, flags) < 0)
>> goto cleanup;
>>
>> -if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
>> +if ((!useAgent && qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) <
>> 0) ||
>> +(useAgent && qemuDomainObjBeginJobWithAgent(driver, vm,
>> +QEMU_JOB_MODIFY,
>> +QEMU_AGENT_JOB_MODIFY)
>> < 0))
>
> The same pattern could be used here.
>
>> goto cleanup;
>>
>> +agentJob = useAgent;
>> +
>> agentForced = agentRequested && !acpiRequested;
>> if (!qemuDomainAgentAvailable(vm, agentForced)) {
>> if (agentForced)
>> @@ -2115,7 +2130,10 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags)
>> }
>>
>> endjob:
>> -qemuDomainObjEndJob(driver, vm);
>> +if (agentJob)
>> +qemuDomainObjEndJobWithAgent(driver, vm);
>> +else
>> +qemuDomainObjEndJob(driver, vm);
>>
>> cleanup:
>> virDomainObjEndAPI();
>> @@ -4949,6 +4967,7 @@ qemuDomainSetVcpusFlags(virDomainPtr dom,
>> virDomainDefPtr def;
>> virDomainDefPtr persistentDef;
>> bool hotpluggable = !!(flags & VIR_DOMAIN_VCPU_HOTPLUGGABLE);
>> +bool useAgent = !!(flags & VIR_DOMAIN_VCPU_GUEST);
>> int ret = -1;
>>
>> virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
>> @@ -4963,13 +4982,14 @@ qemuDomainSetVcpusFlags(virDomainPtr dom,
>> if (virDomainSetVcpusFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
>> goto cleanup;
>>
>> -if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
>> +if ((!useAgent && qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) <
>> 0) ||
>> +(useAgent && qemuDomainObjBeginAgentJob(driver, vm,
>> QEMU_AGENT_JOB_MODIFY) < 0))
>> goto cleanup;
>
> And here.
Actually
[libvirt] [dbus PATCH] maint: fix coding style
Signed-off-by: Katerina Koukiou
---
src/connect.c | 12 ++--
src/domain.c | 2 +-
src/events.c | 24
src/network.c | 2 +-
src/util.c| 6 +++---
src/util.h| 4 ++--
6 files changed, 25 insertions(+), 25 deletions(-)
diff --git a/src/connect.c b/src/connect.c
index 2e57d69..09e5628 100644
--- a/src/connect.c
+++ b/src/connect.c
@@ -1187,12 +1187,12 @@ virtDBusConnectNWFilterDefineXML(GVariant *inArgs,
static void
virtDBusConnectNWFilterLookupByName(GVariant *inArgs,
- GUnixFDList *inFDs G_GNUC_UNUSED,
- const gchar *objectPath G_GNUC_UNUSED,
- gpointer userData,
- GVariant **outArgs,
- GUnixFDList **outFDs G_GNUC_UNUSED,
- GError **error)
+GUnixFDList *inFDs G_GNUC_UNUSED,
+const gchar *objectPath G_GNUC_UNUSED,
+gpointer userData,
+GVariant **outArgs,
+GUnixFDList **outFDs G_GNUC_UNUSED,
+GError **error)
{
virtDBusConnect *connect = userData;
g_autoptr(virNWFilter) nwfilter = NULL;
diff --git a/src/domain.c b/src/domain.c
index 4aeb855..5c5e6da 100644
--- a/src/domain.c
+++ b/src/domain.c
@@ -2189,7 +2189,7 @@ virtDBusDomainMigrateToURI3(GVariant *inArgs,
virtDBusConnect *connect = userData;
g_autoptr(virDomain) domain = NULL;
g_autoptr(GVariantIter) iter = NULL;
-const gchar* dconuri;
+const gchar *dconuri;
g_auto(virtDBusUtilTypedParams) params = { 0 };
guint flags;
diff --git a/src/events.c b/src/events.c
index 7cdc71d..e635a61 100644
--- a/src/events.c
+++ b/src/events.c
@@ -546,10 +546,10 @@ virtDBusEventsDomainDiskChange(virConnectPtr connection
G_GNUC_UNUSED,
static gint
virtDBusEventsNetworkEvent(virConnectPtr connection G_GNUC_UNUSED,
- virNetworkPtr network,
- gint event,
- gint detail G_GNUC_UNUSED,
- gpointer opaque)
+ virNetworkPtr network,
+ gint event,
+ gint detail G_GNUC_UNUSED,
+ gpointer opaque)
{
virtDBusConnect *connect = opaque;
g_autofree gchar *path = NULL;
@@ -592,10 +592,10 @@ virtDBusEventsNodeDeviceEvent(virConnectPtr connection
G_GNUC_UNUSED,
static gint
virtDBusEventsSecretEvent(virConnectPtr connection G_GNUC_UNUSED,
- virSecretPtr secret,
- gint event,
- gint detail,
- gpointer opaque)
+ virSecretPtr secret,
+ gint event,
+ gint detail,
+ gpointer opaque)
{
virtDBusConnect *connect = opaque;
g_autofree gchar *path = NULL;
@@ -615,10 +615,10 @@ virtDBusEventsSecretEvent(virConnectPtr connection
G_GNUC_UNUSED,
static gint
virtDBusEventsStoragePoolEvent(virConnectPtr connection G_GNUC_UNUSED,
- virStoragePoolPtr storagePool,
- gint event,
- gint detail,
- gpointer opaque)
+ virStoragePoolPtr storagePool,
+ gint event,
+ gint detail,
+ gpointer opaque)
{
virtDBusConnect *connect = opaque;
g_autofree gchar *path = NULL;
diff --git a/src/network.c b/src/network.c
index 1bacb81..3957ee6 100644
--- a/src/network.c
+++ b/src/network.c
@@ -235,7 +235,7 @@ virtDBusNetworkGetDHCPLeases(GVariant *inArgs,
g_variant_builder_add(, "(sxisssuss)",
lease->iface, lease->expirytime,
lease->type, lease->mac,
- lease->iaid ? lease->iaid : "" ,
+ lease->iaid ? lease->iaid : "",
lease->ipaddr, lease->prefix,
lease->hostname ? lease->hostname : "",
lease->clientid ? lease->clientid : "");
diff --git a/src/util.c b/src/util.c
index a1b29dd..8c822f2 100644
--- a/src/util.c
+++ b/src/util.c
@@ -348,8 +348,8 @@ virtDBusUtilVirNodeDeviceListFree(virNodeDevicePtr *devs)
virNWFilterPtr
virtDBusUtilVirNWFilterFromBusPath(virConnectPtr connection,
- const gchar *path,
- const gchar *nwfilterPath)
+ const gchar *path,
+
[libvirt] [PATCH 3/6] qemu: command: remove unused LegacyNicStr arg 'prefix'
Hardcode the only string that's passed in
Signed-off-by: Cole Robinson
---
src/qemu/qemu_command.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index f7038a8c5e..31a0b7761a 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3354,15 +3354,13 @@ qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem)
static char *
qemuBuildLegacyNicStr(virDomainNetDefPtr net,
- const char *prefix,
int vlan)
{
char *str;
char macaddr[VIR_MAC_STRING_BUFLEN];
ignore_value(virAsprintf(,
- "%smacaddr=%s,vlan=%d%s%s%s%s",
- prefix ? prefix : "",
+ "nic,macaddr=%s,vlan=%d%s%s%s%s",
virMacAddrFormat(>mac, macaddr),
vlan,
(net->model ? ",model=" : ""),
@@ -8517,7 +8515,7 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
goto cleanup;
virCommandAddArgList(cmd, "-device", nic, NULL);
} else {
-if (!(nic = qemuBuildLegacyNicStr(net, "nic,", vlan)))
+if (!(nic = qemuBuildLegacyNicStr(net, vlan)))
goto cleanup;
virCommandAddArgList(cmd, "-net", nic, NULL);
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 6/6] qemu: command: vhost: cleanup error reporting
- Switch to cleanup: label and share free calls
- Don't overwrite qemuBuildNicDevStr error
Signed-off-by: Cole Robinson
---
src/qemu/qemu_command.c | 28 +++-
1 file changed, 11 insertions(+), 17 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index f2dbb3fadd..1ffcb5b1ae 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8147,11 +8147,12 @@ qemuBuildVhostuserCommandLine(virQEMUDriverPtr driver,
char *netdev = NULL;
unsigned int queues = net->driver.virtio.queues;
char *nic = NULL;
+int ret = -1;
if (!qemuDomainSupportsNicdev(def, net)) {
virReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("Nicdev support unavailable"));
-goto error;
+goto cleanup;
}
switch ((virDomainChrType)net->data.vhostuser->type) {
@@ -8160,7 +8161,7 @@ qemuBuildVhostuserCommandLine(virQEMUDriverPtr driver,
net->data.vhostuser,
net->info.alias, qemuCaps,
false,
chardevStdioLogd)))
-goto error;
+goto cleanup;
break;
case VIR_DOMAIN_CHR_TYPE_NULL:
@@ -8179,7 +8180,7 @@ qemuBuildVhostuserCommandLine(virQEMUDriverPtr driver,
virReportError(VIR_ERR_INTERNAL_ERROR,
_("vhost-user type '%s' not supported"),
virDomainChrTypeToString(net->data.vhostuser->type));
-goto error;
+goto cleanup;
}
if (queues > 1 &&
@@ -8187,45 +8188,38 @@ qemuBuildVhostuserCommandLine(virQEMUDriverPtr driver,
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("multi-queue is not supported for vhost-user "
"with this QEMU binary"));
-goto error;
+goto cleanup;
}
if (!(netdev = qemuBuildHostNetStr(net, driver,
NULL, 0, NULL, 0)))
-goto error;
+goto cleanup;
if
(virNetDevOpenvswitchGetVhostuserIfname(net->data.vhostuser->data.nix.path,
>ifname) < 0)
-goto error;
+goto cleanup;
virCommandAddArg(cmd, "-chardev");
virCommandAddArg(cmd, chardev);
-VIR_FREE(chardev);
virCommandAddArg(cmd, "-netdev");
virCommandAddArg(cmd, netdev);
-VIR_FREE(netdev);
if (!(nic = qemuBuildNicDevStr(def, net, bootindex,
queues, qemuCaps))) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("Error generating NIC -device string"));
-goto error;
+goto cleanup;
}
virCommandAddArgList(cmd, "-device", nic, NULL);
-VIR_FREE(nic);
-virObjectUnref(cfg);
-return 0;
-
- error:
+ret = 0;
+ cleanup:
virObjectUnref(cfg);
VIR_FREE(netdev);
VIR_FREE(chardev);
VIR_FREE(nic);
-return -1;
+return ret;
}
static int
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/6] qemu: command: Make qemuBuildNicStr static
It doesn't have any external callers Signed-off-by: Cole Robinson --- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_command.h | 5 - 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 20c6ac2a04..4625851dc5 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3352,7 +3352,7 @@ qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem) } -char * +static char * qemuBuildNicStr(virDomainNetDefPtr net, const char *prefix, int vlan) diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index da75645ac5..0bcbf3018b 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -92,11 +92,6 @@ char *qemuBuildHostNetStr(virDomainNetDefPtr net, char **vhostfd, size_t vhostfdSize); -/* Legacy, pre device support */ -char *qemuBuildNicStr(virDomainNetDefPtr net, - const char *prefix, - int vlan); - /* Current, best practice */ char *qemuBuildNicDevStr(virDomainDefPtr def, virDomainNetDefPtr net, -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/6] qemu: command: Replace vlan= with netdev=
Most VMs libvirt knows how to launch will have qemu network config like: -netdev $NETDEV_OPTS,id=netdev1 -device e1000,id=netdev1 However for machine types with built-in platform network devices, we currently do: -net nic,model=e1000,vlan=1 -net $NETDEV_OPTS,vlan=1 However for the platform case, all qemu versions we support can do: -netdev $NETDEV_OPTS,id=netdev1 -net nic,model=e1000,netdev=netdev1 Which simplifies our code, and is more future proof as qemu has deprecated the vlan= option. This series switches to the netdev= method, and performs some cleanups in related code. Thanks, Cole Cole Robinson (6): qemu: command: Make qemuBuildNicStr static qemu: command: Rename BuildNicStr to BuildLegacyNicStr qemu: command: remove unused LegacyNicStr arg 'prefix' qemu: command: replace vlan= with netdev= for legacy nic qemu: Remove vlan function arguments qemu: command: vhost: cleanup error reporting src/qemu/qemu_command.c | 99 ++- src/qemu/qemu_command.h | 8 -- src/qemu/qemu_hotplug.c | 3 +- .../arm-vexpressa9-basic.args | 4 +- 4 files changed, 33 insertions(+), 81 deletions(-) -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/6] qemu: command: replace vlan= with netdev= for legacy nic
VMs with hardcoded platform network devices are forced to use old
style '-net nic' command line config. Current we use qemu's vlan
option to hook this with the '-netdev' host side of things.
However since qemu 1.2 there is '-net nic,netdev=X' option for
explicitly referencing a netdev ID, which is more inline with
typical VM commandlines, so let's switch to that
Signed-off-by: Cole Robinson
---
src/qemu/qemu_command.c | 52 ++-
.../arm-vexpressa9-basic.args | 4 +-
2 files changed, 19 insertions(+), 37 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 31a0b7761a..a2687c5693 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3354,15 +3354,15 @@ qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem)
static char *
qemuBuildLegacyNicStr(virDomainNetDefPtr net,
- int vlan)
+ int vlan ATTRIBUTE_UNUSED)
{
char *str;
char macaddr[VIR_MAC_STRING_BUFLEN];
ignore_value(virAsprintf(,
- "nic,macaddr=%s,vlan=%d%s%s%s%s",
+ "nic,macaddr=%s,netdev=host%s%s%s%s%s",
virMacAddrFormat(>mac, macaddr),
- vlan,
+ net->info.alias,
(net->model ? ",model=" : ""),
(net->model ? net->model : ""),
(net->info.alias ? ",name=" : ""),
@@ -3374,7 +3374,7 @@ qemuBuildLegacyNicStr(virDomainNetDefPtr net,
char *
qemuBuildNicDevStr(virDomainDefPtr def,
virDomainNetDefPtr net,
- int vlan,
+ int vlan ATTRIBUTE_UNUSED,
unsigned int bootindex,
size_t vhostfdSize,
virQEMUCapsPtr qemuCaps)
@@ -3523,10 +3523,7 @@ qemuBuildNicDevStr(virDomainDefPtr def,
virBufferAsprintf(, ",host_mtu=%u", net->mtu);
}
-if (vlan == -1)
-virBufferAsprintf(, ",netdev=host%s", net->info.alias);
-else
-virBufferAsprintf(, ",vlan=%d", vlan);
+virBufferAsprintf(, ",netdev=host%s", net->info.alias);
virBufferAsprintf(, ",id=%s", net->info.alias);
virBufferAsprintf(, ",mac=%s",
virMacAddrFormat(>mac, macaddr));
@@ -3555,7 +3552,7 @@ qemuBuildNicDevStr(virDomainDefPtr def,
char *
qemuBuildHostNetStr(virDomainNetDefPtr net,
virQEMUDriverPtr driver,
-int vlan,
+int vlan ATTRIBUTE_UNUSED,
char **tapfd,
size_t tapfdSize,
char **vhostfd,
@@ -3670,13 +3667,7 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
break;
}
-if (vlan >= 0) {
-virBufferAsprintf(, "vlan=%d,", vlan);
-if (net->info.alias)
-virBufferAsprintf(, "name=host%s,", net->info.alias);
-} else {
-virBufferAsprintf(, "id=host%s,", net->info.alias);
-}
+virBufferAsprintf(, "id=host%s,", net->info.alias);
if (is_tap) {
if (vhostfdSize) {
@@ -8494,22 +8485,20 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
goto cleanup;
}
+if (!(host = qemuBuildHostNetStr(net, driver,
+ vlan,
+ tapfdName, tapfdSize,
+ vhostfdName, vhostfdSize)))
+goto cleanup;
+virCommandAddArgList(cmd, "-netdev", host, NULL);
+
/* Possible combinations:
*
- * 1. Old way: -net nic,model=e1000,vlan=1 -net tap,vlan=1
- * 2. New way: -netdev type=tap,id=netdev1 -device e1000,id=netdev1
- *
- * NB: The backend and frontend are reversed above
+ * Old way: -netdev type=tap,id=netdev1 \
+ * -net nic,model=e1000,netdev=netdev1
+ * New way: -netdev type=tap,id=netdev1 -device e1000,id=netdev1
*/
-
if (qemuDomainSupportsNicdev(def, net)) {
-if (!(host = qemuBuildHostNetStr(net, driver,
- vlan,
- tapfdName, tapfdSize,
- vhostfdName, vhostfdSize)))
-goto cleanup;
-virCommandAddArgList(cmd, "-netdev", host, NULL);
-
if (!(nic = qemuBuildNicDevStr(def, net, vlan, bootindex,
vhostfdSize, qemuCaps)))
goto cleanup;
@@ -8518,13 +8507,6 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
if (!(nic = qemuBuildLegacyNicStr(net, vlan)))
goto cleanup;
virCommandAddArgList(cmd, "-net", nic, NULL);
-
-if (!(host = qemuBuildHostNetStr(net, driver,
- vlan,
- tapfdName, tapfdSize,
- vhostfdName,
[libvirt] [PATCH 5/6] qemu: Remove vlan function arguments
They are all unused now
Signed-off-by: Cole Robinson
---
src/qemu/qemu_command.c | 23 +--
src/qemu/qemu_command.h | 3 ---
src/qemu/qemu_hotplug.c | 3 +--
3 files changed, 6 insertions(+), 23 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index a2687c5693..f2dbb3fadd 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3353,8 +3353,7 @@ qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem)
static char *
-qemuBuildLegacyNicStr(virDomainNetDefPtr net,
- int vlan ATTRIBUTE_UNUSED)
+qemuBuildLegacyNicStr(virDomainNetDefPtr net)
{
char *str;
char macaddr[VIR_MAC_STRING_BUFLEN];
@@ -3374,7 +3373,6 @@ qemuBuildLegacyNicStr(virDomainNetDefPtr net,
char *
qemuBuildNicDevStr(virDomainDefPtr def,
virDomainNetDefPtr net,
- int vlan ATTRIBUTE_UNUSED,
unsigned int bootindex,
size_t vhostfdSize,
virQEMUCapsPtr qemuCaps)
@@ -3552,7 +3550,6 @@ qemuBuildNicDevStr(virDomainDefPtr def,
char *
qemuBuildHostNetStr(virDomainNetDefPtr net,
virQEMUDriverPtr driver,
-int vlan ATTRIBUTE_UNUSED,
char **tapfd,
size_t tapfdSize,
char **vhostfd,
@@ -8194,7 +8191,6 @@ qemuBuildVhostuserCommandLine(virQEMUDriverPtr driver,
}
if (!(netdev = qemuBuildHostNetStr(net, driver,
- -1,
NULL, 0, NULL, 0)))
goto error;
@@ -8210,7 +8206,7 @@ qemuBuildVhostuserCommandLine(virQEMUDriverPtr driver,
virCommandAddArg(cmd, netdev);
VIR_FREE(netdev);
-if (!(nic = qemuBuildNicDevStr(def, net, -1, bootindex,
+if (!(nic = qemuBuildNicDevStr(def, net, bootindex,
queues, qemuCaps))) {
virReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("Error generating NIC -device string"));
@@ -8239,7 +8235,6 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
virDomainDefPtr def,
virDomainNetDefPtr net,
virQEMUCapsPtr qemuCaps,
- int vlan,
unsigned int bootindex,
virNetDevVPortProfileOp vmop,
bool standalone,
@@ -8486,7 +8481,6 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
}
if (!(host = qemuBuildHostNetStr(net, driver,
- vlan,
tapfdName, tapfdSize,
vhostfdName, vhostfdSize)))
goto cleanup;
@@ -8499,12 +8493,12 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
* New way: -netdev type=tap,id=netdev1 -device e1000,id=netdev1
*/
if (qemuDomainSupportsNicdev(def, net)) {
-if (!(nic = qemuBuildNicDevStr(def, net, vlan, bootindex,
+if (!(nic = qemuBuildNicDevStr(def, net, bootindex,
vhostfdSize, qemuCaps)))
goto cleanup;
virCommandAddArgList(cmd, "-device", nic, NULL);
} else {
-if (!(nic = qemuBuildLegacyNicStr(net, vlan)))
+if (!(nic = qemuBuildLegacyNicStr(net)))
goto cleanup;
virCommandAddArgList(cmd, "-net", nic, NULL);
}
@@ -8577,16 +8571,9 @@ qemuBuildNetCommandLine(virQEMUDriverPtr driver,
for (i = 0; i < def->nnets; i++) {
virDomainNetDefPtr net = def->nets[i];
-int vlan;
-
-/* VLANs are not used with -netdev and -device, so don't record
them */
-if (qemuDomainSupportsNicdev(def, net))
-vlan = -1;
-else
-vlan = i;
if (qemuBuildInterfaceCommandLine(driver, logManager, cmd, def,
net,
- qemuCaps, vlan, bootNet, vmop,
+ qemuCaps, bootNet, vmop,
standalone, nnicindexes,
nicindexes,
chardevStdioLogd) < 0)
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 0bcbf3018b..c78282eb09 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -83,10 +83,8 @@ qemuBuildChrDeviceStr(char **deviceStr,
virDomainChrDefPtr chr,
virQEMUCapsPtr qemuCaps);
-/* With vlan == -1, use netdev syntax, else old hostnet */
char *qemuBuildHostNetStr(virDomainNetDefPtr net,
virQEMUDriverPtr driver,
- int vlan,
char **tapfd,
size_t tapfdSize,
char
[libvirt] [PATCH 2/6] qemu: command: Rename BuildNicStr to BuildLegacyNicStr
Makes it less ambiguous
Signed-off-by: Cole Robinson
---
src/qemu/qemu_command.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 4625851dc5..f7038a8c5e 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3353,9 +3353,9 @@ qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem)
static char *
-qemuBuildNicStr(virDomainNetDefPtr net,
-const char *prefix,
-int vlan)
+qemuBuildLegacyNicStr(virDomainNetDefPtr net,
+ const char *prefix,
+ int vlan)
{
char *str;
char macaddr[VIR_MAC_STRING_BUFLEN];
@@ -8517,7 +8517,7 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
goto cleanup;
virCommandAddArgList(cmd, "-device", nic, NULL);
} else {
-if (!(nic = qemuBuildNicStr(net, "nic,", vlan)))
+if (!(nic = qemuBuildLegacyNicStr(net, "nic,", vlan)))
goto cleanup;
virCommandAddArgList(cmd, "-net", nic, NULL);
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Escape commas for qemuBuildDiskThrottling
Add comma escaping for disk->blkdeviotune.group_name.
Signed-off-by: Anya Harter
---
src/qemu/qemu_command.c | 4 ++--
tests/qemuxml2argvdata/name-escape.args | 5 +
tests/qemuxml2argvdata/name-escape.xml | 13 +
tests/qemuxml2argvtest.c| 2 ++
4 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 20c6ac2a04..e05b106a5e 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1554,8 +1554,8 @@ qemuBuildDiskThrottling(virDomainDiskDefPtr disk,
IOTUNE_ADD(size_iops_sec, "iops-size");
if (disk->blkdeviotune.group_name) {
-virBufferEscapeString(buf, ",throttling.group=%s",
- disk->blkdeviotune.group_name);
+virBufferAddLit(buf, ",throttling.group=");
+virQEMUBuildBufferEscapeComma(buf, disk->blkdeviotune.group_name);
}
IOTUNE_ADD(total_bytes_sec_max_length, "bps-total-max-length");
diff --git a/tests/qemuxml2argvdata/name-escape.args
b/tests/qemuxml2argvdata/name-escape.args
index 72ed2e8410..aef7c238ca 100644
--- a/tests/qemuxml2argvdata/name-escape.args
+++ b/tests/qemuxml2argvdata/name-escape.args
@@ -24,6 +24,11 @@ bar=2/monitor.sock,server,nowait \
-boot c \
-device usb-ccid,id=ccid0,bus=usb.0,port=1 \
-usb \
+-drive file=/dev/HostVG/QEMUGuest1,format=qcow2,if=none,id=drive-ide0-0-0,\
+cache=none,throttling.bps-total=5000,throttling.iops-total=6000,\
+throttling.bps-total-max=1,throttling.iops-total-max=11000,\
+throttling.group=libvirt_iotune_group1,,foo \
+-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
-device ccid-card-emulated,backend=certificates,cert1=cert1,,foo,cert2=cert2,\
cert3=cert3,db=/etc/pki/nssdb,,foo,id=smartcard0,bus=ccid0.0 \
-chardev tty,id=charserial0,path=/dev/ttyS2,,foo \
diff --git a/tests/qemuxml2argvdata/name-escape.xml
b/tests/qemuxml2argvdata/name-escape.xml
index 0580de1813..70a1ce09d3 100644
--- a/tests/qemuxml2argvdata/name-escape.xml
+++ b/tests/qemuxml2argvdata/name-escape.xml
@@ -14,6 +14,19 @@
destroy
/usr/bin/qemu-system-i686
+
+
+
+
+
+5000
+6000
+1
+11000
+libvirt_iotune_group1,foo
+
+
+
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index a9a493e308..582a9de7bb 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2758,6 +2758,8 @@ mymain(void)
DO_TEST("name-escape",
QEMU_CAPS_NAME_DEBUG_THREADS,
QEMU_CAPS_OBJECT_SECRET,
+QEMU_CAPS_DRIVE_IOTUNE_MAX,
+QEMU_CAPS_DRIVE_IOTUNE_GROUP,
QEMU_CAPS_VNC,
QEMU_CAPS_NAME_GUEST,
QEMU_CAPS_DEVICE_CIRRUS_VGA,
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] rpc: Fix segmentation fault when no worker pool is available
On Tue, Jun 19, 2018 at 05:30 PM +0200, "Daniel P. Berrangé"
wrote:
> On Tue, Jun 19, 2018 at 05:18:17PM +0200, Marc Hartmayer wrote:
>> On Tue, Jun 19, 2018 at 04:55 PM +0200, "Daniel P. Berrangé"
>> wrote:
>> > On Tue, Jun 19, 2018 at 04:51:13PM +0200, Erik Skultety wrote:
>> >> On Mon, Jun 18, 2018 at 12:38:06PM +0200, Marc Hartmayer wrote:
>> >> > On Mon, Jun 18, 2018 at 09:47 AM +0200, Erik Skultety
>> >> > wrote:
>> >> > > On Fri, Jun 15, 2018 at 03:31:34PM +0200, Marc Hartmayer wrote:
>> >> > >> On Fri, Jun 15, 2018 at 01:39 PM +0200, Marc Hartmayer
>> >> > >> wrote:
>> >> > >> > If srv->workers is a NULL pointer, as it is the case if there are
>> >> > >> > no
>> >> > >> > workers, then don't try to dereference it.
>> >> > >> >
>> >> > >> > Signed-off-by: Marc Hartmayer
>> >> > >> > Reviewed-by: Boris Fiuczynski
>> >> > >> > ---
>> >> > >> > src/rpc/virnetserver.c | 22 +++---
>> >> > >> > 1 file changed, 15 insertions(+), 7 deletions(-)
>> >> > >> >
>> >> > >> > diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
>> >> > >> > index 5ae809e372..be6f610880 100644
>> >> > >> > --- a/src/rpc/virnetserver.c
>> >> > >> > +++ b/src/rpc/virnetserver.c
>> >> > >> > @@ -933,13 +933,21 @@
>> >> > >> > virNetServerGetThreadPoolParameters(virNetServerPtr srv,
>> >> > >> > size_t *jobQueueDepth)
>> >> > >> > {
>> >> > >> > virObjectLock(srv);
>> >> > >> > -
>> >> > >> > -*minWorkers = virThreadPoolGetMinWorkers(srv->workers);
>> >> > >> > -*maxWorkers = virThreadPoolGetMaxWorkers(srv->workers);
>> >> > >> > -*freeWorkers = virThreadPoolGetFreeWorkers(srv->workers);
>> >> > >> > -*nWorkers = virThreadPoolGetCurrentWorkers(srv->workers);
>> >> > >> > -*nPrioWorkers = virThreadPoolGetPriorityWorkers(srv->workers);
>> >> > >> > -*jobQueueDepth = virThreadPoolGetJobQueueDepth(srv->workers);
>> >> > >> > +if (srv->workers) {
>> >> > >> > +*minWorkers = virThreadPoolGetMinWorkers(srv->workers);
>> >> > >> > +*maxWorkers = virThreadPoolGetMaxWorkers(srv->workers);
>> >> > >> > +*freeWorkers = virThreadPoolGetFreeWorkers(srv->workers);
>> >> > >> > +*nWorkers = virThreadPoolGetCurrentWorkers(srv->workers);
>> >> > >> > +*nPrioWorkers =
>> >> > >> > virThreadPoolGetPriorityWorkers(srv->workers);
>> >> > >> > +*jobQueueDepth =
>> >> > >> > virThreadPoolGetJobQueueDepth(srv->workers);
>> >> > >> > +} else {
>> >> > >> > +*minWorkers = 0;
>> >> > >> > +*maxWorkers = 0;
>> >> > >> > +*freeWorkers = 0;
>> >> > >> > +*nWorkers = 0;
>> >> > >> > +*nPrioWorkers = 0;
>> >> > >> > +*jobQueueDepth = 0;
>> >> > >> > +}
>> >> > >> >
>> >> > >> > virObjectUnlock(srv);
>> >> > >> > return 0;
>> >> > >> > --
>> >> > >> > 2.13.6
>> >> > >>
>> >> > >> After thinking again it probably makes more sense (and the code more
>> >> > >> beautiful) to initialize the worker pool even for maxworker=0 (within
>> >> > >
>> >> > > I don't understand why should we do that.
>> >> >
>> >> > Because right now there are several functionalities broken. Segmentation
>> >> > faults in virNetServerGet/SetThreadPoolParameters, it’s not possible to
>> >> > start with maxworkers=0 and then change it at runtime via
>> >>
>> >> Naturally, since no workers means noone to process the request, that is
>> >> IMHO
>> >> the expected behaviour.
>> >
>> > Yes, a daemon should either run with no workers, or should run with
>> > 1 or more workers. It is not value to change between these two modes.
>>
>> Okay.
>>
>> >
>> > So if there's a codepath that lets you change from 0 -> 1 workers,
>> > or the reverse, we should make sure that reports an error.
>>
>> virThreadPoolSetParameters allows this change... Does it make sense to
>> you to allow an empty thread pool (just to keep the values) but to
>> prohibit a change from 0 -> i workers (i != 0) (and the reverse)? The
>> condition in virNetServerDispatchNewMessage would be something like 'if
>> (virThreadPoolGetMaxWorkers(srv->workers) > 0) {' instead of 'if
>> (srv->workers)'. If yes, this would, IMHO, simplify the code paths and
>> would be much safer against null pointer dereferencing.
>
> virThreadPoolSetParameters should immediately return an error if
> the caller tries to switch between 0 and non-zero in either
> direction.
Yes, that’s clear (at least since your last answer :) ). In my opinion
it still makes sense to initialize an empty virThreadPool (which will
never be allowed to have threads) since this would avoid constructions
like
“if (virJSONValueObjectAppendNumberUint(object, "min_workers",
srv->workers == NULL ? 0 :
virThreadPoolGetMinWorkers(srv->workers)) < 0) {”
and even more important it would avoid segmentation faults (see the
original patch).
>
>
> Regards,
> Daniel
> --
> |: https://berrange.com -o-
Re: [libvirt] [PATCH 6/8] backup: Introduce virDomainBackup APIs
On Wed, Jun 13, 2018 at 11:42:27AM -0500, Eric Blake wrote: > Introduce a few more new public APIs related to incremental backups. > This builds on the previous notion of a checkpoint (without an > existing checkpoint, the new API is a full backup, differing only > from virDomainCopy in the point of time chosen); and also allows > creation of a new checkpoint at the same time as starting the backup > (after all, an incremental backup is only useful if it covers the > state since the previous backup). It also enhances event reporting > for signaling when a push model backup completes (where the > hypervisor creates the backup); note that the pull model does not > have an event (starting the backup lets a third party access the > data, and only the third party knows when it is finished). First, thanks for the work! (And for doing the detailed write-ups, as is your wont.) A super minor note: I hope you'll also add the API names in the commit message itself (like you did in the past, for the older APIs); it will be handy when browsing `git log` later. So far I see the new APIs are: - virDomainBackupBegin() - virDomainBackupGetXMLDesc() - virDomainBackupEnd() So, OpenStack Nova currently still uses virDomainBlockRebase(); it hasn't even moved to the newer virDomainBlockCopy(). But as we know, currently both of them have the limitation of having to undefine and then re-define the guest XML. As you suggested elsewhere, probably I could explore (once they are 'frozen') moving to these proposed APIs, which will work without having to do the undefine + re-define dance. > Signed-off-by: Eric Blake > --- > include/libvirt/libvirt-domain-checkpoint.h | 11 ++ > include/libvirt/libvirt-domain.h| 14 +- > src/driver-hypervisor.h | 14 ++ > src/libvirt-domain-checkpoint.c | 200 > > src/libvirt-domain.c| 8 +- > src/libvirt_public.syms | 3 + > tools/virsh-domain.c| 3 +- > 7 files changed, 249 insertions(+), 4 deletions(-) > > diff --git a/include/libvirt/libvirt-domain-checkpoint.h > b/include/libvirt/libvirt-domain-checkpoint.h > index 4a7dc73089..c1d382fddc 100644 > --- a/include/libvirt/libvirt-domain-checkpoint.h > +++ b/include/libvirt/libvirt-domain-checkpoint.h > @@ -144,4 +144,15 @@ int virDomainCheckpointDelete(virDomainCheckpointPtr > checkpoint, > int virDomainCheckpointRef(virDomainCheckpointPtr checkpoint); > int virDomainCheckpointFree(virDomainCheckpointPtr checkpoint); > > +/* Begin an incremental backup job, possibly creating a checkpoint. */ > +int virDomainBackupBegin(virDomainPtr domain, const char *diskXml, > + const char *checkpointXml, unsigned int flags); > + > +/* Learn about an ongoing backup job. */ > +char *virDomainBackupGetXMLDesc(virDomainPtr domain, int id, > +unsigned int flags); > + > +/* Complete an incremental backup job. */ > +int virDomainBackupEnd(virDomainPtr domain, int id, unsigned int flags); [...] -- /kashyap -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] rpc: Fix segmentation fault when no worker pool is available
On Tue, Jun 19, 2018 at 05:18:17PM +0200, Marc Hartmayer wrote:
> On Tue, Jun 19, 2018 at 04:55 PM +0200, "Daniel P. Berrangé"
> wrote:
> > On Tue, Jun 19, 2018 at 04:51:13PM +0200, Erik Skultety wrote:
> >> On Mon, Jun 18, 2018 at 12:38:06PM +0200, Marc Hartmayer wrote:
> >> > On Mon, Jun 18, 2018 at 09:47 AM +0200, Erik Skultety
> >> > wrote:
> >> > > On Fri, Jun 15, 2018 at 03:31:34PM +0200, Marc Hartmayer wrote:
> >> > >> On Fri, Jun 15, 2018 at 01:39 PM +0200, Marc Hartmayer
> >> > >> wrote:
> >> > >> > If srv->workers is a NULL pointer, as it is the case if there are no
> >> > >> > workers, then don't try to dereference it.
> >> > >> >
> >> > >> > Signed-off-by: Marc Hartmayer
> >> > >> > Reviewed-by: Boris Fiuczynski
> >> > >> > ---
> >> > >> > src/rpc/virnetserver.c | 22 +++---
> >> > >> > 1 file changed, 15 insertions(+), 7 deletions(-)
> >> > >> >
> >> > >> > diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
> >> > >> > index 5ae809e372..be6f610880 100644
> >> > >> > --- a/src/rpc/virnetserver.c
> >> > >> > +++ b/src/rpc/virnetserver.c
> >> > >> > @@ -933,13 +933,21 @@
> >> > >> > virNetServerGetThreadPoolParameters(virNetServerPtr srv,
> >> > >> > size_t *jobQueueDepth)
> >> > >> > {
> >> > >> > virObjectLock(srv);
> >> > >> > -
> >> > >> > -*minWorkers = virThreadPoolGetMinWorkers(srv->workers);
> >> > >> > -*maxWorkers = virThreadPoolGetMaxWorkers(srv->workers);
> >> > >> > -*freeWorkers = virThreadPoolGetFreeWorkers(srv->workers);
> >> > >> > -*nWorkers = virThreadPoolGetCurrentWorkers(srv->workers);
> >> > >> > -*nPrioWorkers = virThreadPoolGetPriorityWorkers(srv->workers);
> >> > >> > -*jobQueueDepth = virThreadPoolGetJobQueueDepth(srv->workers);
> >> > >> > +if (srv->workers) {
> >> > >> > +*minWorkers = virThreadPoolGetMinWorkers(srv->workers);
> >> > >> > +*maxWorkers = virThreadPoolGetMaxWorkers(srv->workers);
> >> > >> > +*freeWorkers = virThreadPoolGetFreeWorkers(srv->workers);
> >> > >> > +*nWorkers = virThreadPoolGetCurrentWorkers(srv->workers);
> >> > >> > +*nPrioWorkers =
> >> > >> > virThreadPoolGetPriorityWorkers(srv->workers);
> >> > >> > +*jobQueueDepth =
> >> > >> > virThreadPoolGetJobQueueDepth(srv->workers);
> >> > >> > +} else {
> >> > >> > +*minWorkers = 0;
> >> > >> > +*maxWorkers = 0;
> >> > >> > +*freeWorkers = 0;
> >> > >> > +*nWorkers = 0;
> >> > >> > +*nPrioWorkers = 0;
> >> > >> > +*jobQueueDepth = 0;
> >> > >> > +}
> >> > >> >
> >> > >> > virObjectUnlock(srv);
> >> > >> > return 0;
> >> > >> > --
> >> > >> > 2.13.6
> >> > >>
> >> > >> After thinking again it probably makes more sense (and the code more
> >> > >> beautiful) to initialize the worker pool even for maxworker=0 (within
> >> > >
> >> > > I don't understand why should we do that.
> >> >
> >> > Because right now there are several functionalities broken. Segmentation
> >> > faults in virNetServerGet/SetThreadPoolParameters, it’s not possible to
> >> > start with maxworkers=0 and then change it at runtime via
> >>
> >> Naturally, since no workers means noone to process the request, that is
> >> IMHO
> >> the expected behaviour.
> >
> > Yes, a daemon should either run with no workers, or should run with
> > 1 or more workers. It is not value to change between these two modes.
>
> Okay.
>
> >
> > So if there's a codepath that lets you change from 0 -> 1 workers,
> > or the reverse, we should make sure that reports an error.
>
> virThreadPoolSetParameters allows this change... Does it make sense to
> you to allow an empty thread pool (just to keep the values) but to
> prohibit a change from 0 -> i workers (i != 0) (and the reverse)? The
> condition in virNetServerDispatchNewMessage would be something like 'if
> (virThreadPoolGetMaxWorkers(srv->workers) > 0) {' instead of 'if
> (srv->workers)'. If yes, this would, IMHO, simplify the code paths and
> would be much safer against null pointer dereferencing.
virThreadPoolSetParameters should immediately return an error if
the caller tries to switch between 0 and non-zero in either
direction.
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] rpc: Fix segmentation fault when no worker pool is available
On Tue, Jun 19, 2018 at 05:03 PM +0200, "Daniel P. Berrangé"
wrote:
> On Tue, Jun 19, 2018 at 03:55:09PM +0100, Daniel P. Berrangé wrote:
>> On Tue, Jun 19, 2018 at 04:51:13PM +0200, Erik Skultety wrote:
>> > On Mon, Jun 18, 2018 at 12:38:06PM +0200, Marc Hartmayer wrote:
>> > > On Mon, Jun 18, 2018 at 09:47 AM +0200, Erik Skultety
>> > > wrote:
>> > > > On Fri, Jun 15, 2018 at 03:31:34PM +0200, Marc Hartmayer wrote:
>> > > >> On Fri, Jun 15, 2018 at 01:39 PM +0200, Marc Hartmayer
>> > > >> wrote:
>> > > >> > If srv->workers is a NULL pointer, as it is the case if there are no
>> > > >> > workers, then don't try to dereference it.
>> > > >> >
>> > > >> > Signed-off-by: Marc Hartmayer
>> > > >> > Reviewed-by: Boris Fiuczynski
>> > > >> > ---
>> > > >> > src/rpc/virnetserver.c | 22 +++---
>> > > >> > 1 file changed, 15 insertions(+), 7 deletions(-)
>> > > >> >
>> > > >> > diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
>> > > >> > index 5ae809e372..be6f610880 100644
>> > > >> > --- a/src/rpc/virnetserver.c
>> > > >> > +++ b/src/rpc/virnetserver.c
>> > > >> > @@ -933,13 +933,21 @@
>> > > >> > virNetServerGetThreadPoolParameters(virNetServerPtr srv,
>> > > >> > size_t *jobQueueDepth)
>> > > >> > {
>> > > >> > virObjectLock(srv);
>> > > >> > -
>> > > >> > -*minWorkers = virThreadPoolGetMinWorkers(srv->workers);
>> > > >> > -*maxWorkers = virThreadPoolGetMaxWorkers(srv->workers);
>> > > >> > -*freeWorkers = virThreadPoolGetFreeWorkers(srv->workers);
>> > > >> > -*nWorkers = virThreadPoolGetCurrentWorkers(srv->workers);
>> > > >> > -*nPrioWorkers = virThreadPoolGetPriorityWorkers(srv->workers);
>> > > >> > -*jobQueueDepth = virThreadPoolGetJobQueueDepth(srv->workers);
>> > > >> > +if (srv->workers) {
>> > > >> > +*minWorkers = virThreadPoolGetMinWorkers(srv->workers);
>> > > >> > +*maxWorkers = virThreadPoolGetMaxWorkers(srv->workers);
>> > > >> > +*freeWorkers = virThreadPoolGetFreeWorkers(srv->workers);
>> > > >> > +*nWorkers = virThreadPoolGetCurrentWorkers(srv->workers);
>> > > >> > +*nPrioWorkers =
>> > > >> > virThreadPoolGetPriorityWorkers(srv->workers);
>> > > >> > +*jobQueueDepth =
>> > > >> > virThreadPoolGetJobQueueDepth(srv->workers);
>> > > >> > +} else {
>> > > >> > +*minWorkers = 0;
>> > > >> > +*maxWorkers = 0;
>> > > >> > +*freeWorkers = 0;
>> > > >> > +*nWorkers = 0;
>> > > >> > +*nPrioWorkers = 0;
>> > > >> > +*jobQueueDepth = 0;
>> > > >> > +}
>> > > >> >
>> > > >> > virObjectUnlock(srv);
>> > > >> > return 0;
>> > > >> > --
>> > > >> > 2.13.6
>> > > >>
>> > > >> After thinking again it probably makes more sense (and the code more
>> > > >> beautiful) to initialize the worker pool even for maxworker=0 (within
>> > > >
>> > > > I don't understand why should we do that.
>> > >
>> > > Because right now there are several functionalities broken. Segmentation
>> > > faults in virNetServerGet/SetThreadPoolParameters, it’s not possible to
>> > > start with maxworkers=0 and then change it at runtime via
>> >
>> > Naturally, since no workers means noone to process the request, that is
>> > IMHO
>> > the expected behaviour.
>>
>> Yes, a daemon should either run with no workers, or should run with
>> 1 or more workers. It is not value to change between these two modes.
>>
>> So if there's a codepath that lets you change from 0 -> 1 workers,
>> or the reverse, we should make sure that reports an error.
>>
>> Essentially workers=0 is only intended for things like virtlockd
>> or virlogd which don't need to be multithreaded, or indeed must
>> *never* be multithreaded to avoid tickling kernel bugs like
>> virtlockd did in the past.
>
> Also note that workers=0 will cause libvirtd to deadlock, because
> the QEMU driver (and others too) assume that they run in a seperate
> thread from the main event loop.
Yep, I know. What is your preferred way to avoid this?
>
> Regards,
> Daniel
> --
> |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-https://fstop138.berrange.com :|
> |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
>
--
Beste Grüße / Kind regards
Marc Hartmayer
IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] rpc: Fix segmentation fault when no worker pool is available
On Tue, Jun 19, 2018 at 04:55 PM +0200, "Daniel P. Berrangé"
wrote:
> On Tue, Jun 19, 2018 at 04:51:13PM +0200, Erik Skultety wrote:
>> On Mon, Jun 18, 2018 at 12:38:06PM +0200, Marc Hartmayer wrote:
>> > On Mon, Jun 18, 2018 at 09:47 AM +0200, Erik Skultety
>> > wrote:
>> > > On Fri, Jun 15, 2018 at 03:31:34PM +0200, Marc Hartmayer wrote:
>> > >> On Fri, Jun 15, 2018 at 01:39 PM +0200, Marc Hartmayer
>> > >> wrote:
>> > >> > If srv->workers is a NULL pointer, as it is the case if there are no
>> > >> > workers, then don't try to dereference it.
>> > >> >
>> > >> > Signed-off-by: Marc Hartmayer
>> > >> > Reviewed-by: Boris Fiuczynski
>> > >> > ---
>> > >> > src/rpc/virnetserver.c | 22 +++---
>> > >> > 1 file changed, 15 insertions(+), 7 deletions(-)
>> > >> >
>> > >> > diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
>> > >> > index 5ae809e372..be6f610880 100644
>> > >> > --- a/src/rpc/virnetserver.c
>> > >> > +++ b/src/rpc/virnetserver.c
>> > >> > @@ -933,13 +933,21 @@
>> > >> > virNetServerGetThreadPoolParameters(virNetServerPtr srv,
>> > >> > size_t *jobQueueDepth)
>> > >> > {
>> > >> > virObjectLock(srv);
>> > >> > -
>> > >> > -*minWorkers = virThreadPoolGetMinWorkers(srv->workers);
>> > >> > -*maxWorkers = virThreadPoolGetMaxWorkers(srv->workers);
>> > >> > -*freeWorkers = virThreadPoolGetFreeWorkers(srv->workers);
>> > >> > -*nWorkers = virThreadPoolGetCurrentWorkers(srv->workers);
>> > >> > -*nPrioWorkers = virThreadPoolGetPriorityWorkers(srv->workers);
>> > >> > -*jobQueueDepth = virThreadPoolGetJobQueueDepth(srv->workers);
>> > >> > +if (srv->workers) {
>> > >> > +*minWorkers = virThreadPoolGetMinWorkers(srv->workers);
>> > >> > +*maxWorkers = virThreadPoolGetMaxWorkers(srv->workers);
>> > >> > +*freeWorkers = virThreadPoolGetFreeWorkers(srv->workers);
>> > >> > +*nWorkers = virThreadPoolGetCurrentWorkers(srv->workers);
>> > >> > +*nPrioWorkers =
>> > >> > virThreadPoolGetPriorityWorkers(srv->workers);
>> > >> > +*jobQueueDepth = virThreadPoolGetJobQueueDepth(srv->workers);
>> > >> > +} else {
>> > >> > +*minWorkers = 0;
>> > >> > +*maxWorkers = 0;
>> > >> > +*freeWorkers = 0;
>> > >> > +*nWorkers = 0;
>> > >> > +*nPrioWorkers = 0;
>> > >> > +*jobQueueDepth = 0;
>> > >> > +}
>> > >> >
>> > >> > virObjectUnlock(srv);
>> > >> > return 0;
>> > >> > --
>> > >> > 2.13.6
>> > >>
>> > >> After thinking again it probably makes more sense (and the code more
>> > >> beautiful) to initialize the worker pool even for maxworker=0 (within
>> > >
>> > > I don't understand why should we do that.
>> >
>> > Because right now there are several functionalities broken. Segmentation
>> > faults in virNetServerGet/SetThreadPoolParameters, it’s not possible to
>> > start with maxworkers=0 and then change it at runtime via
>>
>> Naturally, since no workers means noone to process the request, that is IMHO
>> the expected behaviour.
>
> Yes, a daemon should either run with no workers, or should run with
> 1 or more workers. It is not value to change between these two modes.
Okay.
>
> So if there's a codepath that lets you change from 0 -> 1 workers,
> or the reverse, we should make sure that reports an error.
virThreadPoolSetParameters allows this change... Does it make sense to
you to allow an empty thread pool (just to keep the values) but to
prohibit a change from 0 -> i workers (i != 0) (and the reverse)? The
condition in virNetServerDispatchNewMessage would be something like 'if
(virThreadPoolGetMaxWorkers(srv->workers) > 0) {' instead of 'if
(srv->workers)'. If yes, this would, IMHO, simplify the code paths and
would be much safer against null pointer dereferencing.
>
> Essentially workers=0 is only intended for things like virtlockd
> or virlogd which don't need to be multithreaded, or indeed must
> *never* be multithreaded to avoid tickling kernel bugs like
> virtlockd did in the past.
>
> Regards,
> Daniel
> --
> |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-https://fstop138.berrange.com :|
> |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
>
--
Beste Grüße / Kind regards
Marc Hartmayer
IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [dbus PATCH 14/14] Implement Reset method for NodeDevice Interface
On Fri, Jun 15, 2018 at 07:03:50PM +0200, Katerina Koukiou wrote: > Signed-off-by: Katerina Koukiou > --- > data/org.libvirt.NodeDevice.xml | 4 > src/nodedev.c | 21 + > 2 files changed, 25 insertions(+) Reviewed-by: Pavel Hrdina signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [dbus PATCH 13/14] Implement ReAttach method for NodeDevice Interface
On Fri, Jun 15, 2018 at 07:03:49PM +0200, Katerina Koukiou wrote: > Signed-off-by: Katerina Koukiou > --- > data/org.libvirt.NodeDevice.xml | 4 > src/nodedev.c | 21 + > 2 files changed, 25 insertions(+) Reviewed-by: Pavel Hrdina signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [dbus PATCH 12/14] Implement NodeDeviceLookupSCSIHostByWWN method for Connect Interface
On Fri, Jun 15, 2018 at 07:03:48PM +0200, Katerina Koukiou wrote: > Signed-off-by: Katerina Koukiou > --- > data/org.libvirt.Connect.xml | 8 > src/connect.c| 32 > 2 files changed, 40 insertions(+) Reviewed-by: Pavel Hrdina signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [v2 RESEND 3/3] qemu: Switch code to use new agent job APIs
On Tue, Jun 19, 2018 at 08:38:02 +0200, Michal Privoznik wrote:
> There are two sets of functions here:
> 1) some functions talk on both monitor and agent monitor,
> 2) some functions only talk on agent monitor.
>
> For functions from set 1) we need to use
> qemuDomainObjBeginJobWithAgent() and for functions from set 2) we
> need to use qemuDomainObjBeginAgentJob() only.
>
> Signed-off-by: Michal Privoznik
> ---
> src/qemu/qemu_driver.c | 91
> --
> 1 file changed, 58 insertions(+), 33 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 3abbe41895..cffd4c928a 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -1954,6 +1954,7 @@ static int qemuDomainShutdownFlags(virDomainPtr dom,
> unsigned int flags)
> bool useAgent = false, agentRequested, acpiRequested;
> bool isReboot = false;
> bool agentForced;
> +bool agentJob = false;
> int agentFlag = QEMU_AGENT_SHUTDOWN_POWERDOWN;
>
> virCheckFlags(VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN |
> @@ -1980,9 +1981,14 @@ static int qemuDomainShutdownFlags(virDomainPtr dom,
> unsigned int flags)
> if (virDomainShutdownFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
> goto cleanup;
>
> -if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
> +if ((!useAgent && qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) <
> 0) ||
> +(useAgent && qemuDomainObjBeginJobWithAgent(driver, vm,
> +QEMU_JOB_MODIFY,
> +QEMU_AGENT_JOB_MODIFY) <
> 0))
Looks a bit hard to parse, it would be easier to just call
qemuDomainObjBeginJobWithAgent:
qemuDomainAgentJob agentJob = QEMU_AGENT_JOB_NONE;
if (useAgent)
agentJob = QEMU_AGENT_JOB_MODIFY;
if (qemuDomainObjBeginJobWithAgent(driver, vm, QEMU_JOB_MODIFY,
agentJob) < 0)
goto cleanup;
Perhaps it would even make sense to document this usage if the caller
does not always need to talk to the agent.
> @@ -2026,7 +2032,10 @@ static int qemuDomainShutdownFlags(virDomainPtr dom,
> unsigned int flags)
> }
>
> endjob:
> -qemuDomainObjEndJob(driver, vm);
> +if (agentJob)
> +qemuDomainObjEndJobWithAgent(driver, vm);
> +else
> +qemuDomainObjEndJob(driver, vm);
And this would still work even with the suggested changes.
>
> cleanup:
> virDomainObjEndAPI();
> @@ -2049,6 +2058,7 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags)
> bool useAgent = false, agentRequested, acpiRequested;
> bool isReboot = true;
> bool agentForced;
> +bool agentJob = false;
> int agentFlag = QEMU_AGENT_SHUTDOWN_REBOOT;
>
> virCheckFlags(VIR_DOMAIN_REBOOT_ACPI_POWER_BTN |
> @@ -2075,9 +2085,14 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags)
> if (virDomainRebootEnsureACL(dom->conn, vm->def, flags) < 0)
> goto cleanup;
>
> -if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
> +if ((!useAgent && qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) <
> 0) ||
> +(useAgent && qemuDomainObjBeginJobWithAgent(driver, vm,
> +QEMU_JOB_MODIFY,
> +QEMU_AGENT_JOB_MODIFY) <
> 0))
The same pattern could be used here.
> goto cleanup;
>
> +agentJob = useAgent;
> +
> agentForced = agentRequested && !acpiRequested;
> if (!qemuDomainAgentAvailable(vm, agentForced)) {
> if (agentForced)
> @@ -2115,7 +2130,10 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags)
> }
>
> endjob:
> -qemuDomainObjEndJob(driver, vm);
> +if (agentJob)
> +qemuDomainObjEndJobWithAgent(driver, vm);
> +else
> +qemuDomainObjEndJob(driver, vm);
>
> cleanup:
> virDomainObjEndAPI();
> @@ -4949,6 +4967,7 @@ qemuDomainSetVcpusFlags(virDomainPtr dom,
> virDomainDefPtr def;
> virDomainDefPtr persistentDef;
> bool hotpluggable = !!(flags & VIR_DOMAIN_VCPU_HOTPLUGGABLE);
> +bool useAgent = !!(flags & VIR_DOMAIN_VCPU_GUEST);
> int ret = -1;
>
> virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
> @@ -4963,13 +4982,14 @@ qemuDomainSetVcpusFlags(virDomainPtr dom,
> if (virDomainSetVcpusFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
> goto cleanup;
>
> -if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
> +if ((!useAgent && qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) <
> 0) ||
> +(useAgent && qemuDomainObjBeginAgentJob(driver, vm,
> QEMU_AGENT_JOB_MODIFY) < 0))
> goto cleanup;
And here.
>
> if (virDomainObjGetDefs(vm, flags, , ) < 0)
> goto endjob;
>
> -if (flags & VIR_DOMAIN_VCPU_GUEST)
> +if (useAgent)
> ret = qemuDomainSetVcpusAgent(vm, nvcpus);
> else if
Re: [libvirt] [PATCH 2/2] rpc: Fix segmentation fault when no worker pool is available
On Tue, Jun 19, 2018 at 03:55:09PM +0100, Daniel P. Berrangé wrote:
> On Tue, Jun 19, 2018 at 04:51:13PM +0200, Erik Skultety wrote:
> > On Mon, Jun 18, 2018 at 12:38:06PM +0200, Marc Hartmayer wrote:
> > > On Mon, Jun 18, 2018 at 09:47 AM +0200, Erik Skultety
> > > wrote:
> > > > On Fri, Jun 15, 2018 at 03:31:34PM +0200, Marc Hartmayer wrote:
> > > >> On Fri, Jun 15, 2018 at 01:39 PM +0200, Marc Hartmayer
> > > >> wrote:
> > > >> > If srv->workers is a NULL pointer, as it is the case if there are no
> > > >> > workers, then don't try to dereference it.
> > > >> >
> > > >> > Signed-off-by: Marc Hartmayer
> > > >> > Reviewed-by: Boris Fiuczynski
> > > >> > ---
> > > >> > src/rpc/virnetserver.c | 22 +++---
> > > >> > 1 file changed, 15 insertions(+), 7 deletions(-)
> > > >> >
> > > >> > diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
> > > >> > index 5ae809e372..be6f610880 100644
> > > >> > --- a/src/rpc/virnetserver.c
> > > >> > +++ b/src/rpc/virnetserver.c
> > > >> > @@ -933,13 +933,21 @@
> > > >> > virNetServerGetThreadPoolParameters(virNetServerPtr srv,
> > > >> > size_t *jobQueueDepth)
> > > >> > {
> > > >> > virObjectLock(srv);
> > > >> > -
> > > >> > -*minWorkers = virThreadPoolGetMinWorkers(srv->workers);
> > > >> > -*maxWorkers = virThreadPoolGetMaxWorkers(srv->workers);
> > > >> > -*freeWorkers = virThreadPoolGetFreeWorkers(srv->workers);
> > > >> > -*nWorkers = virThreadPoolGetCurrentWorkers(srv->workers);
> > > >> > -*nPrioWorkers = virThreadPoolGetPriorityWorkers(srv->workers);
> > > >> > -*jobQueueDepth = virThreadPoolGetJobQueueDepth(srv->workers);
> > > >> > +if (srv->workers) {
> > > >> > +*minWorkers = virThreadPoolGetMinWorkers(srv->workers);
> > > >> > +*maxWorkers = virThreadPoolGetMaxWorkers(srv->workers);
> > > >> > +*freeWorkers = virThreadPoolGetFreeWorkers(srv->workers);
> > > >> > +*nWorkers = virThreadPoolGetCurrentWorkers(srv->workers);
> > > >> > +*nPrioWorkers =
> > > >> > virThreadPoolGetPriorityWorkers(srv->workers);
> > > >> > +*jobQueueDepth =
> > > >> > virThreadPoolGetJobQueueDepth(srv->workers);
> > > >> > +} else {
> > > >> > +*minWorkers = 0;
> > > >> > +*maxWorkers = 0;
> > > >> > +*freeWorkers = 0;
> > > >> > +*nWorkers = 0;
> > > >> > +*nPrioWorkers = 0;
> > > >> > +*jobQueueDepth = 0;
> > > >> > +}
> > > >> >
> > > >> > virObjectUnlock(srv);
> > > >> > return 0;
> > > >> > --
> > > >> > 2.13.6
> > > >>
> > > >> After thinking again it probably makes more sense (and the code more
> > > >> beautiful) to initialize the worker pool even for maxworker=0 (within
> > > >
> > > > I don't understand why should we do that.
> > >
> > > Because right now there are several functionalities broken. Segmentation
> > > faults in virNetServerGet/SetThreadPoolParameters, it’s not possible to
> > > start with maxworkers=0 and then change it at runtime via
> >
> > Naturally, since no workers means noone to process the request, that is IMHO
> > the expected behaviour.
>
> Yes, a daemon should either run with no workers, or should run with
> 1 or more workers. It is not value to change between these two modes.
>
> So if there's a codepath that lets you change from 0 -> 1 workers,
> or the reverse, we should make sure that reports an error.
>
> Essentially workers=0 is only intended for things like virtlockd
> or virlogd which don't need to be multithreaded, or indeed must
> *never* be multithreaded to avoid tickling kernel bugs like
> virtlockd did in the past.
Also note that workers=0 will cause libvirtd to deadlock, because
the QEMU driver (and others too) assume that they run in a seperate
thread from the main event loop.
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [dbus PATCH 11/14] Implement NodeDeviceLookupByName method for Connect Interface
On Fri, Jun 15, 2018 at 07:03:47PM +0200, Katerina Koukiou wrote:
> Signed-off-by: Katerina Koukiou
> ---
> data/org.libvirt.Connect.xml | 6 ++
> src/connect.c| 29 +
> tests/test_connect.py| 13 +
> 3 files changed, 48 insertions(+)
[...]
> diff --git a/src/connect.c b/src/connect.c
> index 5e146a6..6192f47 100644
> --- a/src/connect.c
> +++ b/src/connect.c
> @@ -1098,6 +1098,34 @@ virtDBusConnectNodeDeviceCreateXML(GVariant *inArgs,
> *outArgs = g_variant_new("(o)", path);
> }
>
> +static void
> +virtDBusConnectNodeDeviceLookupByName(GVariant *inArgs,
> + GUnixFDList *inFDs G_GNUC_UNUSED,
> + const gchar *objectPath G_GNUC_UNUSED,
> + gpointer userData,
> + GVariant **outArgs,
> + GUnixFDList **outFDs G_GNUC_UNUSED,
> + GError **error)
> +{
> +virtDBusConnect *connect = userData;
> +g_autoptr(virNodeDevice) dev = NULL;
> +g_autofree gchar *path = NULL;
> +const gchar *name;
> +
> +g_variant_get(inArgs, "(s)", );
s/(s)/()/
Reviewed-by: Pavel Hrdina
signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [dbus PATCH 10/14] Implement ListCaps method for NodeDevicesInterface
On Fri, Jun 15, 2018 at 07:03:46PM +0200, Katerina Koukiou wrote: > Signed-off-by: Katerina Koukiou > --- > data/org.libvirt.NodeDevice.xml | 5 + > src/nodedev.c | 37 + > tests/test_nodedev.py | 6 ++ > 3 files changed, 48 insertions(+) Reviewed-by: Pavel Hrdina signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [dbus PATCH 09/14] Implement GetXMLDesc property for NodeDevice Interface
On Fri, Jun 15, 2018 at 07:03:45PM +0200, Katerina Koukiou wrote: > Signed-off-by: Katerina Koukiou > --- > data/org.libvirt.NodeDevice.xml | 6 ++ > src/nodedev.c | 28 > tests/test_nodedev.py | 6 ++ > 3 files changed, 40 insertions(+) Reviewed-by: Pavel Hrdina signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [dbus PATCH 08/14] Implement Parent property for NodeDevice Interface
On Fri, Jun 15, 2018 at 07:03:44PM +0200, Katerina Koukiou wrote: > Signed-off-by: Katerina Koukiou > --- > data/org.libvirt.NodeDevice.xml | 5 + > src/nodedev.c | 22 ++ > tests/test_nodedev.py | 1 + > 3 files changed, 28 insertions(+) Reviewed-by: Pavel Hrdina signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [dbus PATCH 07/14] Implement Name property for NodeDevice Interface
On Fri, Jun 15, 2018 at 07:03:43PM +0200, Katerina Koukiou wrote: > Signed-off-by: Katerina Koukiou > --- > data/org.libvirt.NodeDevice.xml | 5 + > src/nodedev.c | 22 ++ > tests/test_nodedev.py | 8 > 3 files changed, 35 insertions(+) Reviewed-by: Pavel Hrdina signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] qemu: sev: Don't jump to endjob if SEV measurement retrieval fails
On Tue, Jun 19, 2018 at 03:52:34PM +0200, Marc Hartmayer wrote: > On Mon, Jun 18, 2018 at 09:20 AM +0200, Erik Skultety > wrote: > > If measurement retrieval fails we'd forget to call ExitMonitor to unlock > > the monitor. > > > > Signed-off-by: Erik Skultety > > Reported-by: Luyao Huang > > --- > > src/qemu/qemu_driver.c | 4 +--- > > 1 file changed, 1 insertion(+), 3 deletions(-) > > > > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > > index fd25eb1b0b..d71956988f 100644 > > --- a/src/qemu/qemu_driver.c > > +++ b/src/qemu/qemu_driver.c > > @@ -21513,10 +21513,8 @@ qemuDomainGetSEVMeasurement(virQEMUDriverPtr > > driver, > > > > qemuDomainObjEnterMonitor(driver, vm); > > tmp = qemuMonitorGetSEVMeasurement(QEMU_DOMAIN_PRIVATE(vm)->mon); > > -if (tmp == NULL) > > -goto endjob; > > > > -if (qemuDomainObjExitMonitor(driver, vm) < 0) > > +if (qemuDomainObjExitMonitor(driver, vm) < 0 || !tmp) > > goto endjob; > > > > if (virTypedParamsAddString(params, nparams, , > > -- > > 2.14.4 > > > > -- > > libvir-list mailing list > > libvir-list@redhat.com > > https://www.redhat.com/mailman/listinfo/libvir-list > > > > Small nit: I would probably just move the “if-tmp-block” behind the > “if-…ExitMonitor()-block”. But I have no strong opinion about that :) I'll make the change...we should be more consistent here though :). Thanks, Erik -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] rpc: Fix segmentation fault when no worker pool is available
On Tue, Jun 19, 2018 at 04:51:13PM +0200, Erik Skultety wrote:
> On Mon, Jun 18, 2018 at 12:38:06PM +0200, Marc Hartmayer wrote:
> > On Mon, Jun 18, 2018 at 09:47 AM +0200, Erik Skultety
> > wrote:
> > > On Fri, Jun 15, 2018 at 03:31:34PM +0200, Marc Hartmayer wrote:
> > >> On Fri, Jun 15, 2018 at 01:39 PM +0200, Marc Hartmayer
> > >> wrote:
> > >> > If srv->workers is a NULL pointer, as it is the case if there are no
> > >> > workers, then don't try to dereference it.
> > >> >
> > >> > Signed-off-by: Marc Hartmayer
> > >> > Reviewed-by: Boris Fiuczynski
> > >> > ---
> > >> > src/rpc/virnetserver.c | 22 +++---
> > >> > 1 file changed, 15 insertions(+), 7 deletions(-)
> > >> >
> > >> > diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
> > >> > index 5ae809e372..be6f610880 100644
> > >> > --- a/src/rpc/virnetserver.c
> > >> > +++ b/src/rpc/virnetserver.c
> > >> > @@ -933,13 +933,21 @@
> > >> > virNetServerGetThreadPoolParameters(virNetServerPtr srv,
> > >> > size_t *jobQueueDepth)
> > >> > {
> > >> > virObjectLock(srv);
> > >> > -
> > >> > -*minWorkers = virThreadPoolGetMinWorkers(srv->workers);
> > >> > -*maxWorkers = virThreadPoolGetMaxWorkers(srv->workers);
> > >> > -*freeWorkers = virThreadPoolGetFreeWorkers(srv->workers);
> > >> > -*nWorkers = virThreadPoolGetCurrentWorkers(srv->workers);
> > >> > -*nPrioWorkers = virThreadPoolGetPriorityWorkers(srv->workers);
> > >> > -*jobQueueDepth = virThreadPoolGetJobQueueDepth(srv->workers);
> > >> > +if (srv->workers) {
> > >> > +*minWorkers = virThreadPoolGetMinWorkers(srv->workers);
> > >> > +*maxWorkers = virThreadPoolGetMaxWorkers(srv->workers);
> > >> > +*freeWorkers = virThreadPoolGetFreeWorkers(srv->workers);
> > >> > +*nWorkers = virThreadPoolGetCurrentWorkers(srv->workers);
> > >> > +*nPrioWorkers = virThreadPoolGetPriorityWorkers(srv->workers);
> > >> > +*jobQueueDepth = virThreadPoolGetJobQueueDepth(srv->workers);
> > >> > +} else {
> > >> > +*minWorkers = 0;
> > >> > +*maxWorkers = 0;
> > >> > +*freeWorkers = 0;
> > >> > +*nWorkers = 0;
> > >> > +*nPrioWorkers = 0;
> > >> > +*jobQueueDepth = 0;
> > >> > +}
> > >> >
> > >> > virObjectUnlock(srv);
> > >> > return 0;
> > >> > --
> > >> > 2.13.6
> > >>
> > >> After thinking again it probably makes more sense (and the code more
> > >> beautiful) to initialize the worker pool even for maxworker=0 (within
> > >
> > > I don't understand why should we do that.
> >
> > Because right now there are several functionalities broken. Segmentation
> > faults in virNetServerGet/SetThreadPoolParameters, it’s not possible to
> > start with maxworkers=0 and then change it at runtime via
>
> Naturally, since no workers means noone to process the request, that is IMHO
> the expected behaviour.
Yes, a daemon should either run with no workers, or should run with
1 or more workers. It is not value to change between these two modes.
So if there's a codepath that lets you change from 0 -> 1 workers,
or the reverse, we should make sure that reports an error.
Essentially workers=0 is only intended for things like virtlockd
or virlogd which don't need to be multithreaded, or indeed must
*never* be multithreaded to avoid tickling kernel bugs like
virtlockd did in the past.
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [dbus PATCH 06/14] Implement Detach method for NodeDevice Interface
On Fri, Jun 15, 2018 at 07:03:42PM +0200, Katerina Koukiou wrote: > Signed-off-by: Katerina Koukiou > --- > data/org.libvirt.NodeDevice.xml | 5 + > src/nodedev.c | 25 + > 2 files changed, 30 insertions(+) > > diff --git a/data/org.libvirt.NodeDevice.xml b/data/org.libvirt.NodeDevice.xml > index 4ca2e11..ea9bd7c 100644 > --- a/data/org.libvirt.NodeDevice.xml > +++ b/data/org.libvirt.NodeDevice.xml > @@ -7,5 +7,10 @@ > value="See > https://libvirt.org/html/libvirt-libvirt-nodedev.html#virNodeDeviceDestroy"/> > > + > + +value="See > https://libvirt.org/html/libvirt-libvirt-nodedev.html#virNodeDeviceDetachFlags"/> > + Missing driverName argument. > + > > Reviewed-by: Pavel Hrdina signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] rpc: Fix segmentation fault when no worker pool is available
On Mon, Jun 18, 2018 at 12:38:06PM +0200, Marc Hartmayer wrote:
> On Mon, Jun 18, 2018 at 09:47 AM +0200, Erik Skultety
> wrote:
> > On Fri, Jun 15, 2018 at 03:31:34PM +0200, Marc Hartmayer wrote:
> >> On Fri, Jun 15, 2018 at 01:39 PM +0200, Marc Hartmayer
> >> wrote:
> >> > If srv->workers is a NULL pointer, as it is the case if there are no
> >> > workers, then don't try to dereference it.
> >> >
> >> > Signed-off-by: Marc Hartmayer
> >> > Reviewed-by: Boris Fiuczynski
> >> > ---
> >> > src/rpc/virnetserver.c | 22 +++---
> >> > 1 file changed, 15 insertions(+), 7 deletions(-)
> >> >
> >> > diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
> >> > index 5ae809e372..be6f610880 100644
> >> > --- a/src/rpc/virnetserver.c
> >> > +++ b/src/rpc/virnetserver.c
> >> > @@ -933,13 +933,21 @@
> >> > virNetServerGetThreadPoolParameters(virNetServerPtr srv,
> >> > size_t *jobQueueDepth)
> >> > {
> >> > virObjectLock(srv);
> >> > -
> >> > -*minWorkers = virThreadPoolGetMinWorkers(srv->workers);
> >> > -*maxWorkers = virThreadPoolGetMaxWorkers(srv->workers);
> >> > -*freeWorkers = virThreadPoolGetFreeWorkers(srv->workers);
> >> > -*nWorkers = virThreadPoolGetCurrentWorkers(srv->workers);
> >> > -*nPrioWorkers = virThreadPoolGetPriorityWorkers(srv->workers);
> >> > -*jobQueueDepth = virThreadPoolGetJobQueueDepth(srv->workers);
> >> > +if (srv->workers) {
> >> > +*minWorkers = virThreadPoolGetMinWorkers(srv->workers);
> >> > +*maxWorkers = virThreadPoolGetMaxWorkers(srv->workers);
> >> > +*freeWorkers = virThreadPoolGetFreeWorkers(srv->workers);
> >> > +*nWorkers = virThreadPoolGetCurrentWorkers(srv->workers);
> >> > +*nPrioWorkers = virThreadPoolGetPriorityWorkers(srv->workers);
> >> > +*jobQueueDepth = virThreadPoolGetJobQueueDepth(srv->workers);
> >> > +} else {
> >> > +*minWorkers = 0;
> >> > +*maxWorkers = 0;
> >> > +*freeWorkers = 0;
> >> > +*nWorkers = 0;
> >> > +*nPrioWorkers = 0;
> >> > +*jobQueueDepth = 0;
> >> > +}
> >> >
> >> > virObjectUnlock(srv);
> >> > return 0;
> >> > --
> >> > 2.13.6
> >>
> >> After thinking again it probably makes more sense (and the code more
> >> beautiful) to initialize the worker pool even for maxworker=0 (within
> >
> > I don't understand why should we do that.
>
> Because right now there are several functionalities broken. Segmentation
> faults in virNetServerGet/SetThreadPoolParameters, it’s not possible to
> start with maxworkers=0 and then change it at runtime via
Naturally, since no workers means noone to process the request, that is IMHO
the expected behaviour.
> virNetServerSetThreadPoolParameters. Sure we can fix all these problems,
> but then we’ve to introduce new code paths. Why can’t we just call
> virThreadPoolNew(0, 0 , …)? This will only initialize the memory
> structure of the pool and _no_ threads. The only change we’ve to do then
I got the picture from the previous message, it's just I wasn't convinced.
> is to change the condition 'if (srv->workers)' of
> 'virNetServerDispatchNewMessage' to something like 'if
> (virThreadPoolGetMaxWorkers(srv->workers) > 0)'.
>
> > We don't even initialize it for
> > libvirtd server - the implications are clear - you don't have workers, you
> > don't get to process a job.
>
> Yep. I don’t want to change that behavior either.
>
> >
> >> virNetServerNew) (=> we'll have to adapt virNetServerDispatchNewMessage
> >> as well). BTW, there is also a segmentation fault in
> >> virThreadPoolSetParameters… And currently it’s not possible to start
> >> with maxworkers set to 0 and then increase it via
> >
> > Do I assume correctly that virNetServerDispatchNewMessage would allocate a
> > new
> > worker if there was a request to process but the threadpool was empty?
>
> Do you mean with my proposed changes? Or without?
>
> > If so, I
> > don't see a reason to do that, why would anyone want to run with no
> > workers?
>
> Commit dff6d809fb6c851244ea07afd07f580d7320cc7a which actually
> introduces this feature says:
>
> “Allow RPC server to run single threaded
>
> Refactor the RPC server dispatcher code so that if 'max_workers==0' the
> entire server will run single threaded. This is useful for use cases
> where there will only ever be 1 client connected which serializes its
> requests”.
Having said all of the above, even though I'm surprised we have something like
^this, because to me max_workers == 0 means something different (mind the
risks), we should remain consistent, so thanks for pointing that out.
Besides, the more I think about it, I guess it makes sense to be able to both
expand and shrink the worker pool as the user pleases, even with setting the
worker pool size to 0, it's true that setting it wrong one time (be it for
whatever reason) resulting in essentially locking yourself up.
Now, as long as noone has
Re: [libvirt] [PATCH 4/4] qemu: Escape commas for qemuBuildGrapicsSPICECommandLine
On 06/19/2018 09:47 AM, Cole Robinson wrote:
> On 06/18/2018 07:52 PM, John Ferlan wrote:
>>
>>
>> On 06/18/2018 01:57 PM, Anya Harter wrote:
>>> Add comma escaping for cfg->spiceTLSx509certdir and
>>> graphics->data.spice.rendernode.
>>>
>>> Signed-off-by: Anya Harter
>>> ---
>>> src/qemu/qemu_command.c | 11 ---
>>> tests/qemuxml2argvdata/name-escape.args | 5 +++--
>>> tests/qemuxml2argvdata/name-escape.xml | 1 +
>>> tests/qemuxml2argvtest.c| 5 +
>>> 4 files changed, 17 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>>> index a9a5e200e9..36dccea9b2 100644
>>> --- a/src/qemu/qemu_command.c
>>> +++ b/src/qemu/qemu_command.c
>>> @@ -7974,8 +7974,11 @@
>>> qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg,
>>> !cfg->spicePassword)
>>> virBufferAddLit(, "disable-ticketing,");
>>>
>>> -if (hasSecure)
>>> -virBufferAsprintf(, "x509-dir=%s,", cfg->spiceTLSx509certdir);
>>> +if (hasSecure) {
>>> +virBufferAddLit(, "x509-dir=");
>>> +virQEMUBuildBufferEscapeComma(, cfg->spiceTLSx509certdir);
>>> +virBufferAsprintf(, ",");
>>
>> make syntax-check would have told you:
>>
>> src/qemu/qemu_command.c:7980:virBufferAsprintf(, ",");
>> src/qemu/qemu_command.c:8090:virBufferAsprintf(, ",");
>>
>> So here and below, I changed to virBufferAddLit before pushing.
>>
>>> +}
>>>
>>> switch (graphics->data.spice.defaultMode) {
>>> case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE:
>>> @@ -8082,7 +8085,9 @@
>>> qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg,
>>> goto error;
>>> }
>>>
>>> -virBufferAsprintf(, "rendernode=%s,",
>>> graphics->data.spice.rendernode);
>>> +virBufferAddLit(, "rendernode=");
>>> +virQEMUBuildBufferEscapeComma(,
>>> graphics->data.spice.rendernode);
>>> +virBufferAsprintf(, ",");
>>> }
>>> }
>>
>> Reviewed-by: John Ferlan
>>
>> John
>>
>> >From the last time I passed through this when Sukrit posted patches,
>> still to do are qemuBuildHostNetStr, qemuBuildDiskThrottling (for group
>> name), and various qemuBuildNetworkDriveURI, qemuBuildNetworkDriveStr,
>> and qemuGetDriveSourceString.
>>
>
> Oh cool, I didn't realize you had found more examples! I looked at some
> of these with Anya before the patch series.
>
> NetworkDriveURI is a private subset of NetworkDriveStr, so the former
> doesn't need any direct changes AFAICT.
>
> qemuGetDriveSourceString is called outside qemu_command.c, for example
> passing the result to qemu monitor commands. Anyone know if comma
> escaping applies there too? Same with qemuBuildHostNetStr
>
>From what I can tell qemuGetDriveSourceString and qemuBuildHostNetStr
usages outside of qemu_command.c should _not_ have comma escaping, which
makes sense as the comma isn't used as a delimiter in those substrings.
So the comma escaping should be done at the call sites of those
functions in qemu_command.c
Thanks,
Cole
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [dbus PATCH 05/14] Implement Destroy method for NodeDevice Interface
On Fri, Jun 15, 2018 at 07:03:41PM +0200, Katerina Koukiou wrote: > Signed-off-by: Katerina Koukiou > --- > > Some note about the test: > > 1: The previous test should not use the node_device_create fixture introduced > here > for the following reason: > The fixture is creating the device in the setup of the test since the > device creation is not supposed to be tested in other tests apart from > the CreateXML test. Only that test should not have the creation in the > setup, but in the actual test `call` part. > > 2: I didn't create the get_test_node_device function as other entities have > because ListNodeDevices method is not supported by the test driver > > data/org.libvirt.NodeDevice.xml | 4 > src/nodedev.c | 42 > + > tests/Makefile.am | 1 + > tests/libvirttest.py| 10 ++ > tests/test_nodedev.py | 31 ++ > 5 files changed, 88 insertions(+) > create mode 100755 tests/test_nodedev.py Reviewed-by: Pavel Hrdina signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [dbus PATCH 04/14] Implement NodeDeviceCreateXML method for Connect Interface
On Fri, Jun 15, 2018 at 07:03:40PM +0200, Katerina Koukiou wrote: > Signed-off-by: Katerina Koukiou > --- > data/org.libvirt.Connect.xml | 7 +++ > src/connect.c| 30 ++ > tests/libvirttest.py | 3 +++ > tests/test_connect.py| 14 ++ > tests/xmldata.py | 16 > 5 files changed, 70 insertions(+) Reviewed-by: Pavel Hrdina signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [dbus PATCH 03/14] Register NodeDevice Lifecycle Events
On Fri, Jun 15, 2018 at 07:03:39PM +0200, Katerina Koukiou wrote:
> Signed-off-by: Katerina Koukiou
> ---
> data/org.libvirt.Connect.xml | 7 +++
> src/connect.c| 13 +
> src/connect.h| 1 +
> src/events.c | 42 ++
> 4 files changed, 63 insertions(+)
[...]
> diff --git a/src/connect.h b/src/connect.h
> index 3b62edd..a864041 100644
> --- a/src/connect.h
> +++ b/src/connect.h
> @@ -24,6 +24,7 @@ struct virtDBusConnect {
>
> gint domainCallbackIds[VIR_DOMAIN_EVENT_ID_LAST];
> gint networkCallbackIds[VIR_NETWORK_EVENT_ID_LAST];
> +gint devCallbackIds[VIR_NODE_DEVICE_EVENT_ID_LAST];
I would prefer nodeDevCallbackIds.
> gint secretCallbackIds[VIR_SECRET_EVENT_ID_LAST];
> gint storagePoolCallbackIds[VIR_STORAGE_POOL_EVENT_ID_LAST];
> };
Reviewed-by: Pavel Hrdina
signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [dbus PATCH 02/14] Implement ListNodeDevices method for Connect Interface
On Fri, Jun 15, 2018 at 07:03:38PM +0200, Katerina Koukiou wrote: > Signed-off-by: Katerina Koukiou > --- > data/org.libvirt.Connect.xml | 6 ++ > src/connect.c| 38 ++ > 2 files changed, 44 insertions(+) Reviewed-by: Pavel Hrdina signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/4] qemu: Escape commas for qemuBuildChrChardevStr
On 06/18/2018 07:37 PM, John Ferlan wrote: > > > On 06/18/2018 01:57 PM, Anya Harter wrote: >> Add comma escaping for dev->data.file.path in cases >> VIR_DOMAIN_CHR_TYPE_DEV and VIR_DOMAIN_CHR_TYPE_PIPE. >> >> Signed-off-by: Anya Harter >> --- >> src/qemu/qemu_command.c | 9 + >> tests/qemuxml2argvdata/name-escape.args | 4 >> tests/qemuxml2argvdata/name-escape.xml | 7 +++ >> tests/qemuxml2argvtest.c| 3 ++- >> 4 files changed, 18 insertions(+), 5 deletions(-) >> > > Having tests is awesome! Thanks! > > Not sure why the bite size tasks omitted VIR_DOMAIN_CHR_TYPE_FILE too. > If you want to investigate the FILE case that'd be good - just to make > sure we aren't missing any! I'll still push this as is since it's fine, > but if the FILE needs something it can be added afterwards. VIR_DOMAIN_CHR_TYPE_FILE calls the function qemuBuildChrChardevFileStr and passes the dev->data.file.path into the parameter named fileval which I escape in the second patch. Please let me know if I am missing something here. Thanks, Anya > > Reviewed-by: John Ferlan > > John > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [dbus PATCH 01/14] Introduce NodeDevice Interface
On Fri, Jun 15, 2018 at 07:03:37PM +0200, Katerina Koukiou wrote: > Signed-off-by: Katerina Koukiou > --- > data/Makefile.am| 1 + > data/org.libvirt.NodeDevice.xml | 7 + > src/Makefile.am | 1 + > src/connect.c | 6 > src/connect.h | 1 + > src/nodedev.c | 65 > + > src/nodedev.h | 9 ++ > src/util.c | 35 ++ > src/util.h | 15 ++ > 9 files changed, 140 insertions(+) > create mode 100644 data/org.libvirt.NodeDevice.xml > create mode 100644 src/nodedev.c > create mode 100644 src/nodedev.h > > diff --git a/data/Makefile.am b/data/Makefile.am > index 721b874..b3fa614 100644 > --- a/data/Makefile.am > +++ b/data/Makefile.am > @@ -22,6 +22,7 @@ interfaces_files = \ > org.libvirt.Connect.xml \ > org.libvirt.Domain.xml \ > org.libvirt.Network.xml \ > + org.libvirt.NodeDevice.xml \ > org.libvirt.NWFilter.xml \ > org.libvirt.Secret.xml \ > org.libvirt.StoragePool.xml \ > diff --git a/data/org.libvirt.NodeDevice.xml b/data/org.libvirt.NodeDevice.xml > new file mode 100644 > index 000..7ca26d0 > --- /dev/null > +++ b/data/org.libvirt.NodeDevice.xml > @@ -0,0 +1,7 @@ > + 1.0//EN" > +"http://www.freedesktop.org/standards/dbus/1.0/introspect.dtd;> > + > + > + > + > + > diff --git a/src/Makefile.am b/src/Makefile.am > index 53d1a23..3ef3472 100644 > --- a/src/Makefile.am > +++ b/src/Makefile.am > @@ -10,6 +10,7 @@ DAEMON_SOURCES = \ > events.c events.h \ > gdbus.c gdbus.h \ > network.c network.h \ > + nodedev.c nodedev.h \ > nwfilter.c nwfilter.h \ > secret.c secret.h \ > storagepool.c storagepool.h \ > diff --git a/src/connect.c b/src/connect.c > index 4f2bdb6..08898c8 100644 > --- a/src/connect.c > +++ b/src/connect.c > @@ -2,6 +2,7 @@ > #include "domain.h" > #include "events.h" > #include "network.h" > +#include "nodedev.h" > #include "nwfilter.h" > #include "secret.h" > #include "storagepool.h" > @@ -1668,6 +1669,7 @@ virtDBusConnectFree(virtDBusConnect *connect) > if (connect->connection) > virtDBusConnectClose(connect, TRUE); > > +g_free(connect->devPath); I would prefer nodeDevPath. Reviewed-by: Pavel Hrdina signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] security: Add swtpm paths to the domain's AppArmor profile
This patch extends the AppArmor domain profile with file paths
the swtpm accesses for state, log, pid, and socket files.
Both, QEMU and swtpm, use this AppArmor profile.
Signed-off-by: Stefan Berger
Cc: Christian Ehrhardt
---
examples/apparmor/libvirt-qemu | 5 +
src/security/virt-aa-helper.c | 45 ++
2 files changed, 50 insertions(+)
diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu
index 874aca2092..df5f512487 100644
--- a/examples/apparmor/libvirt-qemu
+++ b/examples/apparmor/libvirt-qemu
@@ -158,6 +158,11 @@
/usr/{lib,lib64}/qemu/*.so mr,
/usr/lib/@{multiarch}/qemu/*.so mr,
+ # swtpm
+ /{usr/,}bin/swtpm rmix,
+ /usr/{lib,lib64}/libswtpm_libtpms.so mr,
+ /usr/lib/@{multiarch}/libswtpm_libtpms.so mr,
+
# for save and resume
/{usr/,}bin/dash rmix,
/{usr/,}bin/dd rmix,
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index 971ee6733c..952b496f21 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -1181,6 +1181,51 @@ get_files(vahControl * ctl)
}
}
+if (ctl->def->tpm) {
+char *shortName = NULL;
+const char *tpmpath = NULL;
+
+switch (ctl->def->tpm->type) {
+case VIR_DOMAIN_TPM_TYPE_EMULATOR:
+shortName = virDomainDefGetShortName(ctl->def);
+
+switch (ctl->def->tpm->version) {
+case VIR_DOMAIN_TPM_VERSION_1_2:
+tpmpath = "tpm1.2";
+break;
+case VIR_DOMAIN_TPM_VERSION_2_0:
+tpmpath = "tpm2";
+break;
+case VIR_DOMAIN_TPM_VERSION_DEFAULT:
+case VIR_DOMAIN_TPM_VERSION_LAST:
+break;
+}
+
+/* Unix socket for QEMU and swtpm to use */
+virBufferAsprintf(,
+" \"/run/libvirt/qemu/swtpm/%s-swtpm.sock\" rw,\n",
+shortName);
+/* Paths for swtpm to use: give it access to its state
+ * directory, log, and PID files.
+ */
+virBufferAsprintf(,
+" \"%s/lib/libvirt/swtpm/%s/%s/**\" rw,\n",
+LOCALSTATEDIR, uuidstr, tpmpath);
+virBufferAsprintf(,
+" \"%s/log/swtpm/libvirt/qemu/%s-swtpm.log\" a,\n",
+LOCALSTATEDIR, ctl->def->name);
+virBufferAsprintf(,
+" \"/run/libvirt/qemu/swtpm/%s-swtpm.pid\" rw,\n",
+shortName);
+
+VIR_FREE(shortName);
+break;
+case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
+case VIR_DOMAIN_TPM_TYPE_LAST:
+break;
+}
+}
+
if (ctl->def->virtType == VIR_DOMAIN_VIRT_KVM) {
for (i = 0; i < ctl->def->nnets; i++) {
virDomainNetDefPtr net = ctl->def->nets[i];
--
2.14.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] qemu: sev: Don't jump to endjob if SEV measurement retrieval fails
On Mon, Jun 18, 2018 at 09:20 AM +0200, Erik Skultety wrote: > If measurement retrieval fails we'd forget to call ExitMonitor to unlock > the monitor. > > Signed-off-by: Erik Skultety > Reported-by: Luyao Huang > --- > src/qemu/qemu_driver.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index fd25eb1b0b..d71956988f 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -21513,10 +21513,8 @@ qemuDomainGetSEVMeasurement(virQEMUDriverPtr driver, > > qemuDomainObjEnterMonitor(driver, vm); > tmp = qemuMonitorGetSEVMeasurement(QEMU_DOMAIN_PRIVATE(vm)->mon); > -if (tmp == NULL) > -goto endjob; > > -if (qemuDomainObjExitMonitor(driver, vm) < 0) > +if (qemuDomainObjExitMonitor(driver, vm) < 0 || !tmp) > goto endjob; > > if (virTypedParamsAddString(params, nparams, , > -- > 2.14.4 > > -- > libvir-list mailing list > libvir-list@redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list > Small nit: I would probably just move the “if-tmp-block” behind the “if-…ExitMonitor()-block”. But I have no strong opinion about that :) Anyway Reviewed-by: Marc Hartmayer -- Beste Grüße / Kind regards Marc Hartmayer IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/8] Work-in-progress: Incremental Backup API additions
CC: Daniel Erez CC: Yaniv Dary CC: Allon Mureinik full thread: https://www.redhat.com/archives/libvir-list/2018-June/msg01066.html On 06/13/2018 12:42 PM, Eric Blake wrote: > I'm offline the rest of this week, but wanted to post the > progress I've made on patches towards the Incremental Backup RFC: > https://www.redhat.com/archives/libvir-list/2018-May/msg01403.html > > Comments welcome, including any naming suggestions > > Still to go: > - Add .rng file for validating the XML format used in virDomainBackupBegin() > - Add flags for validating XML > - Add src/conf/checkpoint_conf.c mirroring src/conf/snapshot_conf.c for > tracking tree of checkpoints > - Add virsh wrappers for calling everything > - Add qemu implementation - my first addition will probably just be for > push model full backups, then additional patches to expand into > pull model (on the qemu list, I still need to review and incorporate > Vladimir's patches for exporting a bitmap over NBD) > - Bug fixes (but why would there be any bugs in the first place? :) > > I've got portions of the qemu code working locally, but not polished > enough to post as a patch yet; my end goal is to have a working demo > against current qemu.git showing the use of virDomainBackupBegin() > for incremental backups with the push model prior to the code freeze > for 4.5.0 this month, even if that code doesn't get checked into > libvirt until later when the qemu code is changed to drop x- prefixes. > (That is, I'm hoping to demo that my API is sound, and thus we can > include the entrypoints in the libvirt.so for this release, even if > the libvirt code for driving pull mode over qemu waits until after a > qemu release where the pieces are promoted to a stable form.) > > Eric Blake (8): > snapshots: Avoid term 'checkpoint' for full system snapshot > backup: Document nuances between different state capture APIs > backup: Introduce virDomainCheckpointPtr > backup: Document new XML for backups > backup: Introduce virDomainCheckpoint APIs > backup: Introduce virDomainBackup APIs > backup: Add new domain:checkpoint access control > backup: Implement backup APIs for remote driver > > docs/Makefile.am| 3 + > docs/apibuild.py| 2 + > docs/docs.html.in | 9 +- > docs/domainstatecapture.html.in | 190 ++ > docs/formatcheckpoint.html.in | 273 + > docs/formatsnapshot.html.in | 16 +- > docs/schemas/domaincheckpoint.rng | 89 +++ > include/libvirt/libvirt-domain-checkpoint.h | 158 + > include/libvirt/libvirt-domain-snapshot.h | 10 +- > include/libvirt/libvirt-domain.h| 14 +- > include/libvirt/libvirt.h | 3 +- > include/libvirt/virterror.h | 5 +- > libvirt.spec.in | 2 + > mingw-libvirt.spec.in | 4 + > po/POTFILES | 1 + > src/Makefile.am | 2 + > src/access/viraccessperm.c | 5 +- > src/access/viraccessperm.h | 8 +- > src/conf/snapshot_conf.c| 2 +- > src/datatypes.c | 62 +- > src/datatypes.h | 31 +- > src/driver-hypervisor.h | 74 ++- > src/libvirt-domain-checkpoint.c | 908 > > src/libvirt-domain-snapshot.c | 4 +- > src/libvirt-domain.c| 8 +- > src/libvirt_private.syms| 2 + > src/libvirt_public.syms | 19 + > src/qemu/qemu_driver.c | 12 +- > src/remote/remote_daemon_dispatch.c | 15 + > src/remote/remote_driver.c | 31 +- > src/remote/remote_protocol.x| 237 +++- > src/remote_protocol-structs | 129 > src/rpc/gendispatch.pl | 32 +- > src/util/virerror.c | 15 +- > tests/domaincheckpointxml2xmlin/empty.xml | 1 + > tests/domaincheckpointxml2xmlout/empty.xml | 10 + > tests/virschematest.c | 2 + > tools/virsh-domain.c| 3 +- > tools/virsh-snapshot.c | 2 +- > tools/virsh.pod | 14 +- > 40 files changed, 2347 insertions(+), 60 deletions(-) > create mode 100644 docs/domainstatecapture.html.in > create mode 100644 docs/formatcheckpoint.html.in > create mode 100644 docs/schemas/domaincheckpoint.rng > create mode 100644 include/libvirt/libvirt-domain-checkpoint.h > create mode 100644 src/libvirt-domain-checkpoint.c > create mode 100644 tests/domaincheckpointxml2xmlin/empty.xml > create mode 100644 tests/domaincheckpointxml2xmlout/empty.xml > -- libvir-list mailing list libvir-list@redhat.com
Re: [libvirt] [PATCH 4/4] qemu: Escape commas for qemuBuildGrapicsSPICECommandLine
On 06/18/2018 07:52 PM, John Ferlan wrote:
>
>
> On 06/18/2018 01:57 PM, Anya Harter wrote:
>> Add comma escaping for cfg->spiceTLSx509certdir and
>> graphics->data.spice.rendernode.
>>
>> Signed-off-by: Anya Harter
>> ---
>> src/qemu/qemu_command.c | 11 ---
>> tests/qemuxml2argvdata/name-escape.args | 5 +++--
>> tests/qemuxml2argvdata/name-escape.xml | 1 +
>> tests/qemuxml2argvtest.c| 5 +
>> 4 files changed, 17 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index a9a5e200e9..36dccea9b2 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -7974,8 +7974,11 @@
>> qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg,
>> !cfg->spicePassword)
>> virBufferAddLit(, "disable-ticketing,");
>>
>> -if (hasSecure)
>> -virBufferAsprintf(, "x509-dir=%s,", cfg->spiceTLSx509certdir);
>> +if (hasSecure) {
>> +virBufferAddLit(, "x509-dir=");
>> +virQEMUBuildBufferEscapeComma(, cfg->spiceTLSx509certdir);
>> +virBufferAsprintf(, ",");
>
> make syntax-check would have told you:
>
> src/qemu/qemu_command.c:7980:virBufferAsprintf(, ",");
> src/qemu/qemu_command.c:8090:virBufferAsprintf(, ",");
>
> So here and below, I changed to virBufferAddLit before pushing.
>
>> +}
>>
>> switch (graphics->data.spice.defaultMode) {
>> case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE:
>> @@ -8082,7 +8085,9 @@
>> qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg,
>> goto error;
>> }
>>
>> -virBufferAsprintf(, "rendernode=%s,",
>> graphics->data.spice.rendernode);
>> +virBufferAddLit(, "rendernode=");
>> +virQEMUBuildBufferEscapeComma(,
>> graphics->data.spice.rendernode);
>> +virBufferAsprintf(, ",");
>> }
>> }
>
> Reviewed-by: John Ferlan
>
> John
>
>>From the last time I passed through this when Sukrit posted patches,
> still to do are qemuBuildHostNetStr, qemuBuildDiskThrottling (for group
> name), and various qemuBuildNetworkDriveURI, qemuBuildNetworkDriveStr,
> and qemuGetDriveSourceString.
>
Oh cool, I didn't realize you had found more examples! I looked at some
of these with Anya before the patch series.
NetworkDriveURI is a private subset of NetworkDriveStr, so the former
doesn't need any direct changes AFAICT.
qemuGetDriveSourceString is called outside qemu_command.c, for example
passing the result to qemu monitor commands. Anyone know if comma
escaping applies there too? Same with qemuBuildHostNetStr
Thanks,
Cole
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] qemu: sev: Use EnterMonitor instead of EnterMonitorAsync
On Mon, Jun 18, 2018 at 09:20 AM +0200, Erik Skultety wrote: > Since it's being called with QEMU_ASYNC_JOB_NONE which is what > qemuDomainObjEnterMonitor is going to use with the internal helper, > let's use that one instead. > > Signed-off-by: Erik Skultety > --- > src/qemu/qemu_driver.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index 42069ee617..fd25eb1b0b 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -21511,9 +21511,7 @@ qemuDomainGetSEVMeasurement(virQEMUDriverPtr driver, > if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0) > return -1; > > -if (qemuDomainObjEnterMonitorAsync(driver, vm, QEMU_ASYNC_JOB_NONE) < 0) > -goto endjob; > - > +qemuDomainObjEnterMonitor(driver, vm); > tmp = qemuMonitorGetSEVMeasurement(QEMU_DOMAIN_PRIVATE(vm)->mon); > if (tmp == NULL) > goto endjob; > -- > 2.14.4 > > -- > libvir-list mailing list > libvir-list@redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list > Reviewed-by: Marc Hartmayer -- Beste Grüße / Kind regards Marc Hartmayer IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 3/3] news: Update for the HTM pSeries feature
Signed-off-by: Andrea Bolognani --- docs/news.xml | 9 + 1 file changed, 9 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index 08e5dcbda3..35cc64866d 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -61,6 +61,15 @@ Support specifying extended TSEG size for SMM in QEMU. + + + qemu: Implement the HTM pSeries feature + + + Users can now decide whether HTM (Hardware Transactional Memory) + support should be available to the guest. + + -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 0/3] Implement the HTM pSeries feature
Applies cleanly on top of b1acfaaf890a16ecb2fecdbe7f121eff314dd0e9. Changes from [v1]: * drop qom-list-properties machinery, as equivalent functionality has been merged in the meantime; * don't introduce scaffolding for uniform machine option handling as a requirement for implementing this specific feature: we can do ad-hoc processing for now, per the status quo, then clean up everything (including existing features) with a separate series later. Changes from [RFC v3]: * can be considered for merging, since the QEMU part already has; * fix compatibility issues with QEMU <2.12 spotted by Shivaprasad; * drop all features except for HTM, at least for the time being; * add documentation. Changes from [RFC v2]: * use qom-list-properties to probe availability; * test all features with a single XML file. Changes from [RFC v1]: * don't nest features inside a element; * implement all optional features. [v1] https://www.redhat.com/archives/libvir-list/2018-March/msg00474.html [RFC v3] https://www.redhat.com/archives/libvir-list/2018-March/msg00042.html [RFC v2] https://www.redhat.com/archives/libvir-list/2018-February/msg00310.html [RFC v1] https://www.redhat.com/archives/libvir-list/2018-January/msg00779.html Andrea Bolognani (3): qemu: Add capability for the HTM pSeries feature qemu: Implement the HTM pSeries feature news: Update for the HTM pSeries feature docs/formatdomain.html.in | 8 + docs/news.xml | 9 + docs/schemas/domaincommon.rng | 5 + src/conf/domain_conf.c| 19 ++ src/conf/domain_conf.h| 1 + src/qemu/qemu_capabilities.c | 8 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 20 ++ src/qemu/qemu_domain.c| 13 ++ .../caps_2.12.0.aarch64.replies | 48 +++-- .../caps_2.12.0.aarch64.xml | 2 +- .../caps_2.12.0.ppc64.replies | 197 -- .../caps_2.12.0.ppc64.xml | 3 +- .../caps_2.12.0.s390x.replies | 52 +++-- .../caps_2.12.0.s390x.xml | 2 +- .../caps_2.12.0.x86_64.replies| 64 +++--- .../caps_2.12.0.x86_64.xml| 2 +- tests/qemuxml2argvdata/pseries-features.args | 3 +- tests/qemuxml2argvdata/pseries-features.xml | 1 + tests/qemuxml2argvtest.c | 1 + tests/qemuxml2xmltest.c | 1 + 21 files changed, 383 insertions(+), 77 deletions(-) -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 1/3] qemu: Add capability for the HTM pSeries feature
Signed-off-by: Andrea Bolognani
---
src/qemu/qemu_capabilities.c | 8 +
src/qemu/qemu_capabilities.h | 1 +
.../caps_2.12.0.aarch64.replies | 48 +++--
.../caps_2.12.0.aarch64.xml | 2 +-
.../caps_2.12.0.ppc64.replies | 197 --
.../caps_2.12.0.ppc64.xml | 3 +-
.../caps_2.12.0.s390x.replies | 52 +++--
.../caps_2.12.0.s390x.xml | 2 +-
.../caps_2.12.0.x86_64.replies| 64 +++---
.../caps_2.12.0.x86_64.xml| 2 +-
10 files changed, 303 insertions(+), 76 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 419208ad5c..4c96038c94 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -500,6 +500,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
/* 310 */
"sev-guest",
+ "machine.pseries.cap-htm",
);
@@ -1428,10 +1429,17 @@ static struct virQEMUCapsStringFlags
virQEMUCapsObjectPropsMemoryBackendFile[] =
{ "discard-data", QEMU_CAPS_OBJECT_MEMORY_FILE_DISCARD },
};
+static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsSPAPRMachine[] = {
+{ "cap-htm", QEMU_CAPS_MACHINE_PSERIES_CAP_HTM },
+};
+
static virQEMUCapsObjectTypeProps virQEMUCapsObjectProps[] = {
{ "memory-backend-file", virQEMUCapsObjectPropsMemoryBackendFile,
ARRAY_CARDINALITY(virQEMUCapsObjectPropsMemoryBackendFile),
QEMU_CAPS_OBJECT_MEMORY_FILE },
+{ "spapr-machine", virQEMUCapsObjectPropsSPAPRMachine,
+ ARRAY_CARDINALITY(virQEMUCapsObjectPropsSPAPRMachine),
+ -1 },
};
static void
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 3519a194e9..78c4e280cd 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -484,6 +484,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for
syntax-check */
/* 310 */
QEMU_CAPS_SEV_GUEST, /* -object sev-guest,... */
+QEMU_CAPS_MACHINE_PSERIES_CAP_HTM, /* -machine pseries.cap-htm */
QEMU_CAPS_LAST /* this must always be the last item */
} virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.replies
b/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.replies
index e0b4f6da38..9a8e54c63d 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.replies
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.replies
@@ -5582,10 +5582,26 @@
}
{
- "execute": "query-machines",
+ "execute": "qom-list-properties",
+ "arguments": {
+"typename": "spapr-machine"
+ },
"id": "libvirt-37"
}
+{
+ "id": "libvirt-37",
+ "error": {
+"class": "DeviceNotFound",
+"desc": "Class 'spapr-machine' not found"
+ }
+}
+
+{
+ "execute": "query-machines",
+ "id": "libvirt-38"
+}
+
{
"return": [
{
@@ -5880,12 +5896,12 @@
"cpu-max": 1
}
],
- "id": "libvirt-37"
+ "id": "libvirt-38"
}
{
"execute": "query-cpu-definitions",
- "id": "libvirt-38"
+ "id": "libvirt-39"
}
{
@@ -6061,35 +6077,35 @@
"static": false
}
],
- "id": "libvirt-38"
+ "id": "libvirt-39"
}
{
"execute": "query-tpm-models",
- "id": "libvirt-39"
+ "id": "libvirt-40"
}
{
"return": [
],
- "id": "libvirt-39"
+ "id": "libvirt-40"
}
{
"execute": "query-tpm-types",
- "id": "libvirt-40"
+ "id": "libvirt-41"
}
{
"return": [
"emulator"
],
- "id": "libvirt-40"
+ "id": "libvirt-41"
}
{
"execute": "query-command-line-options",
- "id": "libvirt-41"
+ "id": "libvirt-42"
}
{
@@ -7250,12 +7266,12 @@
"option": "drive"
}
],
- "id": "libvirt-41"
+ "id": "libvirt-42"
}
{
"execute": "query-migrate-capabilities",
- "id": "libvirt-42"
+ "id": "libvirt-43"
}
{
@@ -7317,12 +7333,12 @@
"capability": "dirty-bitmaps"
}
],
- "id": "libvirt-42"
+ "id": "libvirt-43"
}
{
"execute": "query-qmp-schema",
- "id": "libvirt-43"
+ "id": "libvirt-44"
}
{
@@ -18690,12 +18706,12 @@
"meta-type": "object"
}
],
- "id": "libvirt-43"
+ "id": "libvirt-44"
}
{
"execute": "query-gic-capabilities",
- "id": "libvirt-44"
+ "id": "libvirt-45"
}
{
@@ -18711,7 +18727,7 @@
"kernel": false
}
],
- "id": "libvirt-44"
+ "id": "libvirt-45"
}
{
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml
b/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml
index 5ed0290397..ecc029f403 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml
@@ -171,7 +171,7 @@
2011090
0
- 347313
+ 347550
v2.12.0-rc0
aarch64
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.replies
b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.replies
index 1bd1baa8a8..4f819150fe 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.replies
+++
[libvirt] [PATCH v2 2/3] qemu: Implement the HTM pSeries feature
https://bugzilla.redhat.com/show_bug.cgi?id=1525599
Signed-off-by: Andrea Bolognani
---
docs/formatdomain.html.in| 8
docs/schemas/domaincommon.rng| 5 +
src/conf/domain_conf.c | 19 +++
src/conf/domain_conf.h | 1 +
src/qemu/qemu_command.c | 20
src/qemu/qemu_domain.c | 13 +
tests/qemuxml2argvdata/pseries-features.args | 3 ++-
tests/qemuxml2argvdata/pseries-features.xml | 1 +
tests/qemuxml2argvtest.c | 1 +
tests/qemuxml2xmltest.c | 1 +
10 files changed, 71 insertions(+), 1 deletion(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 7e710d7c4a..0ab3235278 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1927,6 +1927,7 @@
smm state='on'
tseg unit='MiB'48/tseg
/smm
+ htm state='on'/
/features
...
@@ -2155,6 +2156,13 @@
Enable QEMU vmcoreinfo device to let the guest kernel save debug
details. Since 4.4.0 (QEMU only)
+ htm
+ Configure HTM (Hardware Transational Memory) availability for
+ pSeries guests. Possible values for the state attribute
+ are on and off. If the attribute is not
+ defined, the hypervisor default will be used.
+ Since 4.5.0 (QEMU/KVM only)
+
Time keeping
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 4a454dddb4..9a06630363 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -4900,6 +4900,11 @@
+
+
+
+
+
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 522e0c2bf3..9baca84c89 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -152,6 +152,7 @@ VIR_ENUM_IMPL(virDomainFeature, VIR_DOMAIN_FEATURE_LAST,
"ioapic",
"hpt",
"vmcoreinfo",
+ "htm",
);
VIR_ENUM_IMPL(virDomainCapabilitiesPolicy, VIR_DOMAIN_CAPABILITIES_POLICY_LAST,
@@ -19830,6 +19831,22 @@ virDomainDefParseXML(xmlDocPtr xml,
}
break;
+case VIR_DOMAIN_FEATURE_HTM:
+if (!(tmp = virXMLPropString(nodes[i], "state"))) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("missing state attribute '%s' of feature
'%s'"),
+ tmp, virDomainFeatureTypeToString(val));
+goto error;
+}
+if ((def->features[val] = virTristateSwitchTypeFromString(tmp)) <
0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown state attribute '%s' of feature
'%s'"),
+ tmp, virDomainFeatureTypeToString(val));
+goto error;
+}
+VIR_FREE(tmp);
+break;
+
/* coverity[dead_error_begin] */
case VIR_DOMAIN_FEATURE_LAST:
break;
@@ -21964,6 +21981,7 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr
src,
case VIR_DOMAIN_FEATURE_VMPORT:
case VIR_DOMAIN_FEATURE_SMM:
case VIR_DOMAIN_FEATURE_VMCOREINFO:
+case VIR_DOMAIN_FEATURE_HTM:
if (src->features[i] != dst->features[i]) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("State of feature '%s' differs: "
@@ -27622,6 +27640,7 @@ virDomainDefFormatInternal(virDomainDefPtr def,
case VIR_DOMAIN_FEATURE_PMU:
case VIR_DOMAIN_FEATURE_PVSPINLOCK:
case VIR_DOMAIN_FEATURE_VMPORT:
+case VIR_DOMAIN_FEATURE_HTM:
switch ((virTristateSwitch) def->features[i]) {
case VIR_TRISTATE_SWITCH_LAST:
case VIR_TRISTATE_SWITCH_ABSENT:
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 6344c02d1c..e697fd0f26 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1771,6 +1771,7 @@ typedef enum {
VIR_DOMAIN_FEATURE_IOAPIC,
VIR_DOMAIN_FEATURE_HPT,
VIR_DOMAIN_FEATURE_VMCOREINFO,
+VIR_DOMAIN_FEATURE_HTM,
VIR_DOMAIN_FEATURE_LAST
} virDomainFeature;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 20c6ac2a04..9a22baa3df 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7278,6 +7278,26 @@ qemuBuildMachineCommandLine(virCommandPtr cmd,
virBufferAsprintf(, ",resize-hpt=%s", str);
}
+if (def->features[VIR_DOMAIN_FEATURE_HTM] != VIR_TRISTATE_SWITCH_ABSENT) {
+const char *str;
+
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_PSERIES_CAP_HTM)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("HTM
Re: [libvirt] [PATCH] tests: Fix qemucapsfixreplies
On Tue, Jun 19, 2018 at 01:55:51PM +0200, Andrea Bolognani wrote: > Since e6be524508d5 we include the executed command along > with the reply in *.replies files, which breaks the > renumbering logic implemented in qemucapsfixreplies. > > Adapt the script so that it works with the new format. > > Signed-off-by: Andrea Bolognani > --- > tests/qemucapsfixreplies | 8 ++-- > 1 file changed, 6 insertions(+), 2 deletions(-) Reviewed-by: Pavel Hrdina signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] tests: Fix qemucapsfixreplies
Since e6be524508d5 we include the executed command along
with the reply in *.replies files, which breaks the
renumbering logic implemented in qemucapsfixreplies.
Adapt the script so that it works with the new format.
Signed-off-by: Andrea Bolognani
---
tests/qemucapsfixreplies | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/tests/qemucapsfixreplies b/tests/qemucapsfixreplies
index 4e3371a1f7..597f9ecd6e 100755
--- a/tests/qemucapsfixreplies
+++ b/tests/qemucapsfixreplies
@@ -8,7 +8,7 @@ if [ "$#" -ne 1 ] || [ "$1" = "--help" ] || [ ! -f "$1" ]; then
fi
awk -i inplace \
-'BEGIN {count=1; pattern="libvirt-[0-9]+"}
+'BEGIN {last=0; pattern="libvirt-[0-9]+"}
{
if (match($0, "libvirt-1[^0-9]")) {
count=1;
@@ -16,7 +16,11 @@ awk -i inplace \
if (match($0, pattern)) {
str="libvirt-" count;
sub(pattern, str, $0);
-count++;
+if (last != count) {
+last=count;
+} else {
+count++;
+}
}
print
}' "$1"
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [v2 RESEND 1/3] qemu: Introduce qemuDomainAgentJob
On Tue, Jun 19, 2018 at 08:38:00 +0200, Michal Privoznik wrote: > Introduce guest agent specific job categories to allow threads to > run agent monitor specific jobs while normal monitor jobs can > also be running. > > Alter _qemuDomainJobObj in order to duplicate certain fields that > will be used for guest agent specific tasks to increase > concurrency and throughput and reduce serialization. > > Signed-off-by: Michal Privoznik > --- > src/qemu/qemu_domain.c | 6 ++ > src/qemu/qemu_domain.h | 18 ++ > 2 files changed, 24 insertions(+) Reviewed-by: Jiri Denemark -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [v2 RESEND 2/3] qemu: Introduce APIs for manipulating qemuDomainAgentJob
On Tue, Jun 19, 2018 at 08:38:01 +0200, Michal Privoznik wrote:
> The point is to break QEMU_JOB_* into smaller pieces which
> enables us to achieve higher throughput. For instance, if there
> are two threads, one is trying to query something on qemu
> monitor while the other is trying to query something on agent
> monitor these two threads would serialize. There is not much
> reason for that.
>
> Signed-off-by: Michal Privoznik
> ---
> src/qemu/THREADS.txt | 112 ++---
> src/qemu/qemu_domain.c | 187
> +++--
> src/qemu/qemu_domain.h | 12
> 3 files changed, 264 insertions(+), 47 deletions(-)
...
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 91f3c6d236..30a06a1575 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
...
> @@ -6356,6 +6369,15 @@ qemuDomainJobAllowed(qemuDomainObjPrivatePtr priv,
> qemuDomainJob job)
> return !priv->job.active && qemuDomainNestedJobAllowed(priv, job);
> }
>
> +static bool
> +qemuDomainObjCanSetJob(qemuDomainObjPrivatePtr priv,
> + qemuDomainJob job,
> + qemuDomainAgentJob agentJob)
> +{
> +return (job == QEMU_JOB_NONE || !priv->job.active) &&
> + (agentJob == QEMU_AGENT_JOB_NONE || !priv->job.agentActive);
This is pretty strange, everything you compare here is an enum, either
qemuDomainJob or qemuDomainAgentJob and mixing ! with an explicit
comparison with *_NONE is confusing. It's not incorrect, but I think
(job == QEMU_JOB_NONE || priv->job.active == QEMU_JOB_NONE) &&
(agentJob == QEMU_AGENT_JOB_NONE || priv->job.agentActive ==
QEMU_AGENT_JOB_NONE)
would be easier to read. Or alternatively you could use ! everywhere.
> +}
> +
> /* Give up waiting for mutex after 30 seconds */
> #define QEMU_JOB_WAIT_TIME (1000ull * 30)
>
...
> @@ -6434,10 +6456,9 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver,
> goto error;
> }
>
> -while (priv->job.active) {
> +while (!qemuDomainObjCanSetJob(priv, job, agentJob)) {
You're now checking more variables for a single condition which means
waking up a single thread with virCondSignal could easily wake the one
which cannot currently run. We need to use virCondBroadcast instead and
I see you did the change, which is nice. However, it might be useful to
add a comment to the code explaining why we use virCondBroadcast to
avoid any future confusion or even blind attempts to replace it with
virCondSignal.
> if (nowait)
> goto cleanup;
> -
Drop this line removal, it's most likely a rebase artifact anyway.
> VIR_DEBUG("Waiting for job (vm=%p name=%s)", obj, obj->def->name);
> if (virCondWaitUntil(>job.cond, >parent.lock, then) < 0)
> goto error;
> @@ -6448,32 +6469,48 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver,
> if (!nested && !qemuDomainNestedJobAllowed(priv, job))
> goto retry;
>
> -qemuDomainObjResetJob(priv);
> -
> ignore_value(virTimeMillisNow());
>
> -if (job != QEMU_JOB_ASYNC) {
> -VIR_DEBUG("Started job: %s (async=%s vm=%p name=%s)",
> - qemuDomainJobTypeToString(job),
> - qemuDomainAsyncJobTypeToString(priv->job.asyncJob),
> - obj, obj->def->name);
> -priv->job.active = job;
> -priv->job.owner = virThreadSelfID();
> -priv->job.ownerAPI = virThreadJobGet();
> +if (job) {
> +qemuDomainObjResetJob(priv);
> +
> +if (job != QEMU_JOB_ASYNC) {
> +VIR_DEBUG("Started job: %s (async=%s vm=%p name=%s)",
> + qemuDomainJobTypeToString(job),
> + qemuDomainAsyncJobTypeToString(priv->job.asyncJob),
> + obj, obj->def->name);
> +priv->job.active = job;
> +priv->job.owner = virThreadSelfID();
> +priv->job.ownerAPI = virThreadJobGet();
> +priv->job.started = now;
> +} else {
> +VIR_DEBUG("Started async job: %s (vm=%p name=%s)",
> + qemuDomainAsyncJobTypeToString(asyncJob),
> + obj, obj->def->name);
> +qemuDomainObjResetAsyncJob(priv);
> +if (VIR_ALLOC(priv->job.current) < 0)
> +goto cleanup;
> +priv->job.current->status = QEMU_DOMAIN_JOB_STATUS_ACTIVE;
> +priv->job.asyncJob = asyncJob;
> +priv->job.asyncOwner = virThreadSelfID();
> +priv->job.asyncOwnerAPI = virThreadJobGet();
> +priv->job.asyncStarted = now;
> +priv->job.current->started = now;
> +}
> +}
> +
> +if (agentJob) {
> +qemuDomainObjResetAgentJob(priv);
> +
> +VIR_DEBUG("Started agent job: %s (vm=%p name=%s job=%s async=%s)",
> + qemuDomainAgentJobTypeToString(agentJob),
> + obj, obj->def->name,
> +
Re: [libvirt] [PATCH] Add condiitonal definition for constants
On Tue, Jun 19, 2018 at 10:06:25 +0100, Daniel Berrange wrote: > To be able to build against older libvirt, we need to conditionally > define the constants, even though they're not exposed in the public > API. > > Signed-off-by: Daniel P. Berrangé > --- > > Pushed as build fix. Please set up your subject-prefix for this project. signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 2/4] cmdDomblkinfo: add --all to show all block devices info
From: Chen Hanxiao
This patch introduces --all to show all block devices info
of guests like:
virsh # domblkinfo w08 --all
Target CapacityAllocation Physical
---
hda42949672960 9878110208 9878110208
vda10737418240 10736439296 10737418240
Target CapacityAllocation Physical
---
hda40.000 GiB 9.200 GiB 9.200 GiB
vda10.000 GiB 9.999 GiB 10.000 GiB
Signed-off-by: Chen Hanxiao
---
v3:
check error code on network disk
v2:
add support --human to --all
v1.1:
fix self-test
tools/virsh-domain-monitor.c | 128 +--
tools/virsh.pod | 5 +-
2 files changed, 112 insertions(+), 21 deletions(-)
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index daa86e8310..43e39f79c1 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -388,8 +388,7 @@ static const vshCmdInfo info_domblkinfo[] = {
static const vshCmdOptDef opts_domblkinfo[] = {
VIRSH_COMMON_OPT_DOMAIN_FULL(0),
{.name = "device",
- .type = VSH_OT_DATA,
- .flags = VSH_OFLAG_REQ,
+ .type = VSH_OT_STRING,
.completer = virshDomainDiskTargetCompleter,
.help = N_("block device")
},
@@ -397,30 +396,67 @@ static const vshCmdOptDef opts_domblkinfo[] = {
.type = VSH_OT_BOOL,
.help = N_("Human readable output")
},
+{.name = "all",
+ .type = VSH_OT_BOOL,
+ .help = N_("display all block devices info")
+},
{.name = NULL}
};
static void
cmdDomblkinfoPrint(vshControl *ctl,
const virDomainBlockInfo *info,
- bool human)
+ const char *device,
+ bool human, bool title)
{
+char *cap = NULL, *alloc = NULL, *phy = NULL;
+
+if (title) {
+vshPrintExtra(ctl, "%-10s %-15s %-15s %-15s\n", _("Target"),
+ _("Capacity"), _("Allocation"), _("Physical"));
+vshPrintExtra(ctl, "-"
+ "\n");
+return;
+}
+
if (!human) {
-vshPrint(ctl, "%-15s %llu\n", _("Capacity:"), info->capacity);
-vshPrint(ctl, "%-15s %llu\n", _("Allocation:"), info->allocation);
-vshPrint(ctl, "%-15s %llu\n", _("Physical:"), info->physical);
+if (device) {
+vshPrint(ctl, "%-10s %-15llu %-15llu %-15llu\n", device,
+ info->capacity, info->allocation, info->physical);
+} else {
+vshPrint(ctl, "%-15s %llu\n", _("Capacity:"), info->capacity);
+vshPrint(ctl, "%-15s %llu\n", _("Allocation:"), info->allocation);
+vshPrint(ctl, "%-15s %llu\n", _("Physical:"), info->physical);
+}
} else {
-double val;
-const char *unit;
-
-val = vshPrettyCapacity(info->capacity, );
-vshPrint(ctl, "%-15s %-.3lf %s\n", _("Capacity:"), val, unit);
-val = vshPrettyCapacity(info->allocation, );
-vshPrint(ctl, "%-15s %-.3lf %s\n", _("Allocation:"), val, unit);
-val = vshPrettyCapacity(info->physical, );
-vshPrint(ctl, "%-15s %-.3lf %s\n", _("Physical:"), val, unit);
+double val_cap, val_alloc, val_phy;
+const char *unit_cap, *unit_alloc, *unit_phy;
+
+val_cap = vshPrettyCapacity(info->capacity, _cap);
+val_alloc = vshPrettyCapacity(info->allocation, _alloc);
+val_phy = vshPrettyCapacity(info->physical, _phy);
+if (device) {
+if (virAsprintf(, "%.3lf %s", val_cap, unit_cap) < 0 ||
+virAsprintf(, "%.3lf %s", val_alloc, unit_alloc) < 0 ||
+virAsprintf(, "%.3lf %s", val_phy, unit_phy) < 0)
+goto cleanup;
+
+vshPrint(ctl, "%-10s %-15s %-15s %-15s\n",
+ device, cap, alloc, phy);
+} else {
+vshPrint(ctl, "%-15s %-.3lf %s\n", _("Capacity:"),
+ val_cap, unit_cap);
+vshPrint(ctl, "%-15s %-.3lf %s\n", _("Allocation:"),
+ val_alloc, unit_alloc);
+vshPrint(ctl, "%-15s %-.3lf %s\n", _("Physical:"),
+ val_phy, unit_phy);
+}
}
+ cleanup:
+VIR_FREE(cap);
+VIR_FREE(alloc);
+VIR_FREE(phy);
}
static bool
@@ -430,25 +466,77 @@ cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd)
virDomainPtr dom;
bool ret = false;
bool human = false;
+bool all = false;
const char *device = NULL;
+xmlDocPtr xmldoc = NULL;
+xmlXPathContextPtr ctxt = NULL;
+int ndisks;
+size_t i;
+xmlNodePtr *disks = NULL;
+char *target = NULL;
+char *protocol = NULL;
if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
return false;
-if (vshCommandOptStringReq(ctl, cmd, "device", ) < 0)
-goto cleanup;
[libvirt] [PATCH v3 3/4] cmdDomblkinfoPrint: support printing "-" for invalid virDomainBlockInfo
From: Chen Hanxiao
For inactive domain, we'll set virDomainBlockInfo to 0
if specific error code got.
Print "-" to show the value should be ignored in this scenario.
Signed-off-by: Chen Hanxiao
---
tools/virsh-domain-monitor.c | 73
1 file changed, 49 insertions(+), 24 deletions(-)
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 43e39f79c1..3acf5450b3 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -410,6 +410,15 @@ cmdDomblkinfoPrint(vshControl *ctl,
bool human, bool title)
{
char *cap = NULL, *alloc = NULL, *phy = NULL;
+bool invalid = false;
+
+struct blockInfoText {
+char *capacity;
+char *allocation;
+char *physical;
+};
+
+struct blockInfoText *blkInfoText = NULL;
if (title) {
vshPrintExtra(ctl, "%-10s %-15s %-15s %-15s\n", _("Target"),
@@ -419,15 +428,23 @@ cmdDomblkinfoPrint(vshControl *ctl,
return;
}
-if (!human) {
-if (device) {
-vshPrint(ctl, "%-10s %-15llu %-15llu %-15llu\n", device,
- info->capacity, info->allocation, info->physical);
-} else {
-vshPrint(ctl, "%-15s %llu\n", _("Capacity:"), info->capacity);
-vshPrint(ctl, "%-15s %llu\n", _("Allocation:"), info->allocation);
-vshPrint(ctl, "%-15s %llu\n", _("Physical:"), info->physical);
-}
+invalid = info->capacity == 0 &&
+ info->allocation == 0 &&
+ info->physical == 0;
+blkInfoText = vshCalloc(ctl, 1, sizeof(*blkInfoText));
+
+if (invalid) {
+blkInfoText->capacity = vshStrdup(ctl, "-");
+blkInfoText->allocation = vshStrdup(ctl, "-");
+blkInfoText->physical = vshStrdup(ctl, "-");
+} else if (!human) {
+if (virAsprintf(>capacity, "%llu",
+info->capacity) < 0 ||
+virAsprintf(>allocation, "%llu",
+info->allocation) < 0 ||
+virAsprintf(>physical, "%llu",
+info->physical) < 0)
+goto cleanup;
} else {
double val_cap, val_alloc, val_phy;
const char *unit_cap, *unit_alloc, *unit_phy;
@@ -435,28 +452,36 @@ cmdDomblkinfoPrint(vshControl *ctl,
val_cap = vshPrettyCapacity(info->capacity, _cap);
val_alloc = vshPrettyCapacity(info->allocation, _alloc);
val_phy = vshPrettyCapacity(info->physical, _phy);
-if (device) {
-if (virAsprintf(, "%.3lf %s", val_cap, unit_cap) < 0 ||
-virAsprintf(, "%.3lf %s", val_alloc, unit_alloc) < 0 ||
-virAsprintf(, "%.3lf %s", val_phy, unit_phy) < 0)
-goto cleanup;
-vshPrint(ctl, "%-10s %-15s %-15s %-15s\n",
- device, cap, alloc, phy);
-} else {
-vshPrint(ctl, "%-15s %-.3lf %s\n", _("Capacity:"),
- val_cap, unit_cap);
-vshPrint(ctl, "%-15s %-.3lf %s\n", _("Allocation:"),
- val_alloc, unit_alloc);
-vshPrint(ctl, "%-15s %-.3lf %s\n", _("Physical:"),
- val_phy, unit_phy);
-}
+if (virAsprintf(>capacity, "%.3lf %s",
+val_cap, unit_cap) < 0 ||
+virAsprintf(>allocation, "%.3lf %s",
+val_alloc, unit_alloc) < 0 ||
+virAsprintf(>physical, "%.3lf %s",
+val_phy, unit_phy) < 0)
+goto cleanup;
+}
+
+if (device) {
+vshPrint(ctl, "%-10s %-15s %-15s %-15s\n", device,
+ blkInfoText->capacity, blkInfoText->allocation,
+ blkInfoText->physical);
+} else {
+vshPrint(ctl, "%-15s %s\n", _("Capacity:"), blkInfoText->capacity);
+vshPrint(ctl, "%-15s %s\n", _("Allocation:"), blkInfoText->allocation);
+vshPrint(ctl, "%-15s %s\n", _("Physical:"), blkInfoText->physical);
}
cleanup:
VIR_FREE(cap);
VIR_FREE(alloc);
VIR_FREE(phy);
+if (blkInfoText) {
+VIR_FREE(blkInfoText->capacity);
+VIR_FREE(blkInfoText->allocation);
+VIR_FREE(blkInfoText->physical);
+}
+VIR_FREE(blkInfoText);
}
static bool
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 0/4] cmdDomblkinfo: introduce --all to show all block devices info
This series introduce --all to cmdDomblkinfo to show all block devices info in one cmd. Likes a combination of domblklist and domblkinfo. v3: check error code on network disk v2: add support --human for --all v1.1 fix a self test Chen Hanxiao (4): cmdDomblkinfo: introduce helper cmdDomblkinfoPrint cmdDomblkinfo: add --all to show all block devices info cmdDomblkinfoPrint: support printing "-" for invalid virDomainBlockInfo news: add cmdDomblkinfo --all option docs/news.xml| 10 +++ tools/virsh-domain-monitor.c | 160 ++- tools/virsh.pod | 5 +- 3 files changed, 155 insertions(+), 20 deletions(-) -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 4/4] news: add cmdDomblkinfo --all option
From: Chen Hanxiao Update news for cmdDomblkinfo --all option. Signed-off-by: Chen Hanxiao --- v3: update descriptions docs/news.xml | 10 ++ 1 file changed, 10 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index 08e5dcbda3..9bf7442047 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -204,6 +204,16 @@ secret-event, pool-event and nodedev-event) + + + virsh: Add --all to domblkinfo command + + + Alter the domblkinfo command to add the option + --all in order to display the size details of each domain + block device from one command in a output table. + + -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 1/4] cmdDomblkinfo: introduce helper cmdDomblkinfoPrint
From: Chen Hanxiao
Introduce helper cmdDomblkinfoPrint for printing.
Reviewed-by: John Ferlan
Signed-off-by: Chen Hanxiao
---
tools/virsh-domain-monitor.c | 39 ++--
1 file changed, 24 insertions(+), 15 deletions(-)
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 8cbb3db37c..daa86e8310 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -400,6 +400,29 @@ static const vshCmdOptDef opts_domblkinfo[] = {
{.name = NULL}
};
+static void
+cmdDomblkinfoPrint(vshControl *ctl,
+ const virDomainBlockInfo *info,
+ bool human)
+{
+if (!human) {
+vshPrint(ctl, "%-15s %llu\n", _("Capacity:"), info->capacity);
+vshPrint(ctl, "%-15s %llu\n", _("Allocation:"), info->allocation);
+vshPrint(ctl, "%-15s %llu\n", _("Physical:"), info->physical);
+} else {
+double val;
+const char *unit;
+
+val = vshPrettyCapacity(info->capacity, );
+vshPrint(ctl, "%-15s %-.3lf %s\n", _("Capacity:"), val, unit);
+val = vshPrettyCapacity(info->allocation, );
+vshPrint(ctl, "%-15s %-.3lf %s\n", _("Allocation:"), val, unit);
+val = vshPrettyCapacity(info->physical, );
+vshPrint(ctl, "%-15s %-.3lf %s\n", _("Physical:"), val, unit);
+}
+
+}
+
static bool
cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd)
{
@@ -420,21 +443,7 @@ cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd)
human = vshCommandOptBool(cmd, "human");
-if (!human) {
-vshPrint(ctl, "%-15s %llu\n", _("Capacity:"), info.capacity);
-vshPrint(ctl, "%-15s %llu\n", _("Allocation:"), info.allocation);
-vshPrint(ctl, "%-15s %llu\n", _("Physical:"), info.physical);
-} else {
-double val;
-const char *unit;
-
-val = vshPrettyCapacity(info.capacity, );
-vshPrint(ctl, "%-15s %-.3lf %s\n", _("Capacity:"), val, unit);
-val = vshPrettyCapacity(info.allocation, );
-vshPrint(ctl, "%-15s %-.3lf %s\n", _("Allocation:"), val, unit);
-val = vshPrettyCapacity(info.physical, );
-vshPrint(ctl, "%-15s %-.3lf %s\n", _("Physical:"), val, unit);
-}
+cmdDomblkinfoPrint(ctl, , human);
ret = true;
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Add condiitonal definition for constants
To be able to build against older libvirt, we need to conditionally define the constants, even though they're not exposed in the public API. Signed-off-by: Daniel P. Berrangé --- Pushed as build fix. connect_compat.h | 16 domain_compat.h | 4 2 files changed, 20 insertions(+) diff --git a/connect_compat.h b/connect_compat.h index cd6d678..f780e58 100644 --- a/connect_compat.h +++ b/connect_compat.h @@ -243,4 +243,20 @@ int virNodeGetSEVInfoCompat(virConnectPtr conn, int *nparams, unsigned int flags); +#ifndef VIR_NODE_SEV_CBITPOS +#define VIR_NODE_SEV_CBITPOS "cbitpos" +#endif + +#ifndef VIR_NODE_SEV_REDUCED_PHYS_BITS +#define VIR_NODE_SEV_REDUCED_PHYS_BITS "reduced-phys-bits" +#endif + +#ifndef VIR_NODE_SEV_PDH +#define VIR_NODE_SEV_PDH "pdh" +#endif + +#ifndef VIR_NODE_SEV_CERT_CHAIN +#define VIR_NODE_SEV_CERT_CHAIN "cert-chain" +#endif + #endif /* LIBVIRT_GO_CONNECT_COMPAT_H__ */ diff --git a/domain_compat.h b/domain_compat.h index 5c93ef5..345505c 100644 --- a/domain_compat.h +++ b/domain_compat.h @@ -1042,4 +1042,8 @@ int virDomainGetLaunchSecurityInfoCompat(virDomainPtr domain, int *nparams, unsigned int flags); +#ifndef VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT +#define VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT "sev-measurement" +#endif + #endif /* LIBVIRT_GO_DOMAIN_COMPAT_H__ */ -- 2.17.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [dbus PATCH] README: Drop warning about lack of API/ABI guarantees
On Mon, Jun 18, 2018 at 01:29:56PM +0200, Andrea Bolognani wrote: > We have promised not to break compatibility after v1.0.0, > which means the warning is no longer accurate and should > be dropped. > > Signed-off-by: Andrea Bolognani > --- Reviewed-by: Erik Skultety -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [v2 RESEND 3/3] qemu: Switch code to use new agent job APIs
There are two sets of functions here:
1) some functions talk on both monitor and agent monitor,
2) some functions only talk on agent monitor.
For functions from set 1) we need to use
qemuDomainObjBeginJobWithAgent() and for functions from set 2) we
need to use qemuDomainObjBeginAgentJob() only.
Signed-off-by: Michal Privoznik
---
src/qemu/qemu_driver.c | 91 --
1 file changed, 58 insertions(+), 33 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 3abbe41895..cffd4c928a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1954,6 +1954,7 @@ static int qemuDomainShutdownFlags(virDomainPtr dom,
unsigned int flags)
bool useAgent = false, agentRequested, acpiRequested;
bool isReboot = false;
bool agentForced;
+bool agentJob = false;
int agentFlag = QEMU_AGENT_SHUTDOWN_POWERDOWN;
virCheckFlags(VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN |
@@ -1980,9 +1981,14 @@ static int qemuDomainShutdownFlags(virDomainPtr dom,
unsigned int flags)
if (virDomainShutdownFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
goto cleanup;
-if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
+if ((!useAgent && qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
||
+(useAgent && qemuDomainObjBeginJobWithAgent(driver, vm,
+QEMU_JOB_MODIFY,
+QEMU_AGENT_JOB_MODIFY) <
0))
goto cleanup;
+agentJob = useAgent;
+
if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_RUNNING) {
virReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("domain is not running"));
@@ -2026,7 +2032,10 @@ static int qemuDomainShutdownFlags(virDomainPtr dom,
unsigned int flags)
}
endjob:
-qemuDomainObjEndJob(driver, vm);
+if (agentJob)
+qemuDomainObjEndJobWithAgent(driver, vm);
+else
+qemuDomainObjEndJob(driver, vm);
cleanup:
virDomainObjEndAPI();
@@ -2049,6 +2058,7 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags)
bool useAgent = false, agentRequested, acpiRequested;
bool isReboot = true;
bool agentForced;
+bool agentJob = false;
int agentFlag = QEMU_AGENT_SHUTDOWN_REBOOT;
virCheckFlags(VIR_DOMAIN_REBOOT_ACPI_POWER_BTN |
@@ -2075,9 +2085,14 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags)
if (virDomainRebootEnsureACL(dom->conn, vm->def, flags) < 0)
goto cleanup;
-if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
+if ((!useAgent && qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
||
+(useAgent && qemuDomainObjBeginJobWithAgent(driver, vm,
+QEMU_JOB_MODIFY,
+QEMU_AGENT_JOB_MODIFY) <
0))
goto cleanup;
+agentJob = useAgent;
+
agentForced = agentRequested && !acpiRequested;
if (!qemuDomainAgentAvailable(vm, agentForced)) {
if (agentForced)
@@ -2115,7 +2130,10 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags)
}
endjob:
-qemuDomainObjEndJob(driver, vm);
+if (agentJob)
+qemuDomainObjEndJobWithAgent(driver, vm);
+else
+qemuDomainObjEndJob(driver, vm);
cleanup:
virDomainObjEndAPI();
@@ -4949,6 +4967,7 @@ qemuDomainSetVcpusFlags(virDomainPtr dom,
virDomainDefPtr def;
virDomainDefPtr persistentDef;
bool hotpluggable = !!(flags & VIR_DOMAIN_VCPU_HOTPLUGGABLE);
+bool useAgent = !!(flags & VIR_DOMAIN_VCPU_GUEST);
int ret = -1;
virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
@@ -4963,13 +4982,14 @@ qemuDomainSetVcpusFlags(virDomainPtr dom,
if (virDomainSetVcpusFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
goto cleanup;
-if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
+if ((!useAgent && qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
||
+(useAgent && qemuDomainObjBeginAgentJob(driver, vm,
QEMU_AGENT_JOB_MODIFY) < 0))
goto cleanup;
if (virDomainObjGetDefs(vm, flags, , ) < 0)
goto endjob;
-if (flags & VIR_DOMAIN_VCPU_GUEST)
+if (useAgent)
ret = qemuDomainSetVcpusAgent(vm, nvcpus);
else if (flags & VIR_DOMAIN_VCPU_MAXIMUM)
ret = qemuDomainSetVcpusMax(driver, def, persistentDef, nvcpus);
@@ -4978,7 +4998,10 @@ qemuDomainSetVcpusFlags(virDomainPtr dom,
nvcpus, hotpluggable);
endjob:
-qemuDomainObjEndJob(driver, vm);
+if (useAgent)
+qemuDomainObjEndAgentJob(vm);
+else
+qemuDomainObjEndJob(driver, vm);
cleanup:
virDomainObjEndAPI();
@@ -5429,7 +5452,7 @@ qemuDomainGetVcpusFlags(virDomainPtr dom, unsigned int
flags)
goto cleanup;
if (flags & VIR_DOMAIN_VCPU_GUEST) {
-if (qemuDomainObjBeginJob(driver, vm,
[libvirt] [v2 RESEND 2/3] qemu: Introduce APIs for manipulating qemuDomainAgentJob
The point is to break QEMU_JOB_* into smaller pieces which enables us to achieve higher throughput. For instance, if there are two threads, one is trying to query something on qemu monitor while the other is trying to query something on agent monitor these two threads would serialize. There is not much reason for that. Signed-off-by: Michal Privoznik --- src/qemu/THREADS.txt | 112 ++--- src/qemu/qemu_domain.c | 187 +++-- src/qemu/qemu_domain.h | 12 3 files changed, 264 insertions(+), 47 deletions(-) diff --git a/src/qemu/THREADS.txt b/src/qemu/THREADS.txt index 7243161fe3..d17f3f4e0c 100644 --- a/src/qemu/THREADS.txt +++ b/src/qemu/THREADS.txt @@ -51,8 +51,8 @@ There are a number of locks on various objects Since virDomainObjPtr lock must not be held during sleeps, the job conditions provide additional protection for code making updates. -QEMU driver uses two kinds of job conditions: asynchronous and -normal. +QEMU driver uses three kinds of job conditions: asynchronous, agent +and normal. Asynchronous job condition is used for long running jobs (such as migration) that consist of several monitor commands and it is @@ -69,16 +69,23 @@ There are a number of locks on various objects it needs to wait until the asynchronous job ends and try to acquire the job again. +Agent job condition is then used when thread wishes to talk to qemu +agent monitor. It is possible to acquire just agent job +(qemuDomainObjBeginAgentJob), or only normal job +(qemuDomainObjBeginJob) or both at the same time +(qemuDomainObjBeginJobWithAgent). Which type of job to grab depends +whether caller wishes to communicate only with agent socket, or only +with qemu monitor socket or both, respectively. + Immediately after acquiring the virDomainObjPtr lock, any method -which intends to update state must acquire either asynchronous or -normal job condition. The virDomainObjPtr lock is released while -blocking on these condition variables. Once the job condition is -acquired, a method can safely release the virDomainObjPtr lock -whenever it hits a piece of code which may sleep/wait, and -re-acquire it after the sleep/wait. Whenever an asynchronous job -wants to talk to the monitor, it needs to acquire nested job (a -special kind of normal job) to obtain exclusive access to the -monitor. +which intends to update state must acquire asynchronous, normal or +agent job . The virDomainObjPtr lock is released while blocking on +these condition variables. Once the job condition is acquired, a +method can safely release the virDomainObjPtr lock whenever it hits +a piece of code which may sleep/wait, and re-acquire it after the +sleep/wait. Whenever an asynchronous job wants to talk to the +monitor, it needs to acquire nested job (a special kind of normal +job) to obtain exclusive access to the monitor. Since the virDomainObjPtr lock was dropped while waiting for the job condition, it is possible that the domain is no longer active @@ -132,6 +139,30 @@ To acquire the normal job condition +To acquire the agent job condition + + qemuDomainObjBeginAgentJob() +- Waits until there is no other agent job set +- Sets job.agentActive tp the job type + + qemuDomainObjEndAgentJob() +- Sets job.agentActive to 0 +- Signals on job.cond condition + + + +To acquire both normal and agent job condition + + qemuDomainObjBeginJobWithAgent() +- Waits until there is no normal and no agent job set +- Sets both job.active and job.agentActive with required job types + + qemuDomainObjEndJobWithAgent() +- Sets both job.active and job.agentActive to 0 +- Signals on job.cond condition + + + To acquire the asynchronous job condition qemuDomainObjBeginAsyncJob() @@ -247,6 +278,65 @@ Design patterns virDomainObjEndAPI(); + * Invoking an agent command on a virDomainObjPtr + + virDomainObjPtr obj; + qemuAgentPtr agent; + + obj = qemuDomObjFromDomain(dom); + + qemuDomainObjBeginAgentJob(obj, QEMU_AGENT_JOB_TYPE); + + ...do prep work... + + if (!qemuDomainAgentAvailable(obj, true)) + goto cleanup; + + agent = qemuDomainObjEnterAgent(obj); + qemuAgent(agent, ..); + qemuDomainObjExitAgent(obj, agent); + + ...do final work... + + qemuDomainObjEndAgentJob(obj); + virDomainObjEndAPI(); + + + * Invoking both monitor and agent commands on a virDomainObjPtr + + virDomainObjPtr obj; + qemuAgentPtr agent; + + obj = qemuDomObjFromDomain(dom); + + qemuDomainObjBeginJobWithAgent(obj, QEMU_JOB_TYPE, QEMU_AGENT_JOB_TYPE); + + if (!virDomainObjIsActive(dom)) + goto cleanup; + + ...do prep work... + + if (!qemuDomainAgentAvailable(obj, true)) + goto cleanup; + + agent =
[libvirt] [v2 RESEND 0/3] qemu: Allow concurrent access to monitor and guest agent
Obsoletes: https://www.redhat.com/archives/libvir-list/2018-June/msg01320.html This is the same version as the one linked above, but since I've merged my other patchset that touches the same area in the code reviewers will get merge conflicts when applying these. Rebase & resend. Michal Privoznik (3): qemu: Introduce qemuDomainAgentJob qemu: Introduce APIs for manipulating qemuDomainAgentJob qemu: Switch code to use new agent job APIs src/qemu/THREADS.txt | 112 +--- src/qemu/qemu_domain.c | 193 - src/qemu/qemu_domain.h | 30 src/qemu/qemu_driver.c | 91 ++- 4 files changed, 346 insertions(+), 80 deletions(-) -- 2.16.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [v2 RESEND 1/3] qemu: Introduce qemuDomainAgentJob
Introduce guest agent specific job categories to allow threads to
run agent monitor specific jobs while normal monitor jobs can
also be running.
Alter _qemuDomainJobObj in order to duplicate certain fields that
will be used for guest agent specific tasks to increase
concurrency and throughput and reduce serialization.
Signed-off-by: Michal Privoznik
---
src/qemu/qemu_domain.c | 6 ++
src/qemu/qemu_domain.h | 18 ++
2 files changed, 24 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 8a2d4750a5..91f3c6d236 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -93,6 +93,12 @@ VIR_ENUM_IMPL(qemuDomainJob, QEMU_JOB_LAST,
"async nested",
);
+VIR_ENUM_IMPL(qemuDomainAgentJob, QEMU_AGENT_JOB_LAST,
+ "none",
+ "query",
+ "modify",
+);
+
VIR_ENUM_IMPL(qemuDomainAsyncJob, QEMU_ASYNC_JOB_LAST,
"none",
"migration out",
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 9e2da0a37c..12573e5f86 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -82,6 +82,15 @@ typedef enum {
} qemuDomainJob;
VIR_ENUM_DECL(qemuDomainJob)
+typedef enum {
+QEMU_AGENT_JOB_NONE = 0,/* No agent job. */
+QEMU_AGENT_JOB_QUERY, /* Does not change state of domain */
+QEMU_AGENT_JOB_MODIFY, /* May change state of domain */
+
+QEMU_AGENT_JOB_LAST
+} qemuDomainAgentJob;
+VIR_ENUM_DECL(qemuDomainAgentJob)
+
/* Async job consists of a series of jobs that may change state. Independent
* jobs that do not change state (and possibly others if explicitly allowed by
* current async job) are allowed to be run even if async job is active.
@@ -158,11 +167,20 @@ typedef struct _qemuDomainJobObj qemuDomainJobObj;
typedef qemuDomainJobObj *qemuDomainJobObjPtr;
struct _qemuDomainJobObj {
virCond cond; /* Use to coordinate jobs */
+
+/* The following members are for QEMU_JOB_* */
qemuDomainJob active; /* Currently running job */
unsigned long long owner; /* Thread id which set current job */
const char *ownerAPI; /* The API which owns the job */
unsigned long long started; /* When the current job started */
+/* The following members are for QEMU_AGENT_JOB_* */
+qemuDomainAgentJob agentActive; /* Currently running agent job */
+unsigned long long agentOwner; /* Thread id which set current agent
job */
+const char *agentOwnerAPI; /* The API which owns the agent job */
+unsigned long long agentStarted;/* When the current agent job started
*/
+
+/* The following members are for QEMU_ASYNC_JOB_* */
virCond asyncCond; /* Use to coordinate with async jobs */
qemuDomainAsyncJob asyncJob;/* Currently active async job */
unsigned long long asyncOwner; /* Thread which set current async job
*/
--
2.16.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 4/5] Introduce VIR_CONNECT_GET_ALL_DOMAINS_STATS_NOWAIT
On 06/19/2018 01:25 AM, John Ferlan wrote: > > > On 06/15/2018 04:18 AM, Michal Privoznik wrote: >> https://bugzilla.redhat.com/show_bug.cgi?id=1552092 >> >> If there's a long running job it might cause us to wait 30 >> seconds before we give up acquiring the job. This is problematic >> to interactive applications that fetch stats repeatedly every few >> seconds. >> >> The solution is to introduce >> VIR_CONNECT_GET_ALL_DOMAINS_STATS_NOWAIT flag which tries to >> acquire job but does not wait if acquiring failed. >> >> Signed-off-by: Michal Privoznik >> --- >> include/libvirt/libvirt-domain.h | 2 ++ >> src/libvirt-domain.c | 12 >> src/qemu/qemu_driver.c | 15 --- >> 3 files changed, 26 insertions(+), 3 deletions(-) >> > > Reviewed-by: John Ferlan > > John > > Is the 30 seconds qemu specific? Could drop it from the commit message > - if you feel so compelled. > Yes & no. Each driver has its own timeout but all set it to 30 seconds: #define LXC_JOB_WAIT_TIME (1000ull * 30) #define LIBXL_JOB_WAIT_TIME (1000ull * 30) #define VZ_JOB_WAIT_TIME (1000 * 30) #define QEMU_JOB_WAIT_TIME (1000ull * 30) Therefore I rather keep 30seconds in the commit message. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
