[libvirt] [PATCH] qemu: Forcibly mknod() even if it exists
by Michal Privoznik
Another weird bug appeared concerning qemu namespaces. Basically
the problem is as follows:
1) Issue an API that causes libvirt to create a node in domain's
namespace, say /dev/sda with 8:0 as major:minor (the API can
be attach-disk for instance). Or simply create the node from
a console by hand.
2) Detach the disk from qemu.
3) Do something that makes /dev/sda change it's minor number.
4) Try to attach the disk again.
The problem is, in a few cases - like disk-detach - we don't
remove the corresponding /dev node from the mount namespace
(because it may be used by some other disk's backing chain).
But this creates a problem, because if the node changes its
MAJ:MIN numbers we don't propagate the change into the domain's
namespace. We do plain mknod() and ignore EEXIST which obviously
is not enough because it doesn't guarantee that the node has
updated MAJ:MIN pair.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1752978
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_domain.c | 25 +++++++++----------------
1 file changed, 9 insertions(+), 16 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 262b74d1ab..f54b9b21ff 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -13203,16 +13203,14 @@ qemuDomainCreateDeviceRecursive(const char *device,
allow_noent, ttl - 1) < 0)
goto cleanup;
} else if (isDev) {
- if (create &&
- mknod(devicePath, sb.st_mode, sb.st_rdev) < 0) {
- if (errno == EEXIST) {
- ret = 0;
- } else {
+ if (create) {
+ unlink(devicePath);
+ if (mknod(devicePath, sb.st_mode, sb.st_rdev) < 0) {
virReportSystemError(errno,
_("Failed to make device %s"),
devicePath);
+ goto cleanup;
}
- goto cleanup;
}
} else if (isReg) {
if (create &&
@@ -13996,17 +13994,12 @@ qemuDomainAttachDeviceMknodHelper(pid_t pid G_GNUC_UNUSED,
} else if (isDev) {
VIR_DEBUG("Creating dev %s (%d,%d)",
data->file, major(data->sb.st_rdev), minor(data->sb.st_rdev));
+ unlink(data->file);
if (mknod(data->file, data->sb.st_mode, data->sb.st_rdev) < 0) {
- /* Because we are not removing devices on hotunplug, or
- * we might be creating part of backing chain that
- * already exist due to a different disk plugged to
- * domain, accept EEXIST. */
- if (errno != EEXIST) {
- virReportSystemError(errno,
- _("Unable to create device %s"),
- data->file);
- goto cleanup;
- }
+ virReportSystemError(errno,
+ _("Unable to create device %s"),
+ data->file);
+ goto cleanup;
} else {
delDevice = true;
}
--
2.23.0
5 years
[libvirt] [PATCH 0/4] qemu: block: implement optional removal of committed snapshot images
by Pavel Mores
Adds handling of the VIR_DOMAIN_BLOCK_COMMIT_DELETE flag to the qemu driver
which previously just ignored it. This should fix a problem with the virsh
blockcommit command where --delete did nothing.
https://bugzilla.redhat.com/show_bug.cgi?id=1008350
Pavel Mores (4):
qemu: block: propagate the delete flag to where it can actually be
used
qemu: block: use the delete flag to delete snapshot images if
requested
qemu: block: store the delete flag in libvirtd's status XML
qemu: block: add unit test of storing the delete flag in status XML
src/qemu/qemu_blockjob.c | 35 ++++++++++++++++++-
src/qemu/qemu_blockjob.h | 4 ++-
src/qemu/qemu_domain.c | 4 +++
src/qemu/qemu_driver.c | 5 +--
.../blockjob-blockdev-in.xml | 1 +
5 files changed, 45 insertions(+), 4 deletions(-)
--
2.21.0
5 years
[libvirt] [PATCH] lib: fix documentation typo in virDomainGetGuestInfo()
by Jonathon Jongsma
Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com>
---
src/libvirt-domain.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 02622cb2ca..2d9c4061e2 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -12272,7 +12272,7 @@ virDomainSetVcpu(virDomainPtr domain,
* "timezone.offset" - the offset to UTC in seconds as an int
*
* VIR_DOMAIN_GUEST_INFO_FILESYSTEM:
- * Returns inforamtion about the filesystems within the domain. The typed
+ * Returns information about the filesystems within the domain. The typed
* parameter keys are in this format:
*
* "fs.count" - the number of filesystems defined on this domain
--
2.21.0
5 years
Re: [libvirt] [PATCH 0/6] VFIO mdev aggregated resources handling
by Alex Williamson
On Thu, 24 Oct 2019 13:08:23 +0800
Zhenyu Wang <zhenyuw(a)linux.intel.com> wrote:
> Hi,
>
> This is a refresh for previous send of this series. I got impression that
> some SIOV drivers would still deploy their own create and config method so
> stopped effort on this. But seems this would still be useful for some other
> SIOV driver which may simply want capability to aggregate resources. So here's
> refreshed series.
>
> Current mdev device create interface depends on fixed mdev type, which get uuid
> from user to create instance of mdev device. If user wants to use customized
> number of resource for mdev device, then only can create new mdev type for that
> which may not be flexible. This requirement comes not only from to be able to
> allocate flexible resources for KVMGT, but also from Intel scalable IO
> virtualization which would use vfio/mdev to be able to allocate arbitrary
> resources on mdev instance. More info on [1] [2] [3].
>
> To allow to create user defined resources for mdev, it trys to extend mdev
> create interface by adding new "aggregate=xxx" parameter following UUID, for
> target mdev type if aggregation is supported, it can create new mdev device
> which contains resources combined by number of instances, e.g
>
> echo "<uuid>,aggregate=10" > create
>
> VM manager e.g libvirt can check mdev type with "aggregation" attribute which
> can support this setting. If no "aggregation" attribute found for mdev type,
> previous behavior is still kept for one instance allocation. And new sysfs
> attribute "aggregated_instances" is created for each mdev device to show allocated number.
Given discussions we've had recently around libvirt interacting with
mdev, I think that libvirt would rather have an abstract interface via
mdevctl[1]. Therefore can you evaluate how mdevctl would support this
creation extension? It seems like it would fit within the existing
mdev and mdevctl framework if aggregation were simply a sysfs attribute
for the device. For example, the mdevctl steps might look like this:
mdevctl define -u UUID -p PARENT -t TYPE
mdevctl modify -u UUID --addattr=mdev/aggregation --value=2
mdevctl start -u UUID
When mdevctl starts the mdev, it will first create it using the
existing mechanism, then apply aggregation attribute, which can consume
the necessary additional instances from the parent device, or return an
error, which would unwind and return a failure code to the caller
(libvirt). I think the vendor driver would then have freedom to decide
when the attribute could be modified, for instance it would be entirely
reasonable to return -EBUSY if the user attempts to modify the
attribute while the mdev device is in-use. Effectively aggregation
simply becomes a standardized attribute with common meaning. Thoughts?
[cc libvirt folks for their impression] Thanks,
Alex
[1] https://github.com/mdevctl/mdevctl
> References:
> [1] https://software.intel.com/en-us/download/intel-virtualization-technology...
> [2] https://software.intel.com/en-us/download/intel-scalable-io-virtualizatio...
> [3] https://schd.ws/hosted_files/lc32018/00/LC3-SIOV-final.pdf
>
> Zhenyu Wang (6):
> vfio/mdev: Add new "aggregate" parameter for mdev create
> vfio/mdev: Add "aggregation" attribute for supported mdev type
> vfio/mdev: Add "aggregated_instances" attribute for supported mdev
> device
> Documentation/driver-api/vfio-mediated-device.rst: Update for
> vfio/mdev aggregation support
> Documentation/ABI/testing/sysfs-bus-vfio-mdev: Update for vfio/mdev
> aggregation support
> drm/i915/gvt: Add new type with aggregation support
>
> Documentation/ABI/testing/sysfs-bus-vfio-mdev | 24 ++++++
> .../driver-api/vfio-mediated-device.rst | 23 ++++++
> drivers/gpu/drm/i915/gvt/gvt.c | 4 +-
> drivers/gpu/drm/i915/gvt/gvt.h | 11 ++-
> drivers/gpu/drm/i915/gvt/kvmgt.c | 53 ++++++++++++-
> drivers/gpu/drm/i915/gvt/vgpu.c | 56 ++++++++++++-
> drivers/vfio/mdev/mdev_core.c | 36 ++++++++-
> drivers/vfio/mdev/mdev_private.h | 6 +-
> drivers/vfio/mdev/mdev_sysfs.c | 79 ++++++++++++++++++-
> include/linux/mdev.h | 19 +++++
> 10 files changed, 294 insertions(+), 17 deletions(-)
>
5 years
[libvirt] [PATCH] lxc: Fix 'domblkstat' error with attached disk devices.
by jcfaracco＠gmail.com
From: Julio Faracco <jcfaracco(a)gmail.com>
LXC was not working when you attached a new disk that points to block
device. See: https://bugzilla.redhat.com/show_bug.cgi?id=1697115.
Command line from virsh was showing problems with alias first (this
feature is not supported) and after, problems to query block device. It
was happening because extra disks were not being included into
cgroup blkio.throttle properties and libvirt could not query this info.
Applying those devices into 'allowed' list is not enough, libvirt should
reset blkio.throttle as a workaround to include disks into container's
cgroup directory.
Before:
virsh # domblkstat CentOS hda
error: Failed to get block stats for domain 'CentOS' device 'hda'
error: internal error: domain stats query failed
Now:
virsh # domblkstat CentOS hda
hda rd_req 0
hda rd_bytes 0
hda wr_req 0
hda wr_bytes 0
Signed-off-by: Julio Faracco <jcfaracco(a)gmail.com>
---
src/lxc/lxc_cgroup.c | 29 ++++++++++++++++++++++++++++-
src/lxc/lxc_cgroup.h | 4 ++++
src/lxc/lxc_driver.c | 8 ++++----
3 files changed, 36 insertions(+), 5 deletions(-)
diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c
index 5efb495b56..de6d892521 100644
--- a/src/lxc/lxc_cgroup.c
+++ b/src/lxc/lxc_cgroup.c
@@ -302,6 +302,24 @@ virLXCTeardownHostUSBDeviceCgroup(virUSBDevicePtr dev G_GNUC_UNUSED,
return 0;
}
+int
+virLXCCgroupResetBlkioDeviceThrottle(virCgroupPtr cgroup,
+ const char *path)
+{
+ if (virCgroupSetBlkioDeviceReadBps(cgroup, path, 0) < 0)
+ return -1;
+
+ if (virCgroupSetBlkioDeviceWriteBps(cgroup, path, 0) < 0)
+ return -1;
+
+ if (virCgroupSetBlkioDeviceReadIops(cgroup, path, 0) < 0)
+ return -1;
+
+ if (virCgroupSetBlkioDeviceWriteIops(cgroup, path, 0) < 0)
+ return -1;
+
+ return 0;
+}
static int virLXCCgroupSetupDeviceACL(virDomainDefPtr def,
virCgroupPtr cgroup)
@@ -309,6 +327,7 @@ static int virLXCCgroupSetupDeviceACL(virDomainDefPtr def,
int capMknod = def->caps_features[VIR_DOMAIN_CAPS_FEATURE_MKNOD];
int ret = -1;
size_t i;
+ const char *src_path = NULL;
static virLXCCgroupDevicePolicy devices[] = {
{'c', LXC_DEV_MAJ_MEMORY, LXC_DEV_MIN_NULL},
{'c', LXC_DEV_MAJ_MEMORY, LXC_DEV_MIN_ZERO},
@@ -346,8 +365,16 @@ static int virLXCCgroupSetupDeviceACL(virDomainDefPtr def,
!virStorageSourceIsBlockLocal(def->disks[i]->src))
continue;
+ /* Workaround to include disks into blkio.throttle.
+ * To include it, we need to reset any feature of
+ * blkio.throttle.* */
+ src_path = virDomainDiskGetSource(def->disks[i]);
+ if (virLXCCgroupResetBlkioDeviceThrottle(cgroup,
+ src_path) < 0)
+ goto cleanup;
+
if (virCgroupAllowDevicePath(cgroup,
- virDomainDiskGetSource(def->disks[i]),
+ src_path,
(def->disks[i]->src->readonly ?
VIR_CGROUP_DEVICE_READ :
VIR_CGROUP_DEVICE_RW) |
diff --git a/src/lxc/lxc_cgroup.h b/src/lxc/lxc_cgroup.h
index 63e9e837b0..3d643a4fea 100644
--- a/src/lxc/lxc_cgroup.h
+++ b/src/lxc/lxc_cgroup.h
@@ -46,3 +46,7 @@ int
virLXCTeardownHostUSBDeviceCgroup(virUSBDevicePtr dev,
const char *path,
void *opaque);
+
+int
+virLXCCgroupResetBlkioDeviceThrottle(virCgroupPtr cgroup,
+ const char *path);
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 204a3ed522..87da55f308 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -2339,14 +2339,14 @@ lxcDomainBlockStats(virDomainPtr dom,
goto endjob;
}
- if (!disk->info.alias) {
+ if (!disk->src->path) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("missing disk device alias name for %s"), disk->dst);
goto endjob;
}
ret = virCgroupGetBlkioIoDeviceServiced(priv->cgroup,
- disk->info.alias,
+ disk->src->path,
&stats->rd_bytes,
&stats->wr_bytes,
&stats->rd_req,
@@ -2424,14 +2424,14 @@ lxcDomainBlockStatsFlags(virDomainPtr dom,
goto endjob;
}
- if (!disk->info.alias) {
+ if (!disk->src->path) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("missing disk device alias name for %s"), disk->dst);
goto endjob;
}
if (virCgroupGetBlkioIoDeviceServiced(priv->cgroup,
- disk->info.alias,
+ disk->src->path,
&rd_bytes,
&wr_bytes,
&rd_req,
--
2.20.1
5 years
[libvirt] [PATCH v2 0/3] remove 'cleanup: return' labels
by Daniel Henrique Barboza
changes from v1 [1], all suggested by Ján Tomko:
- qemu patch (former patch 10):
* qemu_driver.c: using 'return -1' instead of 'return ret' in qemuDumpToFd()
and qemuDomainBlockPivot()
* qemu_hotplug.c: removed the use of 'ret' variable in
qemuDomainChangeGraphics(), returning -1 in all error
conditions and returning 0 in the end
- vbox patch (former patch 13):
* vbox_common.c: changed 'return -2' to 'return -1' in vboxDomainGetInfo()
- tests patch (former patch 20):
* domainconftest.c: rolled back the change since the 'goto cleanup'
is more readable than copying EXIT_SUCCESS multiple times.
[1] https://www.redhat.com/archives/libvir-list/2019-October/msg01420.html
Daniel Henrique Barboza (3):
qemu: remove unneeded cleanup labels
vbox: remove unneeded cleanup labels
tests: remove unneeded cleanup labels
src/qemu/qemu_cgroup.c | 15 ++--
src/qemu/qemu_domain.c | 84 ++++++------------
src/qemu/qemu_domain_address.c | 31 +++----
src/qemu/qemu_driver.c | 145 +++++++++++--------------------
src/qemu/qemu_hotplug.c | 75 +++++++---------
src/qemu/qemu_interface.c | 26 +++---
src/qemu/qemu_migration.c | 38 +++-----
src/qemu/qemu_process.c | 69 +++++----------
src/qemu/qemu_tpm.c | 18 ++--
src/vbox/vbox_common.c | 12 ++-
src/vbox/vbox_snapshot_conf.c | 75 +++++++---------
tests/commandtest.c | 17 ++--
tests/networkxml2firewalltest.c | 10 +--
tests/nsstest.c | 27 +++---
tests/nwfilterebiptablestest.c | 4 +-
tests/nwfilterxml2firewalltest.c | 16 ++--
tests/qemuhotplugtest.c | 19 ++--
tests/qemuxml2argvtest.c | 11 +--
tests/virauthconfigtest.c | 11 +--
tests/vircgroupmock.c | 11 +--
tests/virendiantest.c | 58 ++++++-------
tests/virkeycodetest.c | 14 +--
tests/virmacmaptest.c | 5 +-
tests/virnetdevtest.c | 11 +--
tests/virpcimock.c | 31 +++----
tests/virpcitest.c | 3 +-
tests/virpolkittest.c | 65 +++++---------
tests/virstringtest.c | 18 ++--
28 files changed, 333 insertions(+), 586 deletions(-)
--
2.21.0
5 years
[libvirt] [PATCH 0/3] tests: schema: Try to improve debugability
by Peter Krempa
Peter Krempa (3):
tests: schema: Simplify memory handling using g_autofree
tests: utils: Introduce helper for dispatching libvirt errors
tests: virschema: Propagate errors from directory traversal in
testSchemaDir
tests/testutils.c | 29 +++++++++++++++++++++++++----
tests/testutils.h | 1 +
tests/virschematest.c | 34 +++++++++++++---------------------
3 files changed, 39 insertions(+), 25 deletions(-)
--
2.23.0
5 years
[libvirt] [PATCH 1/3] qemuProcessStop: Set @def early
by Michal Privoznik
The @def variable holds pointer to the domain defintion, but is
set only somewhere in the middle of the function. This is
suboptimal.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_process.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 90ebdb43a0..8cb12ac9a6 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -7365,6 +7365,8 @@ void qemuProcessStop(virQEMUDriverPtr driver,
goto endjob;
}
+ def = vm->def;
+
qemuProcessBuildDestroyMemoryPaths(driver, vm, NULL, false);
if (virAtomicIntDecAndTest(&driver->nactive) && driver->inhibitCallback)
@@ -7386,7 +7388,6 @@ void qemuProcessStop(virQEMUDriverPtr driver,
virDomainConfVMNWFilterTeardown(vm);
if (cfg->macFilter) {
- def = vm->def;
for (i = 0; i < def->nnets; i++) {
virDomainNetDefPtr net = def->nets[i];
if (net->ifname == NULL)
@@ -7483,7 +7484,6 @@ void qemuProcessStop(virQEMUDriverPtr driver,
qemuHostdevReAttachDomainDevices(driver, vm->def);
- def = vm->def;
for (i = 0; i < def->nnets; i++) {
virDomainNetDefPtr net = def->nets[i];
vport = virDomainNetGetActualVirtPortProfile(net);
--
2.23.0
5 years
[libvirt] [PATCH] Add news for virDomainAgentSetResponseTimeout()
by Jonathon Jongsma
Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com>
---
docs/news.xml | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/docs/news.xml b/docs/news.xml
index 240d288c37..72c0edb317 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -66,6 +66,19 @@
initialized the vGPU.
</description>
</change>
+ <change>
+ <summary>
+ Add API to change the response timeout for guest agent commands
+ </summary>
+ <description>
+ By default, when a command is sent to the guest agent, libvirt waits
+ forever for a response from the guest agent. If the guest is
+ unresponsive for any reason, this can block the calling thread
+ indefinitely. By setting a custom timeout using
+ <code>virDomainAgentSetResponseTimeout()</code>, API users can change
+ this behavior.
+ </description>
+ </change>
</section>
<section title="Improvements">
</section>
--
2.21.0
5 years
[libvirt] [libvirt-dbus][PATCH v4 0/2] Implement snapshots
by Simon Kobyda
Changes since v2:
Implement and use virtDBusDomainSnapshotGetVirDomainSnapshot
Add domain snapshot xml file to data/meson.build
Fix flake8 style errors
Simon Kobyda (2):
Introduce Domain Snapshot Interface
Implement snapshots APIs
data/meson.build | 1 +
data/org.libvirt.Domain.xml | 26 +++
data/org.libvirt.DomainSnapshot.xml | 41 ++++
src/connect.c | 6 +
src/connect.h | 1 +
src/domain.c | 154 +++++++++++++++
src/domainsnapshot.c | 291 ++++++++++++++++++++++++++++
src/domainsnapshot.h | 9 +
src/meson.build | 1 +
src/util.c | 53 +++++
src/util.h | 16 ++
tests/libvirttest.py | 12 ++
tests/meson.build | 1 +
tests/test_domain.py | 8 +
tests/test_snapshot.py | 45 +++++
tests/xmldata.py | 6 +
16 files changed, 671 insertions(+)
create mode 100644 data/org.libvirt.DomainSnapshot.xml
create mode 100644 src/domainsnapshot.c
create mode 100644 src/domainsnapshot.h
create mode 100755 tests/test_snapshot.py
--
2.21.0
5 years
