[PATCH] virtiofs: rename member to 'openfiles' for clarity
by Adam Julis
New element 'openfiles' had confusing name. Since the patch with
this new element wasn't propagate yet, old name ('rlimit_nofile')
was changed.
...
<binary>
<openfiles max='122333'/>
</binary>
...
Signed-off-by: Adam Julis <ajulis(a)redhat.com>
---
docs/formatdomain.rst | 6 +++---
src/conf/domain_conf.c | 14 +++++++-------
src/conf/domain_conf.h | 2 +-
src/conf/schemas/domaincommon.rng | 4 ++--
src/qemu/qemu_virtiofs.c | 4 ++--
... vhost-user-fs-fd-openfiles.x86_64-latest.args} | 0
.../vhost-user-fs-fd-openfiles.x86_64-latest.xml | 1 +
...d-rlimit.xml => vhost-user-fs-fd-openfiles.xml} | 2 +-
.../vhost-user-fs-fd-rlimit.x86_64-latest.xml | 1 -
tests/qemuxmlconftest.c | 2 +-
10 files changed, 18 insertions(+), 18 deletions(-)
rename tests/qemuxmlconfdata/{vhost-user-fs-fd-rlimit.x86_64-latest.args => vhost-user-fs-fd-openfiles.x86_64-latest.args} (100%)
create mode 120000 tests/qemuxmlconfdata/vhost-user-fs-fd-openfiles.x86_64-latest.xml
rename tests/qemuxmlconfdata/{vhost-user-fs-fd-rlimit.xml => vhost-user-fs-fd-openfiles.xml} (97%)
delete mode 120000 tests/qemuxmlconfdata/vhost-user-fs-fd-rlimit.x86_64-latest.xml
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index 10584dfe83..c2a6d0b910 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -3734,10 +3734,10 @@ A directory on the host that can be accessed directly from the guest.
The thread pool helps increase the number of requests in flight when used with
storage that has a higher latency. However, it has an overhead, and so for
fast, low latency filesystems, it may be best to turn it off. ( :since:`Since 8.5.0` )
- Element ``rlimit_profile`` accepts one attribute ``size`` which defines the
+ Element ``openfiles`` accepts one attribute ``max`` which defines the
maximum number of file descriptors. Non-positive values are forbidden.
- Although numbers greater than 1M are allowed, the virtiofsd documentation
- states that in this case its set by virtiofsd to the 1M. ( :since:`Since 10.6.0` )
+ The upper bound on the number of open files is implementation defined.
+ ( :since:`Since 10.6.0` )
``source``
The resource on the host that is being accessed in the guest. The ``name``
attribute must be used with ``type='template'``, and the ``dir`` attribute
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6733857a3a..d1c59f7a91 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8868,7 +8868,7 @@ virDomainFSDefParseXML(virDomainXMLOption *xmlopt,
g_autofree char *queue_size = virXPathString("string(./driver/@queue)", ctxt);
g_autofree char *binary = virXPathString("string(./binary/@path)", ctxt);
g_autofree char *thread_pool_size = virXPathString("string(./binary/thread_pool/@size)", ctxt);
- g_autofree char *rlimit_nofile = virXPathString("string(./binary/rlimit_nofile/@size)", ctxt);
+ g_autofree char *openfiles = virXPathString("string(./binary/openfiles/@max)", ctxt);
xmlNodePtr binary_node = virXPathNode("./binary", ctxt);
xmlNodePtr binary_lock_node = virXPathNode("./binary/lock", ctxt);
xmlNodePtr binary_cache_node = virXPathNode("./binary/cache", ctxt);
@@ -8892,11 +8892,11 @@ virDomainFSDefParseXML(virDomainXMLOption *xmlopt,
goto error;
}
- if (rlimit_nofile &&
- virStrToLong_ull(rlimit_nofile, NULL, 10, &def->rlimit_nofile) < 0) {
+ if (openfiles &&
+ virStrToLong_ull(openfiles, NULL, 10, &def->openfiles) < 0) {
virReportError(VIR_ERR_XML_ERROR,
- _("cannot parse rlimit_nofile '%1$s' for virtiofs"),
- rlimit_nofile);
+ _("cannot parse openfiles '%1$s' for virtiofs"),
+ openfiles);
goto error;
}
@@ -23424,8 +23424,8 @@ virDomainFSDefFormat(virBuffer *buf,
if (def->thread_pool_size >= 0)
virBufferAsprintf(&binaryBuf, "<thread_pool size='%d'/>\n", def->thread_pool_size);
- if (def->rlimit_nofile > 0)
- virBufferAsprintf(&binaryBuf, "<rlimit_nofile size='%lld'/>\n", def->rlimit_nofile);
+ if (def->openfiles > 0)
+ virBufferAsprintf(&binaryBuf, "<openfiles max='%lld'/>\n", def->openfiles);
}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 8283493dfc..6972f6ae9b 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -890,7 +890,7 @@ struct _virDomainFSDef {
bool symlinksResolved;
char *binary;
unsigned long long queue_size;
- unsigned long long rlimit_nofile;
+ unsigned long long openfiles;
virTristateSwitch xattr;
virDomainFSCacheMode cache;
virTristateSwitch posix_lock;
diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng
index ab5374d5f0..8dae6416e9 100644
--- a/src/conf/schemas/domaincommon.rng
+++ b/src/conf/schemas/domaincommon.rng
@@ -3381,9 +3381,9 @@
</element>
</optional>
<optional>
- <element name="rlimit_nofile">
+ <element name="openfiles">
<optional>
- <attribute name="size">
+ <attribute name="max">
<data type="positiveInteger"/>
</attribute>
</optional>
diff --git a/src/qemu/qemu_virtiofs.c b/src/qemu/qemu_virtiofs.c
index 703f1226a2..a8f2416273 100644
--- a/src/qemu/qemu_virtiofs.c
+++ b/src/qemu/qemu_virtiofs.c
@@ -194,8 +194,8 @@ qemuVirtioFSBuildCommandLine(virQEMUDriverConfig *cfg,
if (fs->thread_pool_size >= 0)
virCommandAddArgFormat(cmd, "--thread-pool-size=%i", fs->thread_pool_size);
- if (fs->rlimit_nofile > 0)
- virCommandAddArgFormat(cmd, "--rlimit-nofile=%llu", fs->rlimit_nofile);
+ if (fs->openfiles > 0)
+ virCommandAddArgFormat(cmd, "--rlimit-nofile=%llu", fs->openfiles);
if (cfg->virtiofsdDebug) {
if (virBitmapIsBitSet(fs->caps, QEMU_VHOST_USER_FS_FEATURE_SEPARATE_OPTIONS))
diff --git a/tests/qemuxmlconfdata/vhost-user-fs-fd-rlimit.x86_64-latest.args b/tests/qemuxmlconfdata/vhost-user-fs-fd-openfiles.x86_64-latest.args
similarity index 100%
rename from tests/qemuxmlconfdata/vhost-user-fs-fd-rlimit.x86_64-latest.args
rename to tests/qemuxmlconfdata/vhost-user-fs-fd-openfiles.x86_64-latest.args
diff --git a/tests/qemuxmlconfdata/vhost-user-fs-fd-openfiles.x86_64-latest.xml b/tests/qemuxmlconfdata/vhost-user-fs-fd-openfiles.x86_64-latest.xml
new file mode 120000
index 0000000000..74761b0d0f
--- /dev/null
+++ b/tests/qemuxmlconfdata/vhost-user-fs-fd-openfiles.x86_64-latest.xml
@@ -0,0 +1 @@
+vhost-user-fs-fd-openfiles.xml
\ No newline at end of file
diff --git a/tests/qemuxmlconfdata/vhost-user-fs-fd-rlimit.xml b/tests/qemuxmlconfdata/vhost-user-fs-fd-openfiles.xml
similarity index 97%
rename from tests/qemuxmlconfdata/vhost-user-fs-fd-rlimit.xml
rename to tests/qemuxmlconfdata/vhost-user-fs-fd-openfiles.xml
index 2983d3f275..3e925a8c8b 100644
--- a/tests/qemuxmlconfdata/vhost-user-fs-fd-rlimit.xml
+++ b/tests/qemuxmlconfdata/vhost-user-fs-fd-openfiles.xml
@@ -32,7 +32,7 @@
<cache mode='always'/>
<sandbox mode='chroot'/>
<thread_pool size='16'/>
- <rlimit_nofile size='122333'/>
+ <openfiles max='122333'/>
</binary>
<idmap>
<uid start='0' target='100000' count='65535'/>
diff --git a/tests/qemuxmlconfdata/vhost-user-fs-fd-rlimit.x86_64-latest.xml b/tests/qemuxmlconfdata/vhost-user-fs-fd-rlimit.x86_64-latest.xml
deleted file mode 120000
index 68c4e8a482..0000000000
--- a/tests/qemuxmlconfdata/vhost-user-fs-fd-rlimit.x86_64-latest.xml
+++ /dev/null
@@ -1 +0,0 @@
-vhost-user-fs-fd-rlimit.xml
\ No newline at end of file
diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c
index cc984440ea..bf88bd2f8d 100644
--- a/tests/qemuxmlconftest.c
+++ b/tests/qemuxmlconftest.c
@@ -2816,7 +2816,7 @@ mymain(void)
DO_TEST_CAPS_ARCH_LATEST("launch-security-s390-pv", "s390x");
DO_TEST_CAPS_LATEST("vhost-user-fs-fd-memory");
- DO_TEST_CAPS_LATEST("vhost-user-fs-fd-rlimit");
+ DO_TEST_CAPS_LATEST("vhost-user-fs-fd-openfiles");
DO_TEST_CAPS_LATEST("vhost-user-fs-hugepages");
DO_TEST_CAPS_LATEST_PARSE_ERROR("vhost-user-fs-readonly");
--
2.45.2
8 months, 3 weeks
[PATCH] qemu_hotplug: Do not allow absent values in rom settings
by Kristina Hanicova
If there are absent values in an already existing element
specifying rom settings, we simply use the old ones. This
behaviour is not desired, as users might think that deleting the
element from XML would delete the setting (because the hotplug
succeeds) - which does not happen. Because of that, we should not
accept an interface without elements that cannot be changed.
Therefore, we should not allow absent values for already existing
rom setting during hotplug.
Resolves: https://issues.redhat.com/browse/RHEL-7109
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 1f4620d833..7f158ddcd5 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3838,16 +3838,12 @@ qemuDomainChangeNet(virQEMUDriver *driver,
/* device alias is checked already in virDomainDefCompatibleDevice */
- if (newdev->info.rombar == VIR_TRISTATE_SWITCH_ABSENT)
- newdev->info.rombar = olddev->info.rombar;
if (olddev->info.rombar != newdev->info.rombar) {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
_("cannot modify network device rom bar setting"));
goto cleanup;
}
- if (!newdev->info.romfile)
- newdev->info.romfile = g_strdup(olddev->info.romfile);
if (STRNEQ_NULLABLE(olddev->info.romfile, newdev->info.romfile)) {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
_("cannot modify network rom file"));
@@ -3860,8 +3856,6 @@ qemuDomainChangeNet(virQEMUDriver *driver,
goto cleanup;
}
- if (newdev->info.romenabled == VIR_TRISTATE_BOOL_ABSENT)
- newdev->info.romenabled = olddev->info.romenabled;
if (olddev->info.romenabled != newdev->info.romenabled) {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
_("cannot modify network device rom enabled setting"));
--
2.45.1
8 months, 3 weeks
[PATCH v3] qemu: add a monitor to /proc/$pid when killing times out
by Boris Fiuczynski
In cases when a QEMU process takes longer than the time sigterm and
sigkill are issued to kill the process do not simply fail and leave the
VM in state VIR_DOMAIN_SHUTDOWN until the daemon stops. Instead set up
an fd on /proc/$pid and get notified when the QEMU process finally has
terminated to cleanup the VM state.
Resolves: https://issues.redhat.com/browse/RHEL-28819
Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
---
src/qemu/qemu_domain.c | 8 +++
src/qemu/qemu_domain.h | 2 +
src/qemu/qemu_driver.c | 18 ++++++
src/qemu/qemu_process.c | 124 ++++++++++++++++++++++++++++++++++++++--
src/qemu/qemu_process.h | 1 +
5 files changed, 148 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 2134b11038..8147ff02fd 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1889,6 +1889,11 @@ qemuDomainObjPrivateFree(void *data)
virChrdevFree(priv->devs);
+ if (priv->pidMonitored >= 0) {
+ virEventRemoveHandle(priv->pidMonitored);
+ priv->pidMonitored = -1;
+ }
+
/* This should never be non-NULL if we get here, but just in case... */
if (priv->mon) {
VIR_ERROR(_("Unexpected QEMU monitor still active during domain deletion"));
@@ -1934,6 +1939,8 @@ qemuDomainObjPrivateAlloc(void *opaque)
priv->blockjobs = virHashNew(virObjectUnref);
priv->fds = virHashNew(g_object_unref);
+ priv->pidMonitored = -1;
+
/* agent commands block by default, user can choose different behavior */
priv->agentTimeout = VIR_DOMAIN_AGENT_RESPONSE_TIMEOUT_BLOCK;
priv->migMaxBandwidth = QEMU_DOMAIN_MIG_BANDWIDTH_MAX;
@@ -11680,6 +11687,7 @@ qemuProcessEventFree(struct qemuProcessEvent *event)
case QEMU_PROCESS_EVENT_RESET:
case QEMU_PROCESS_EVENT_NBDKIT_EXITED:
case QEMU_PROCESS_EVENT_MONITOR_EOF:
+ case QEMU_PROCESS_EVENT_SHUTDOWN_COMPLETED:
case QEMU_PROCESS_EVENT_LAST:
break;
}
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index d777559119..a5092dd7f0 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -119,6 +119,7 @@ struct _qemuDomainObjPrivate {
bool beingDestroyed;
char *pidfile;
+ int pidMonitored;
virDomainPCIAddressSet *pciaddrs;
virDomainUSBAddressSet *usbaddrs;
@@ -469,6 +470,7 @@ typedef enum {
QEMU_PROCESS_EVENT_UNATTENDED_MIGRATION,
QEMU_PROCESS_EVENT_RESET,
QEMU_PROCESS_EVENT_NBDKIT_EXITED,
+ QEMU_PROCESS_EVENT_SHUTDOWN_COMPLETED,
QEMU_PROCESS_EVENT_LAST
} qemuProcessEventType;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 9f3013e231..6b1e4084f6 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4041,6 +4041,21 @@ processNbdkitExitedEvent(virDomainObj *vm,
}
+static void
+processShutdownCompletedEvent(virQEMUDriver *driver,
+ virDomainObj *vm)
+{
+ if (virDomainObjBeginJob(vm, VIR_JOB_MODIFY) < 0)
+ return;
+
+ if (virDomainObjIsActive(vm))
+ qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_UNKNOWN,
+ VIR_ASYNC_JOB_NONE, 0);
+
+ virDomainObjEndJob(vm);
+}
+
+
static void qemuProcessEventHandler(void *data, void *opaque)
{
struct qemuProcessEvent *processEvent = data;
@@ -4101,6 +4116,9 @@ static void qemuProcessEventHandler(void *data, void *opaque)
case QEMU_PROCESS_EVENT_NBDKIT_EXITED:
processNbdkitExitedEvent(vm, processEvent->data);
break;
+ case QEMU_PROCESS_EVENT_SHUTDOWN_COMPLETED:
+ processShutdownCompletedEvent(driver, vm);
+ break;
case QEMU_PROCESS_EVENT_LAST:
break;
}
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 25dfd04272..c6f7d34168 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -25,6 +25,9 @@
#include <unistd.h>
#include <signal.h>
#include <sys/stat.h>
+#if WITH_SYS_SYSCALL_H
+# include <sys/syscall.h>
+#endif
#if defined(__linux__)
# include <linux/capability.h>
#elif defined(__FreeBSD__)
@@ -8387,9 +8390,114 @@ qemuProcessCreatePretendCmdBuild(virDomainObj *vm,
}
+#if WITH_SYS_SYSCALL_H && defined(SYS_pidfd_open)
+typedef struct {
+ virDomainObj *vm;
+ int pidfd;
+} qemuProcessInShutdownEventData;
+
+
+static qemuProcessInShutdownEventData*
+qemuProcessInShutdownEventDataNew(virDomainObj *vm, int pidfd)
+{
+ qemuProcessInShutdownEventData *d = g_new(qemuProcessInShutdownEventData, 1);
+ d->vm = virObjectRef(vm);
+ d->pidfd = pidfd;
+ return d;
+}
+
+
+static void
+qemuProcessInShutdownEventDataFree(qemuProcessInShutdownEventData *d)
+{
+ virObjectUnref(d->vm);
+ VIR_FORCE_CLOSE(d->pidfd);
+ g_free(d);
+}
+
+
+static void
+qemuProcessInShutdownPidfdCb(int watch,
+ int fd,
+ int events G_GNUC_UNUSED,
+ void *opaque)
+{
+ qemuProcessInShutdownEventData *data = opaque;
+ virDomainObj *vm = data->vm;
+
+ VIR_DEBUG("vm=%p name=%s pid=%lld fd=%d",
+ vm, vm->def->name, (long long)vm->pid, fd);
+
+ virEventRemoveHandle(watch);
+
+ virObjectLock(vm);
+
+ VIR_INFO("QEMU process %lld finally completed termination",
+ (long long)vm->pid);
+
+ QEMU_DOMAIN_PRIVATE(vm)->pidMonitored = -1;
+ qemuProcessEventSubmit(vm, QEMU_PROCESS_EVENT_SHUTDOWN_COMPLETED,
+ 0, 0, NULL);
+
+ virObjectUnlock(vm);
+}
+#endif /* WITH_SYS_SYSCALL_H && defined(SYS_pidfd_open) */
+
+
+static int
+qemuProcessInShutdownStartMonitor(virDomainObj *vm)
+{
+#if WITH_SYS_SYSCALL_H && defined(SYS_pidfd_open)
+ qemuDomainObjPrivate *priv = vm->privateData;
+ qemuProcessInShutdownEventData *data;
+ int pidfd;
+ int ret = -1;
+
+ VIR_DEBUG("vm=%p name=%s pid=%lld pidMonitored=%d",
+ vm, vm->def->name, (long long)vm->pid,
+ priv->pidMonitored);
+
+ if (priv->pidMonitored >= 0) {
+ VIR_DEBUG("Monitoring qemu in-shutdown process %i already set up", vm->pid);
+ goto cleanup;
+ }
+
+ pidfd = syscall(SYS_pidfd_open, vm->pid, 0);
+ if (pidfd < 0) {
+ if (errno == ESRCH) /* process has already terminated */
+ ret = 1;
+ goto cleanup;
+ }
+
+ data = qemuProcessInShutdownEventDataNew(vm, pidfd);
+ if ((priv->pidMonitored = virEventAddHandle(pidfd,
+ VIR_EVENT_HANDLE_READABLE,
+ qemuProcessInShutdownPidfdCb,
+ data,
+ (virFreeCallback)qemuProcessInShutdownEventDataFree)) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("failed to monitor qemu in-shutdown process %1$i"),
+ vm->pid);
+ qemuProcessInShutdownEventDataFree(data);
+ goto cleanup;
+ }
+ VIR_DEBUG("Monitoring qemu in-shutdown process %i for termination", vm->pid);
+ ret = 0;
+
+ cleanup:
+ return ret;
+#else /* !WITH_SYS_SYSCALL_H || !defined(SYS_pidfd_open) */
+ VIR_DEBUG("Monitoring qemu process %i not implemented", vm->pid);
+ return -1;
+#endif /* !WITH_SYS_SYSCALL_H || !defined(SYS_pidfd_open) */
+}
+
+
int
qemuProcessKill(virDomainObj *vm, unsigned int flags)
{
+ int ret = -1;
+
VIR_DEBUG("vm=%p name=%s pid=%lld flags=0x%x",
vm, vm->def->name,
(long long)vm->pid, flags);
@@ -8410,10 +8518,16 @@ qemuProcessKill(virDomainObj *vm, unsigned int flags)
/* Request an extra delay of two seconds per current nhostdevs
* to be safe against stalls by the kernel freeing up the resources */
- return virProcessKillPainfullyDelay(vm->pid,
- !!(flags & VIR_QEMU_PROCESS_KILL_FORCE),
- vm->def->nhostdevs * 2,
- false);
+ ret = virProcessKillPainfullyDelay(vm->pid,
+ !!(flags & VIR_QEMU_PROCESS_KILL_FORCE),
+ vm->def->nhostdevs * 2,
+ false);
+
+ if (ret < 0 && (flags & VIR_QEMU_PROCESS_KILL_MONITOR_ON_ERROR))
+ if (qemuProcessInShutdownStartMonitor(vm) == 1)
+ ret = 0; /* process termination detected */
+
+ return ret;
}
@@ -8438,7 +8552,7 @@ qemuProcessBeginStopJob(virDomainObj *vm,
* cleared inside qemuProcessStop */
priv->beingDestroyed = true;
- if (qemuProcessKill(vm, killFlags) < 0)
+ if (qemuProcessKill(vm, killFlags|VIR_QEMU_PROCESS_KILL_MONITOR_ON_ERROR) < 0)
goto error;
/* Wake up anything waiting on domain condition */
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index cb67bfcd2d..2324aeb7bd 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -180,6 +180,7 @@ typedef enum {
VIR_QEMU_PROCESS_KILL_FORCE = 1 << 0,
VIR_QEMU_PROCESS_KILL_NOWAIT = 1 << 1,
VIR_QEMU_PROCESS_KILL_NOCHECK = 1 << 2, /* bypass the running vm check */
+ VIR_QEMU_PROCESS_KILL_MONITOR_ON_ERROR = 1 << 3, /* on error enable process monitor */
} virQemuProcessKillMode;
int qemuProcessKill(virDomainObj *vm, unsigned int flags);
--
2.45.0
8 months, 3 weeks
[PATCH v3] virsh: Provide completer for some pool-X-as commands
by Abhiram Tilak
From: notpua <atp(a)tutamail.com>
Provides completers for auth-type and source-format commands for
virsh pool-create-as and pool-define-as commands. Use Empty completers
for options where completions are not required.
Related Issue: https://gitlab.com/libvirt/libvirt/-/issues/9
Signed-off-by: Abhiram Tilak <atp.exp(a)gmail.com>
---
Changes in v2:
- Fix all formatting errors
- Change some options using Empty completers, to use
LocalPath completers.
- Add completers for AdapterName and AdapterParent using information
from node devices.
Changes in v3:
- Call virshNodeDeviceNameComplete with modified flags instead of
duplicating the whole implementation.
src/libvirt_private.syms | 2 ++
tools/virsh-completer-pool.c | 70 ++++++++++++++++++++++++++++++++++++
tools/virsh-completer-pool.h | 20 +++++++++++
tools/virsh-pool.c | 9 +++++
4 files changed, 101 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c35366c9e1..6ba1e8e2c5 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1123,6 +1123,8 @@ virStorageAuthDefCopy;
virStorageAuthDefFormat;
virStorageAuthDefFree;
virStorageAuthDefParse;
+virStorageAuthTypeFromString;
+virStorageAuthTypeToString;
virStorageFileFeatureTypeFromString;
virStorageFileFeatureTypeToString;
virStorageFileFormatTypeFromString;
diff --git a/tools/virsh-completer-pool.c b/tools/virsh-completer-pool.c
index 3568bb985b..ba7855fdba 100644
--- a/tools/virsh-completer-pool.c
+++ b/tools/virsh-completer-pool.c
@@ -23,6 +23,7 @@
#include "virsh-completer-pool.h"
#include "virsh-util.h"
#include "conf/storage_conf.h"
+#include "conf/node_device_conf.h"
#include "virsh-pool.h"
#include "virsh.h"
@@ -106,3 +107,72 @@ virshPoolTypeCompleter(vshControl *ctl,
return virshCommaStringListComplete(type_str, (const char **)tmp);
}
+
+
+char **
+virshPoolFormatCompleter(vshControl *ctl G_GNUC_UNUSED,
+ const vshCmd *cmd G_GNUC_UNUSED,
+ unsigned int flags)
+{
+ size_t i = 0;
+ size_t j = 0;
+ g_auto(GStrv) tmp = NULL;
+ size_t nformats = VIR_STORAGE_POOL_FS_LAST + VIR_STORAGE_POOL_NETFS_LAST +
+ VIR_STORAGE_POOL_DISK_LAST + VIR_STORAGE_POOL_LOGICAL_LAST;
+
+ virCheckFlags(0, NULL);
+
+ tmp = g_new0(char *, nformats + 1);
+
+ /* Club all PoolFormats for completion */
+ for (i = 0; i < VIR_STORAGE_POOL_FS_LAST; i++)
+ tmp[j++] = g_strdup(virStoragePoolFormatFileSystemTypeToString(i));
+
+ for (i = 0; i < VIR_STORAGE_POOL_NETFS_LAST; i++)
+ tmp[j++] = g_strdup(virStoragePoolFormatFileSystemNetTypeToString(i));
+
+ for (i = 1; i < VIR_STORAGE_POOL_DISK_LAST; i++)
+ tmp[j++] = g_strdup(virStoragePoolFormatDiskTypeToString(i));
+
+ for (i = 1; i < VIR_STORAGE_POOL_LOGICAL_LAST; i++)
+ tmp[j++] = g_strdup(virStoragePoolFormatLogicalTypeToString(i));
+
+ return g_steal_pointer(&tmp);
+}
+
+
+char **
+virshPoolAuthTypeCompleter(vshControl *ctl G_GNUC_UNUSED,
+ const vshCmd *cmd G_GNUC_UNUSED,
+ unsigned int flags)
+{
+ size_t i = 0;
+ g_auto(GStrv) tmp = NULL;
+
+ virCheckFlags(0, NULL);
+
+ tmp = g_new0(char *, VIR_STORAGE_AUTH_TYPE_LAST + 1);
+
+ for (i = 0; i < VIR_STORAGE_AUTH_TYPE_LAST; i++)
+ tmp[i] = g_strdup(virStorageAuthTypeToString(i));
+
+ return g_steal_pointer(&tmp);
+}
+
+
+char **
+virshAdapterNameCompleter(vshControl *ctl,
+ const vshCmd *cmd, unsigned int flags)
+{
+ flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI_HOST;
+ return virshNodeDeviceNameCompleter(ctl, cmd, flags);
+}
+
+
+char **
+virshAdapterParentCompleter(vshControl *ctl,
+ const vshCmd *cmd, unsigned int flags)
+{
+ flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_VPORTS;
+ return virshNodeDeviceNameCompleter(ctl, cmd, flags);
+}
diff --git a/tools/virsh-completer-pool.h b/tools/virsh-completer-pool.h
index bff3e5742b..eccc08a73f 100644
--- a/tools/virsh-completer-pool.h
+++ b/tools/virsh-completer-pool.h
@@ -40,3 +40,23 @@ char **
virshPoolTypeCompleter(vshControl *ctl,
const vshCmd *cmd,
unsigned int flags);
+
+char **
+virshPoolFormatCompleter(vshControl *ctl,
+ const vshCmd *cmd,
+ unsigned int flags);
+
+char **
+virshPoolAuthTypeCompleter(vshControl *ctl,
+ const vshCmd *cmd,
+ unsigned int flags);
+
+char **
+virshAdapterNameCompleter(vshControl *ctl,
+ const vshCmd *cmd,
+ unsigned int flags);
+
+char **
+virshAdapterParentCompleter(vshControl *ctl,
+ const vshCmd *cmd,
+ unsigned int flags);
diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c
index f9aad8ded0..0cbd1417e6 100644
--- a/tools/virsh-pool.c
+++ b/tools/virsh-pool.c
@@ -80,31 +80,37 @@
{.name = "source-path", \
.type = VSH_OT_STRING, \
.unwanted_positional = true, \
+ .completer = virshCompletePathLocalExisting, \
.help = N_("source path for underlying storage") \
}, \
{.name = "source-dev", \
.type = VSH_OT_STRING, \
.unwanted_positional = true, \
+ .completer = virshCompletePathLocalExisting, \
.help = N_("source device for underlying storage") \
}, \
{.name = "source-name", \
.type = VSH_OT_STRING, \
.unwanted_positional = true, \
+ .completer = virshCompleteEmpty, \
.help = N_("source name for underlying storage") \
}, \
{.name = "target", \
.type = VSH_OT_STRING, \
.unwanted_positional = true, \
+ .completer = virshCompletePathLocalExisting, \
.help = N_("target for underlying storage") \
}, \
{.name = "source-format", \
.type = VSH_OT_STRING, \
.unwanted_positional = true, \
+ .completer = virshPoolFormatCompleter, \
.help = N_("format for underlying storage") \
}, \
{.name = "auth-type", \
.type = VSH_OT_STRING, \
.unwanted_positional = true, \
+ .completer = virshPoolAuthTypeCompleter, \
.help = N_("auth type to be used for underlying storage") \
}, \
{.name = "auth-username", \
@@ -126,6 +132,7 @@
{.name = "adapter-name", \
.type = VSH_OT_STRING, \
.unwanted_positional = true, \
+ .completer = virshAdapterNameCompleter, \
.help = N_("adapter name to be used for underlying storage") \
}, \
{.name = "adapter-wwnn", \
@@ -141,6 +148,7 @@
{.name = "adapter-parent", \
.type = VSH_OT_STRING, \
.unwanted_positional = true, \
+ .completer = virshAdapterParentCompleter, \
.help = N_("adapter parent scsi_hostN to be used for underlying vHBA storage") \
}, \
{.name = "adapter-parent-wwnn", \
@@ -161,6 +169,7 @@
{.name = "source-protocol-ver", \
.type = VSH_OT_STRING, \
.unwanted_positional = true, \
+ .completer = virshCompleteEmpty, \
.help = N_("nfsvers value for NFS pool mount option") \
}, \
{.name = "source-initiator", \
--
2.39.2
8 months, 3 weeks
[PATCH v2] virsh: Provide completer for some pool-X-as commands
by Abhiram Tilak
Provides completers for auth-type and source-format commands for
virsh pool-create-as and pool-define-as commands. Use Empty completers
for options where completions are not required.
Related Issue: https://gitlab.com/libvirt/libvirt/-/issues/9
Signed-off-by: Abhiram Tilak <atp.exp(a)gmail.com>
---
Changes in v2:
- Fix all formatting errors
- Change some options using Empty completers, to use
LocalPath completers.
- Add completers for AdapterName and AdapterParent using information
from node devices.
src/libvirt_private.syms | 2 +
tools/virsh-completer-pool.c | 128 +++++++++++++++++++++++++++++++++++
tools/virsh-completer-pool.h | 20 ++++++
tools/virsh-pool.c | 9 +++
4 files changed, 159 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index f0f7aa8654..fcb0ef7afe 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1117,6 +1117,8 @@ virStorageAuthDefCopy;
virStorageAuthDefFormat;
virStorageAuthDefFree;
virStorageAuthDefParse;
+virStorageAuthTypeFromString;
+virStorageAuthTypeToString;
virStorageFileFeatureTypeFromString;
virStorageFileFeatureTypeToString;
virStorageFileFormatTypeFromString;
diff --git a/tools/virsh-completer-pool.c b/tools/virsh-completer-pool.c
index 3568bb985b..7db2a20347 100644
--- a/tools/virsh-completer-pool.c
+++ b/tools/virsh-completer-pool.c
@@ -23,6 +23,7 @@
#include "virsh-completer-pool.h"
#include "virsh-util.h"
#include "conf/storage_conf.h"
+#include "conf/node_device_conf.h"
#include "virsh-pool.h"
#include "virsh.h"
@@ -106,3 +107,130 @@ virshPoolTypeCompleter(vshControl *ctl,
return virshCommaStringListComplete(type_str, (const char **)tmp);
}
+
+
+char **
+virshPoolFormatCompleter(vshControl *ctl G_GNUC_UNUSED,
+ const vshCmd *cmd G_GNUC_UNUSED,
+ unsigned int flags)
+{
+ size_t i = 0;
+ size_t j = 0;
+ g_auto(GStrv) tmp = NULL;
+ size_t nformats = VIR_STORAGE_POOL_FS_LAST + VIR_STORAGE_POOL_NETFS_LAST +
+ VIR_STORAGE_POOL_DISK_LAST + VIR_STORAGE_POOL_LOGICAL_LAST;
+
+ virCheckFlags(0, NULL);
+
+ tmp = g_new0(char *, nformats + 1);
+
+ /* Club all PoolFormats for completion */
+ for (i = 0; i < VIR_STORAGE_POOL_FS_LAST; i++)
+ tmp[j++] = g_strdup(virStoragePoolFormatFileSystemTypeToString(i));
+
+ for (i = 0; i < VIR_STORAGE_POOL_NETFS_LAST; i++)
+ tmp[j++] = g_strdup(virStoragePoolFormatFileSystemNetTypeToString(i));
+
+ for (i = 1; i < VIR_STORAGE_POOL_DISK_LAST; i++)
+ tmp[j++] = g_strdup(virStoragePoolFormatDiskTypeToString(i));
+
+ for (i = 1; i < VIR_STORAGE_POOL_LOGICAL_LAST; i++)
+ tmp[j++] = g_strdup(virStoragePoolFormatLogicalTypeToString(i));
+
+ return g_steal_pointer(&tmp);
+}
+
+
+char **
+virshPoolAuthTypeCompleter(vshControl *ctl G_GNUC_UNUSED,
+ const vshCmd *cmd G_GNUC_UNUSED,
+ unsigned int flags)
+{
+ size_t i = 0;
+ g_auto(GStrv) tmp = NULL;
+
+ virCheckFlags(0, NULL);
+
+ tmp = g_new0(char *, VIR_STORAGE_AUTH_TYPE_LAST + 1);
+
+ for (i = 0; i < VIR_STORAGE_AUTH_TYPE_LAST; i++)
+ tmp[i] = g_strdup(virStorageAuthTypeToString(i));
+
+ return g_steal_pointer(&tmp);
+}
+
+
+char **
+virshAdapterNameCompleter(vshControl *ctl,
+ const vshCmd *cmd G_GNUC_UNUSED,
+ unsigned int flags)
+{
+ virshControl *priv = ctl->privData;
+ virNodeDevicePtr *devs = NULL;
+ int ndevs = 0;
+ size_t i = 0;
+ char **ret = NULL;
+ g_auto(GStrv) tmp = NULL;
+
+ virCheckFlags(0, NULL);
+
+ if (!priv->conn || virConnectIsAlive(priv->conn) <= 0)
+ return NULL;
+
+ flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI_HOST;
+ if ((ndevs = virConnectListAllNodeDevices(priv->conn, &devs, flags)) < 0)
+ return NULL;
+
+ tmp = g_new0(char *, ndevs + 1);
+
+ for (i = 0; i < ndevs; i++) {
+ const char *name = virNodeDeviceGetName(devs[i]);
+
+ tmp[i] = g_strdup(name);
+ }
+
+ ret = g_steal_pointer(&tmp);
+
+ for (i = 0; i < ndevs; i++)
+ virshNodeDeviceFree(devs[i]);
+ g_free(devs);
+ return ret;
+}
+
+
+char **
+virshAdapterParentCompleter(vshControl *ctl,
+ const vshCmd *cmd G_GNUC_UNUSED,
+ unsigned int flags)
+{
+ virshControl *priv = ctl->privData;
+ virNodeDevicePtr *devs = NULL;
+ int ndevs = 0;
+ size_t i = 0;
+ char **ret = NULL;
+ g_auto(GStrv) tmp = NULL;
+
+ virCheckFlags(0, NULL);
+
+ if (!priv->conn || virConnectIsAlive(priv->conn) <= 0)
+ return NULL;
+
+ flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_VPORTS;
+ if ((ndevs = virConnectListAllNodeDevices(priv->conn, &devs, flags)) < 0)
+ return NULL;
+
+ tmp = g_new0(char *, ndevs + 1);
+
+ for (i = 0; i < ndevs; i++) {
+ const char *name = virNodeDeviceGetName(devs[i]);
+
+ tmp[i] = g_strdup(name);
+ }
+
+ ret = g_steal_pointer(&tmp);
+
+ for (i = 0; i < ndevs; i++)
+ virshNodeDeviceFree(devs[i]);
+ g_free(devs);
+ return ret;
+}
diff --git a/tools/virsh-completer-pool.h b/tools/virsh-completer-pool.h
index bff3e5742b..eccc08a73f 100644
--- a/tools/virsh-completer-pool.h
+++ b/tools/virsh-completer-pool.h
@@ -40,3 +40,23 @@ char **
virshPoolTypeCompleter(vshControl *ctl,
const vshCmd *cmd,
unsigned int flags);
+
+char **
+virshPoolFormatCompleter(vshControl *ctl,
+ const vshCmd *cmd,
+ unsigned int flags);
+
+char **
+virshPoolAuthTypeCompleter(vshControl *ctl,
+ const vshCmd *cmd,
+ unsigned int flags);
+
+char **
+virshAdapterNameCompleter(vshControl *ctl,
+ const vshCmd *cmd,
+ unsigned int flags);
+
+char **
+virshAdapterParentCompleter(vshControl *ctl,
+ const vshCmd *cmd,
+ unsigned int flags);
diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c
index f9aad8ded0..0cbd1417e6 100644
--- a/tools/virsh-pool.c
+++ b/tools/virsh-pool.c
@@ -80,31 +80,37 @@
{.name = "source-path", \
.type = VSH_OT_STRING, \
.unwanted_positional = true, \
+ .completer = virshCompletePathLocalExisting, \
.help = N_("source path for underlying storage") \
}, \
{.name = "source-dev", \
.type = VSH_OT_STRING, \
.unwanted_positional = true, \
+ .completer = virshCompletePathLocalExisting, \
.help = N_("source device for underlying storage") \
}, \
{.name = "source-name", \
.type = VSH_OT_STRING, \
.unwanted_positional = true, \
+ .completer = virshCompleteEmpty, \
.help = N_("source name for underlying storage") \
}, \
{.name = "target", \
.type = VSH_OT_STRING, \
.unwanted_positional = true, \
+ .completer = virshCompletePathLocalExisting, \
.help = N_("target for underlying storage") \
}, \
{.name = "source-format", \
.type = VSH_OT_STRING, \
.unwanted_positional = true, \
+ .completer = virshPoolFormatCompleter, \
.help = N_("format for underlying storage") \
}, \
{.name = "auth-type", \
.type = VSH_OT_STRING, \
.unwanted_positional = true, \
+ .completer = virshPoolAuthTypeCompleter, \
.help = N_("auth type to be used for underlying storage") \
}, \
{.name = "auth-username", \
@@ -126,6 +132,7 @@
{.name = "adapter-name", \
.type = VSH_OT_STRING, \
.unwanted_positional = true, \
+ .completer = virshAdapterNameCompleter, \
.help = N_("adapter name to be used for underlying storage") \
}, \
{.name = "adapter-wwnn", \
@@ -141,6 +148,7 @@
{.name = "adapter-parent", \
.type = VSH_OT_STRING, \
.unwanted_positional = true, \
+ .completer = virshAdapterParentCompleter, \
.help = N_("adapter parent scsi_hostN to be used for underlying vHBA storage") \
}, \
{.name = "adapter-parent-wwnn", \
@@ -161,6 +169,7 @@
{.name = "source-protocol-ver", \
.type = VSH_OT_STRING, \
.unwanted_positional = true, \
+ .completer = virshCompleteEmpty, \
.help = N_("nfsvers value for NFS pool mount option") \
}, \
{.name = "source-initiator", \
--
2.39.2
8 months, 3 weeks
[PATCH] qemuProcessStop: Don't unlock domain until most of the cleanup is done
by Michal Privoznik
Imagine two threads. Thread A is executing qemuProcessStop() and
thread B is executing qemuDomainCreateXML(). To make things more
interesting, the domain XML passed to qemuDomainCreateXML matches
name + UUID of that the virDomainObj that qemuProcessStop() is
currently working with. Here's what happens.
1) Thread A locks @vm, enters qemuProcessStop().
2) Thread B parses given XML, calls virDomainObjListAdd() ->
virDomainObjListAdd() -> virDomainObjListAddLocked() ->
virDomainObjListFindByUUIDLocked(). Since there's a match on
UUID, an virDomainObj object is returned and the thread
proceeds to calling virObjectLock(). NB, it's the same object
as in thread A.
3) Thread A sets vm->def->id = -1; this means that from this
point on, virDomainObjIsActive() will return false.
4) Thread A calls qemuDomainObjStopWorker() which unlocks the
@vm.
5) Thread B acquires the @vm lock and since
virDomainObjIsActive() returns false, it proceeds to calling
virDomainObjAssignDef() where vm->def is replaced.
6) Thread B then calls qemuProcessBeginJob() which unlocks the
@vm temporarily.
7) Thread A, still in qemuDomainObjStopWorker() acquires @vm lock
and proceeds with cleanup.
8) Thread A finds different definition than the one needing
cleanup.
In my testing I've seen stale pointers, e.g.
vm->def->nets[0]->priv was NULL, which lead to a SIGSEGV as
there's 'QEMU_DOMAIN_NETWORK_PRIVATE(net)->created' line when
cleaning up nets. Your mileage may vary.
Even if we did not crash, the plain fact that vm->def is changed
in the middle of qemuProcessStop() means we might be cleaning up
something else than intended.
As a fix, I'm moving all lines that obviously touch vm->def
before the domain object is unlocked. That left
virHookCall(VIR_HOOK_QEMU_OP_STOPPED, VIR_HOOK_SUBOP_END) nearly
next to virHookCall(VIR_HOOK_QEMU_OP_RELEASE,
VIR_HOOK_SUBOP_END) which I figured is not something we want. So
I've shuffled things a bit more.
Fixes: 3865410e7f67ca4ec66e9a905e75f452762a97f0
Resolves: https://issues.redhat.com/browse/RHEL-49607
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_process.c | 122 ++++++++++++++++++++--------------------
1 file changed, 62 insertions(+), 60 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 25dfd04272..9ea6c678b8 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8530,6 +8530,18 @@ void qemuProcessStop(virQEMUDriver *driver,
VIR_QEMU_PROCESS_KILL_FORCE|
VIR_QEMU_PROCESS_KILL_NOCHECK));
+ vm->pid = 0;
+
+ /* now that we know it's stopped call the hook if present */
+ if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) {
+ g_autofree char *xml = qemuDomainDefFormatXML(driver, NULL, vm->def, 0);
+
+ /* we can't stop the operation even if the script raised an error */
+ ignore_value(virHookCall(VIR_HOOK_DRIVER_QEMU, vm->def->name,
+ VIR_HOOK_QEMU_OP_STOPPED, VIR_HOOK_SUBOP_END,
+ NULL, xml, NULL));
+ }
+
if (priv->agent) {
g_clear_pointer(&priv->agent, qemuAgentClose);
}
@@ -8553,25 +8565,6 @@ void qemuProcessStop(virQEMUDriver *driver,
qemuDBusStop(driver, vm);
- /* Only after this point we can reset 'priv->beingDestroyed' so that
- * there's no point at which the VM could be considered as alive between
- * entering the destroy job and this point where the active "flag" is
- * cleared.
- */
- vm->def->id = -1;
- priv->beingDestroyed = false;
-
- /* Wake up anything waiting on domain condition */
- virDomainObjBroadcast(vm);
-
- /* IMPORTANT: qemuDomainObjStopWorker() unlocks @vm in order to prevent
- * deadlocks with the per-VM event loop thread. This MUST be done after
- * marking the VM as dead */
- qemuDomainObjStopWorker(vm);
-
- if (!!g_atomic_int_dec_and_test(&driver->nactive) && driver->inhibitCallback)
- driver->inhibitCallback(false, driver->inhibitOpaque);
-
/* Clear network bandwidth */
virDomainClearNetBandwidth(vm->def);
@@ -8588,18 +8581,6 @@ void qemuProcessStop(virQEMUDriver *driver,
}
}
- virPortAllocatorRelease(priv->nbdPort);
- priv->nbdPort = 0;
-
- if (priv->monConfig) {
- if (priv->monConfig->type == VIR_DOMAIN_CHR_TYPE_UNIX)
- unlink(priv->monConfig->data.nix.path);
- g_clear_pointer(&priv->monConfig, virObjectUnref);
- }
-
- /* Remove the master key */
- qemuDomainMasterKeyRemove(priv);
-
ignore_value(virDomainChrDefForeach(vm->def,
false,
qemuProcessCleanupChardevDevice,
@@ -8609,22 +8590,6 @@ void qemuProcessStop(virQEMUDriver *driver,
/* Its namespace is also gone then. */
qemuDomainDestroyNamespace(driver, vm);
- virFileDeleteTree(priv->libDir);
- virFileDeleteTree(priv->channelTargetDir);
-
- /* Stop autodestroy in case guest is restarted */
- virCloseCallbacksDomainRemove(vm, NULL, qemuProcessAutoDestroy);
-
- /* now that we know it's stopped call the hook if present */
- if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) {
- g_autofree char *xml = qemuDomainDefFormatXML(driver, NULL, vm->def, 0);
-
- /* we can't stop the operation even if the script raised an error */
- ignore_value(virHookCall(VIR_HOOK_DRIVER_QEMU, vm->def->name,
- VIR_HOOK_QEMU_OP_STOPPED, VIR_HOOK_SUBOP_END,
- NULL, xml, NULL));
- }
-
/* Reset Security Labels unless caller don't want us to */
if (!(flags & VIR_QEMU_PROCESS_STOP_NO_RELABEL))
qemuSecurityRestoreAllLabel(driver, vm,
@@ -8672,8 +8637,6 @@ void qemuProcessStop(virQEMUDriver *driver,
virResctrlAllocRemove(vm->def->resctrls[i]->alloc);
}
- qemuProcessRemoveDomainStatus(driver, vm);
-
/* Remove VNC and Spice ports from port reservation bitmap, but only if
they were reserved by the driver (autoport=yes)
*/
@@ -8706,20 +8669,9 @@ void qemuProcessStop(virQEMUDriver *driver,
}
}
- for (i = 0; i < vm->ndeprecations; i++)
- g_free(vm->deprecations[i]);
- g_clear_pointer(&vm->deprecations, g_free);
- vm->ndeprecations = 0;
- vm->taint = 0;
- vm->pid = 0;
- virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, reason);
for (i = 0; i < vm->def->niothreadids; i++)
vm->def->iothreadids[i]->thread_id = 0;
- /* clean up a possible backup job */
- if (priv->backup)
- qemuBackupJobTerminate(vm, VIR_DOMAIN_JOB_STATUS_CANCELED);
-
/* Do this explicitly after vm->pid is reset so that security drivers don't
* try to enter the domain's namespace which is non-existent by now as qemu
* is no longer running. */
@@ -8753,6 +8705,56 @@ void qemuProcessStop(virQEMUDriver *driver,
qemuSecurityReleaseLabel(driver->securityManager, vm->def);
+ /* Only after this point we can reset 'priv->beingDestroyed' so that
+ * there's no point at which the VM could be considered as alive between
+ * entering the destroy job and this point where the active "flag" is
+ * cleared.
+ */
+ vm->def->id = -1;
+ priv->beingDestroyed = false;
+
+ /* Wake up anything waiting on domain condition */
+ virDomainObjBroadcast(vm);
+
+ /* IMPORTANT: qemuDomainObjStopWorker() unlocks @vm in order to prevent
+ * deadlocks with the per-VM event loop thread. This MUST be done after
+ * marking the VM as dead */
+ qemuDomainObjStopWorker(vm);
+
+ if (!!g_atomic_int_dec_and_test(&driver->nactive) && driver->inhibitCallback)
+ driver->inhibitCallback(false, driver->inhibitOpaque);
+
+ virPortAllocatorRelease(priv->nbdPort);
+ priv->nbdPort = 0;
+
+ if (priv->monConfig) {
+ if (priv->monConfig->type == VIR_DOMAIN_CHR_TYPE_UNIX)
+ unlink(priv->monConfig->data.nix.path);
+ g_clear_pointer(&priv->monConfig, virObjectUnref);
+ }
+
+ /* Remove the master key */
+ qemuDomainMasterKeyRemove(priv);
+
+ virFileDeleteTree(priv->libDir);
+ virFileDeleteTree(priv->channelTargetDir);
+
+ /* Stop autodestroy in case guest is restarted */
+ virCloseCallbacksDomainRemove(vm, NULL, qemuProcessAutoDestroy);
+
+ qemuProcessRemoveDomainStatus(driver, vm);
+
+ for (i = 0; i < vm->ndeprecations; i++)
+ g_free(vm->deprecations[i]);
+ g_clear_pointer(&vm->deprecations, g_free);
+ vm->ndeprecations = 0;
+ vm->taint = 0;
+ virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, reason);
+
+ /* clean up a possible backup job */
+ if (priv->backup)
+ qemuBackupJobTerminate(vm, VIR_DOMAIN_JOB_STATUS_CANCELED);
+
/* clear all private data entries which are no longer needed */
qemuDomainObjPrivateDataClear(priv);
--
2.44.2
8 months, 3 weeks
[PATCH v2] virt-host-validate: Allow longer list of CPU flags
by Michal Privoznik
On various occasions, virt-host-validate parses /proc/cpuinfo to
learn about CPU flags (see virHostValidateGetCPUFlags()). It does
so, by reading the file line by line until the line with CPU
flags is reached. Then the line is split into individual flags
(using space as a delimiter) and the list of flags is then
iterated over.
This works, except for cases when the line with CPU flags is too
long. Problem is - the line is capped at 1024 bytes and on newer
CPUs (and newer kernels), the line can be significantly longer.
I've seen a line that's ~1200 characters long (with 164 flags
reported).
Switch to unbounded read from the file (getline()).
Resolves: https://issues.redhat.com/browse/RHEL-39969
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
v2 of:
https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/LK...
diff to v2:
- Keep trimming of the optional newline
tools/virt-host-validate-common.c | 18 +++++++-----------
1 file changed, 7 insertions(+), 11 deletions(-)
diff --git a/tools/virt-host-validate-common.c b/tools/virt-host-validate-common.c
index 591143c24d..63cc3dbe7b 100644
--- a/tools/virt-host-validate-common.c
+++ b/tools/virt-host-validate-common.c
@@ -106,21 +106,19 @@ virBitmap *virHostValidateGetCPUFlags(void)
{
FILE *fp;
virBitmap *flags = NULL;
+ g_autofree char *line = NULL;
+ size_t linelen = 0;
if (!(fp = fopen("/proc/cpuinfo", "r")))
return NULL;
flags = virBitmapNew(VIR_HOST_VALIDATE_CPU_FLAG_LAST);
- do {
- char line[1024];
+ while (getline(&line, &linelen, fp) > 0) {
char *start;
g_auto(GStrv) tokens = NULL;
GStrv next;
- if (!fgets(line, sizeof(line), fp))
- break;
-
/* The line we're interested in is marked differently depending
* on the architecture, so check possible prefixes */
if (!STRPREFIX(line, "flags") &&
@@ -129,11 +127,9 @@ virBitmap *virHostValidateGetCPUFlags(void)
!STRPREFIX(line, "facilities"))
continue;
- /* fgets() includes the trailing newline in the output buffer,
- * so we need to clean that up ourselves. We can safely access
- * line[strlen(line) - 1] because the checks above would cause
- * us to skip empty strings */
- line[strlen(line) - 1] = '\0';
+ /* getline() may include the trailing newline in the output
+ * buffer, so we need to clean that up ourselves. */
+ virStringTrimOptionalNewline(line);
/* Skip to the separator */
if (!(start = strchr(line, ':')))
@@ -153,7 +149,7 @@ virBitmap *virHostValidateGetCPUFlags(void)
if ((value = virHostValidateCPUFlagTypeFromString(*next)) >= 0)
ignore_value(virBitmapSetBit(flags, value));
}
- } while (1);
+ }
VIR_FORCE_FCLOSE(fp);
--
2.44.2
8 months, 3 weeks
[PATCH] virt-host-validate: Allow longer list of CPU flags
by Michal Privoznik
On various occasions, virt-host-validate parses /proc/cpuinfo to
learn about CPU flags (see virHostValidateGetCPUFlags()). It does
so, by reading the file line by line until the line with CPU
flags is reached. Then the line is split into individual flags
(using space as a delimiter) and the list of flags is then
iterated over.
This works, except for cases when the line with CPU flags is too
long. Problem is - the line is capped at 1024 bytes and on newer
CPUs (and newer kernels), the line can be significantly longer.
I've seen a line that's ~1200 characters long (with 164 flags
reported).
Switch to unbounded read from the file (getline()).
Resolves: https://issues.redhat.com/browse/RHEL-39969
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
tools/virt-host-validate-common.c | 16 ++++------------
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/tools/virt-host-validate-common.c b/tools/virt-host-validate-common.c
index 591143c24d..e5fa6606d2 100644
--- a/tools/virt-host-validate-common.c
+++ b/tools/virt-host-validate-common.c
@@ -106,21 +106,19 @@ virBitmap *virHostValidateGetCPUFlags(void)
{
FILE *fp;
virBitmap *flags = NULL;
+ g_autofree char *line = NULL;
+ size_t linelen = 0;
if (!(fp = fopen("/proc/cpuinfo", "r")))
return NULL;
flags = virBitmapNew(VIR_HOST_VALIDATE_CPU_FLAG_LAST);
- do {
- char line[1024];
+ while (getline(&line, &linelen, fp) > 0) {
char *start;
g_auto(GStrv) tokens = NULL;
GStrv next;
- if (!fgets(line, sizeof(line), fp))
- break;
-
/* The line we're interested in is marked differently depending
* on the architecture, so check possible prefixes */
if (!STRPREFIX(line, "flags") &&
@@ -129,12 +127,6 @@ virBitmap *virHostValidateGetCPUFlags(void)
!STRPREFIX(line, "facilities"))
continue;
- /* fgets() includes the trailing newline in the output buffer,
- * so we need to clean that up ourselves. We can safely access
- * line[strlen(line) - 1] because the checks above would cause
- * us to skip empty strings */
- line[strlen(line) - 1] = '\0';
-
/* Skip to the separator */
if (!(start = strchr(line, ':')))
continue;
@@ -153,7 +145,7 @@ virBitmap *virHostValidateGetCPUFlags(void)
if ((value = virHostValidateCPUFlagTypeFromString(*next)) >= 0)
ignore_value(virBitmapSetBit(flags, value));
}
- } while (1);
+ }
VIR_FORCE_FCLOSE(fp);
--
2.44.2
8 months, 3 weeks
[PATCH] rpc: report error from filing to add timer
by Daniel P. Berrangé
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/rpc/virnetclientstream.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/rpc/virnetclientstream.c b/src/rpc/virnetclientstream.c
index 98034d737d..380b785869 100644
--- a/src/rpc/virnetclientstream.c
+++ b/src/rpc/virnetclientstream.c
@@ -725,6 +725,8 @@ int virNetClientStreamEventAddCallback(virNetClientStream *st,
virNetClientStreamEventTimer,
st,
virObjectUnref)) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Unable to add timer to event loop"));
virObjectUnref(st);
goto cleanup;
}
--
2.45.2
8 months, 3 weeks
cpugroups for hyperv hypervisor
by Praveen K Paladugu
Hey folks,
My team is working on exposing `cpugroups` to Libvirt while using
'hyperv' hypervisor with cloud-hypervisor(VMM). cpugroups are relevant
in a specific configuration of hyperv called 'minroot'. In Minroot
configuration, hypervisor artificially restricts Dom0 to run on a subset
of cpus (Logical Processors). The rest of the cpus can be assigned to
guests.
cpugroups manage the CPUs assigned to guests and their scheduling
properties. Initially this looks similar to `cpuset` (in cgroups), but
the controls available with cpugroups don't map easily to those in
cgroups. For example:
* "IdleLPs" are the number of Logical Processors in a cpugroup, that
should be reserved to a guest even if they are idle
* "SchedulingPriority", the priority(values between 0..7) with which to
schedule CPUs in a cpugroup.
As controls like above don't easily map to anything in cgroups, using a
driver specific element in Domain xml, to configure cpugroups seems like
a right approach. For example:
<ch:cpugroups>
<idle_lps value='4'/>
<scheduling_priority value='6'/>
</ch:cpugroups>
As cpugroups is only relevant while using minroot configuration on
hyperv, I don't see any value in generalizing this setting. So, having
some "ch" driver specific settings seems like a good approach to
implement this feature.
Question1: Do you see any concerns with this approach?
The cpugroup settings can be applied/modified using sysfs interface or
using a cmdline tool on the host. I see Libvirt uses both these
mechanisms for various use cases. But, given a choice, sysfs based
interface seems like a simpler approach to me. With sysfs interface
Libvirt does not have to take install time dependencies on new tools.
Question2: Of "sysfs" vs "cmdline tool" which is preferred, given a choice?
Early feedback from the community will help us invest in the preferred
choices sooner than later. Thanks for your consideration.
References:
*
https://learn.microsoft.com/en-us/windows-server/virtualization/hyper-v/m...
--
Regards,
Praveen
8 months, 3 weeks
