Re: [libvirt] [PATCH] vz: support cpu time in driver's domainGetInfo
28.10.2015 17:29, Nikolay Shirokovskiy пишет:
Just straight-forward patch.
Use reference counting for privdom as stats internally could drop domain lock.
Signed-off-by: Nikolay Shirokovskiy
---
src/vz/vz_driver.c | 19 ---
1 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index 6f1cbfb..0a968b9 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -554,7 +554,7 @@ vzDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info)
virDomainObjPtr privdom;
int ret = -1;
-if (!(privdom = vzDomObjFromDomain(domain)))
+if (!(privdom = vzDomObjFromDomainRef(domain)))
goto cleanup;
info->state = virDomainObjGetState(privdom, NULL);
@@ -562,11 +562,24 @@ vzDomainGetInfo(virDomainPtr domain, virDomainInfoPtr
info)
info->maxMem = virDomainDefGetMemoryActual(privdom->def);
info->nrVirtCpu = privdom->def->vcpus;
info->cpuTime = 0;
+
+if (virDomainObjIsActive(privdom)) {
+unsigned long long vtime;
+size_t i;
+
+for (i = 0; i < privdom->def->vcpus; ++i) {
+if (prlsdkGetVcpuStats(privdom, i, &vtime) < 0) {
+virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+ _("cannot read cputime for domain"));
+goto cleanup;
+}
+info->cpuTime += vtime;
+}
+}
ret = 0;
cleanup:
-if (privdom)
-virObjectUnlock(privdom);
+virDomainObjEndAPI(&privdom);
return ret;
}
ACK. Looks good.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: set error if DAD is not finished
On 10/30/2015 02:15 AM, Laine Stump wrote: On 10/29/2015 08:32 AM, Maxim Perevedentsev wrote: On 10/29/2015 12:47 PM, Luyao Huang wrote: If DAD not finished in 5 seconds, user will get an unknown error like this: # virsh net-start ipv6 error: Failed to start network ipv6 error: An error occurred, but the cause is unknown Call virReportError to set an error. Signed-off-by: Luyao Huang --- I found the DAD will take 7 seconds on my machine, and i cannot create a network which use ipv6 now :( . Can we offer a way allow user to change this timeout ? maybe add a configuration file option in libvirtd.conf. Could you please send the related records of your sysctl -a? Max DAD timeout should be rand() % net.ipv6.conf.default.router_solicitation_delay + net.ipv6.conf.default.dad_transmits * net.ipv6.neigh.default.retrans_time_ms I wonder whether my calculations were faulty or it is your specific configuration. On my system, these are set to 1, 1, and 1000, and I've found that DAD takes something between 5.7 and 6.8 seconds to complete (it was at the lower end with a single IP address, and at the high end with 70 IP addresses (or also with 20 IPs, so I don't think it's going to get substantially higher). (Too bad I didn't do this *before* I pushed, rather than relying on a successful build and reports of proper operation on your system :-/) Based on that, I think it makes sense to push a patch that sets the timeout to 20 seconds (and push Luyao's patch ragardless). Since the timeout is meant to catch an "infinite" wait, I think 20 seconds is okay; I don't want to add yet another tunable parameter unless we really need to. yes, change the timeout to 20 seconds is a good way to avoid this issue. I was trying to figure out why DAD take so long time on my machine. And thanks a lot for your quick review. Luyao -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: set error if DAD is not finished
On 10/29/2015 08:32 PM, Maxim Perevedentsev wrote: On 10/29/2015 12:47 PM, Luyao Huang wrote: If DAD not finished in 5 seconds, user will get an unknown error like this: # virsh net-start ipv6 error: Failed to start network ipv6 error: An error occurred, but the cause is unknown Call virReportError to set an error. Signed-off-by: Luyao Huang --- I found the DAD will take 7 seconds on my machine, and i cannot create a network which use ipv6 now :( . Can we offer a way allow user to change this timeout ? maybe add a configuration file option in libvirtd.conf. Could you please send the related records of your sysctl -a? Max DAD timeout should be rand() % net.ipv6.conf.default.router_solicitation_delay + net.ipv6.conf.default.dad_transmits * net.ipv6.neigh.default.retrans_time_ms I wonder whether my calculations were faulty or it is your specific configuration. I haven't change them before, and they are: net.ipv6.conf.default.router_solicitation_delay = 1 net.ipv6.conf.default.dad_transmits = 1 net.ipv6.neigh.default.retrans_time_ms = 1000 Thanks your quick reply. Luyao -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [sandbox] Weird apparmor problems
On Thu, Oct 29, 2015 at 05:35:23PM +0100, Cedric Bosdonnat wrote: > Hi all, > > I'm seeing weird apparmor errors when running virt-sandbox here. Here are the > log entries: > > apparmor="ALLOWED" operation="mknod" parent=1 > profile="libvirt-634ed189-cca0-4126-830c-4e4a76846b25" > name="/var/lib/libvirt/qemu/sandbox.monitor" pid=2251 comm="qemu-system-x86" > requested_mask="c" denied_mask="c" fsuid=493 ouid=493 > apparmor="ALLOWED" operation="open" parent=1 > profile="libvirt-634ed189-cca0-4126-830c-4e4a76846b25" name="/dev/ptmx" > pid=2251 comm="qemu-system-x86" requested_mask="w" denied_mask="w" fsuid=493 > ouid=0 > apparmor="ALLOWED" operation="open" parent=1 > profile="libvirt-634ed189-cca0-4126-830c-4e4a76846b25" name="/dev/pts/2" > pid=2251 comm="qemu-system-x86" requested_mask="w" denied_mask="w" fsuid=493 > ouid=493 > apparmor="ALLOWED" operation="file_perm" parent=1 > profile="libvirt-634ed189-cca0-4126-830c-4e4a76846b25" > name="/var/log/libvirt/qemu/sandbox.log" pid=2251 comm="qemu-system-x86" > requested_mask="w" denied_mask="w" fsuid=493 ouid=0 > apparmor="ALLOWED" operation="open" parent=1 > profile="libvirt-634ed189-cca0-4126-830c-4e4a76846b25" name="/dev/ptmx" > pid=2251 comm="qemu-system-x86" requested_mask="w" denied_mask="w" fsuid=493 > ouid=0 > apparmor="ALLOWED" operation="open" parent=1 > profile="libvirt-634ed189-cca0-4126-830c-4e4a76846b25" name="/dev/pts/3" > pid=2251 comm="qemu-system-x86" requested_mask="w" denied_mask="w" fsuid=493 > ouid=493 > apparmor="ALLOWED" operation="file_perm" parent=1 > profile="libvirt-634ed189-cca0-4126-830c-4e4a76846b25" > name="/var/log/libvirt/qemu/sandbox.log" pid=2251 comm="qemu-system-x86" > requested_mask="w" denied_mask="w" fsuid=493 ouid=0 > apparmor="ALLOWED" operation="open" parent=1 > profile="libvirt-634ed189-cca0-4126-830c-4e4a76846b25" name="/dev/kvm" > pid=2251 comm="qemu-system-x86" requested_mask="w" denied_mask="w" fsuid=493 > ouid=0 > > > The weird thing is that /dev/kvm, /var/log/libvirt/qemu/sandbox.log > and /var/lib/libvirt/qemu/sandbox.monitor already have rules. > > And I'm wondering if it's normal to have write access to /dev/pts/* > and /dev/ptmx. NB in containers we have two PTYs involved. The libvirt_lxc process opens one pty in the host context and that is used to communicate between virsh console & libvirt_lxc. The libvirt_lxc process opens one pty in the guest context and that is used to commnuicate between libvirt_lxc and the container master console. Libvirt_lxc forwards data between the two PTYs. So, yes, it is normal for libvirt_lxc to access /dev/ptmx to create a new master PTY and to read/write to /dev/pts/NN associated with the file descriptor retrieved from /dev/ptmx. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 8/8] Wait for iommmu device to go away before reprobing the host driver
Author: Shivaprasad G Bhat
There could be a delay of 1 or 2 seconds before the vfio-pci driver
is unbound and the device file /dev/vfio/ is actually
removed. If the file exists, the host driver probing the device
can lead to crash. So, wait and avoid the crash. Setting the
timeout to 15 seconds for now.
Signed-off-by: Shivaprasad G Bhat
---
src/util/virpci.c | 39 +++
1 file changed, 39 insertions(+)
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 425c1a7..6bf640d 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -1097,6 +1097,42 @@ static bool virPCIIsAKnownStub(char *driver)
return ret;
}
+#define VFIO_UNBIND_TIMEOUT 15
+
+/* It is not safe to initiate host driver probe if the vfio driver has not
+ * completely unbound the device.
+ * So, return if the unbind didn't complete in 15 seconds.
+ */
+static int virPCIWaitForVfioUnbindCompletion(virPCIDevicePtr dev)
+{
+int retry = 0;
+int ret = -1;
+char *path = NULL;
+
+if (!(path = virPCIDeviceGetIOMMUGroupDev(dev)))
+goto cleanup;
+
+while (retry++ < VFIO_UNBIND_TIMEOUT) {
+if (!virFileExists(path))
+break;
+ sleep(1);
+}
+
+if (virFileExists(path)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("The VFIO unbind not completed even after %d seconds
for device %.4x:%.2x:%.2x.%.1x"),
+ retry, dev->domain, dev->bus, dev->slot, dev->function);
+goto cleanup;
+}
+
+ret = 0;
+cleanup :
+VIR_FREE(path);
+return ret;
+
+}
+
+
static int virPCIDeviceReprobeHostDriver(virPCIDevicePtr dev, char *driver,
char *drvdir)
{
char *path = NULL;
@@ -1203,6 +1239,9 @@ virPCIDeviceUnbindFromStub(virPCIDevicePtr dev,
goto cleanup;
}
+if (virPCIWaitForVfioUnbindCompletion(dev) < 0)
+goto cleanup;
+
while (inactiveDevs && (i < virPCIDeviceListCount(inactiveDevs))) {
virPCIDevicePtr pcidev = virPCIDeviceListGet(inactiveDevs, i);
if (dev->iommuGroup == pcidev->iommuGroup) {
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 6/8] Pass activeDevs and inactiveDevs to virPCIDeviceUnbindFromStub and virPCIDeviceBindToStub
The inactiveDevs need to be selectively altered for more than one
device in case of vfio devices. So, pass the whole list.
Signed-off-by: Shivaprasad G Bhat
---
src/util/virpci.c | 38 +-
1 file changed, 21 insertions(+), 17 deletions(-)
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 2709ddd..6c24a81 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -1129,7 +1129,9 @@ static int virPCIDeviceReprobeHostDriver(virPCIDevicePtr
dev, char *driver, char
}
static int
-virPCIDeviceUnbindFromStub(virPCIDevicePtr dev)
+virPCIDeviceUnbindFromStub(virPCIDevicePtr dev,
+ virPCIDeviceListPtr activeDevs ATTRIBUTE_UNUSED,
+ virPCIDeviceListPtr inactiveDevs)
{
int result = -1;
char *drvdir = NULL;
@@ -1177,6 +1179,9 @@ virPCIDeviceUnbindFromStub(virPCIDevicePtr dev)
reprobe:
if (virPCIDeviceReprobeHostDriver(dev, driver, drvdir) < 0)
goto cleanup;
+/* Steal the dev from list inactiveDevs */
+if (inactiveDevs)
+virPCIDeviceListDel(inactiveDevs, dev);
result = 0;
@@ -1195,7 +1200,9 @@ virPCIDeviceUnbindFromStub(virPCIDevicePtr dev)
static int
virPCIDeviceBindToStub(virPCIDevicePtr dev,
- const char *stubDriverName)
+ const char *stubDriverName,
+ virPCIDeviceListPtr activeDevs,
+ virPCIDeviceListPtr inactiveDevs)
{
int result = -1;
bool reprobe = false;
@@ -1317,6 +1324,14 @@ virPCIDeviceBindToStub(virPCIDevicePtr dev,
goto cleanup;
}
+/* Add *a copy of* the dev into list inactiveDevs, if
+ * it's not already there.
+ */
+if (inactiveDevs && !virPCIDeviceListFind(inactiveDevs, dev) &&
+virPCIDeviceListAddCopy(inactiveDevs, dev) < 0) {
+result = -1;
+}
+
cleanup:
VIR_FREE(stubDriverPath);
VIR_FREE(driverLink);
@@ -1324,7 +1339,7 @@ virPCIDeviceBindToStub(virPCIDevicePtr dev,
if (result < 0) {
VIR_FREE(newDriverName);
-virPCIDeviceUnbindFromStub(dev);
+virPCIDeviceUnbindFromStub(dev, activeDevs, NULL);
} else {
VIR_FREE(dev->stubDriver);
dev->stubDriver = newDriverName;
@@ -1371,16 +1386,9 @@ virPCIDeviceDetach(virPCIDevicePtr dev,
return -1;
}
-if (virPCIDeviceBindToStub(dev, dev->stubDriver) < 0)
-return -1;
-
-/* Add *a copy of* the dev into list inactiveDevs, if
- * it's not already there.
- */
-if (inactiveDevs && !virPCIDeviceListFind(inactiveDevs, dev) &&
-virPCIDeviceListAddCopy(inactiveDevs, dev) < 0) {
+if (virPCIDeviceBindToStub(dev, dev->stubDriver,
+ activeDevs, inactiveDevs) < 0)
return -1;
-}
return 0;
}
@@ -1396,13 +1404,9 @@ virPCIDeviceReattach(virPCIDevicePtr dev,
return -1;
}
-if (virPCIDeviceUnbindFromStub(dev) < 0)
+if (virPCIDeviceUnbindFromStub(dev, activeDevs, inactiveDevs) < 0)
return -1;
-/* Steal the dev from list inactiveDevs */
-if (inactiveDevs)
-virPCIDeviceListDel(inactiveDevs, dev);
-
return 0;
}
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 7/8] Postpone reprobing till all the devices in iommu group are unbound from vfio
Author: Shivaprasad G Bhat
The host will crash if a device is bound to host driver when the device
belonging to same iommu group is in use by any of the guests. So,
do the host driver probe only after all the devices in the iommu group
have unbound from the vfio.
The patch fixes
https://bugzilla.redhat.com/show_bug.cgi?id=1272300
Signed-off-by: Shivaprasad G Bhat
---
src/util/virpci.c | 50 +-
1 file changed, 45 insertions(+), 5 deletions(-)
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 6c24a81..425c1a7 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -1128,6 +1128,23 @@ static int virPCIDeviceReprobeHostDriver(virPCIDevicePtr
dev, char *driver, char
return result;
}
+static int virPCIDeviceBoundToVFIODriver(virPCIDeviceAddressPtr devAddr, void
*opaque ATTRIBUTE_UNUSED)
+{
+int ret = 0;
+virPCIDevicePtr pci = NULL;
+
+if (!(pci = virPCIDeviceNew(devAddr->domain, devAddr->bus,
+devAddr->slot, devAddr->function)))
+goto cleanup;
+
+if (STREQ_NULLABLE(pci->stubDriver, "vfio-pci"))
+ret = -1;
+
+ cleanup:
+virPCIDeviceFree(pci);
+return ret;
+}
+
static int
virPCIDeviceUnbindFromStub(virPCIDevicePtr dev,
virPCIDeviceListPtr activeDevs ATTRIBUTE_UNUSED,
@@ -1177,11 +1194,34 @@ virPCIDeviceUnbindFromStub(virPCIDevicePtr dev,
dev->remove_slot = false;
reprobe:
-if (virPCIDeviceReprobeHostDriver(dev, driver, drvdir) < 0)
-goto cleanup;
-/* Steal the dev from list inactiveDevs */
-if (inactiveDevs)
-virPCIDeviceListDel(inactiveDevs, dev);
+if (STREQ_NULLABLE(dev->stubDriver, "vfio-pci")) {
+size_t i = 0;
+virPCIDeviceAddress devAddr = { dev->domain, dev->bus,
+dev->slot, dev->function };
+if (virPCIDeviceAddressIOMMUGroupIterate(&devAddr,
virPCIDeviceBoundToVFIODriver, NULL) < 0) {
+result = 0;
+goto cleanup;
+}
+
+while (inactiveDevs && (i < virPCIDeviceListCount(inactiveDevs))) {
+virPCIDevicePtr pcidev = virPCIDeviceListGet(inactiveDevs, i);
+if (dev->iommuGroup == pcidev->iommuGroup) {
+if (virPCIDeviceReprobeHostDriver(pcidev, driver, drvdir) < 0)
+ goto cleanup;
+/* Steal the dev from list inactiveDevs */
+virPCIDeviceListDel(inactiveDevs, pcidev);
+continue;
+}
+i++;
+}
+} else {
+if (virPCIDeviceReprobeHostDriver(dev, driver, drvdir) < 0)
+goto cleanup;
+/* Steal the dev from list inactiveDevs */
+
+if (inactiveDevs)
+virPCIDeviceListDel(inactiveDevs, dev);
+}
result = 0;
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/8] Split reprobe action from the virPCIUnbindFromStub into a new function
The reprobe needs to be called multiple times for vfio devices for each
device in the iommu group in future patch. So split the reprobe into a
new function. No functional change.
Signed-off-by: Shivaprasad G Bhat
---
src/util/virpci.c | 54 +++--
1 file changed, 32 insertions(+), 22 deletions(-)
diff --git a/src/util/virpci.c b/src/util/virpci.c
index eba285a..2709ddd 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -1097,6 +1097,37 @@ static bool virPCIIsAKnownStub(char *driver)
return ret;
}
+static int virPCIDeviceReprobeHostDriver(virPCIDevicePtr dev, char *driver,
char *drvdir)
+{
+char *path = NULL;
+int result = -1;
+
+if (!dev->reprobe)
+return 0;
+
+/* Trigger a re-probe of the device is not in the stub's dynamic
+ * ID table. If the stub is available, but 'remove_id' isn't
+ * available, then re-probing would just cause the device to be
+ * re-bound to the stub.
+ */
+if (driver && !(path = virPCIDriverFile(driver, "remove_id")))
+goto cleanup;
+
+if (!driver || !virFileExists(drvdir) || virFileExists(path)) {
+if (virFileWriteStr(PCI_SYSFS "drivers_probe", dev->name, 0) < 0) {
+virReportSystemError(errno,
+ _("Failed to trigger a re-probe for PCI
device '%s'"),
+ dev->name);
+goto cleanup;
+}
+}
+result = 0;
+ cleanup:
+VIR_FREE(path);
+dev->reprobe = false;
+return result;
+}
+
static int
virPCIDeviceUnbindFromStub(virPCIDevicePtr dev)
{
@@ -1144,28 +1175,8 @@ virPCIDeviceUnbindFromStub(virPCIDevicePtr dev)
dev->remove_slot = false;
reprobe:
-if (!dev->reprobe) {
-result = 0;
+if (virPCIDeviceReprobeHostDriver(dev, driver, drvdir) < 0)
goto cleanup;
-}
-
-/* Trigger a re-probe of the device is not in the stub's dynamic
- * ID table. If the stub is available, but 'remove_id' isn't
- * available, then re-probing would just cause the device to be
- * re-bound to the stub.
- */
-VIR_FREE(path);
-if (driver && !(path = virPCIDriverFile(driver, "remove_id")))
-goto cleanup;
-
-if (!driver || !virFileExists(drvdir) || virFileExists(path)) {
-if (virFileWriteStr(PCI_SYSFS "drivers_probe", dev->name, 0) < 0) {
-virReportSystemError(errno,
- _("Failed to trigger a re-probe for PCI
device '%s'"),
- dev->name);
-goto cleanup;
-}
-}
result = 0;
@@ -1173,7 +1184,6 @@ virPCIDeviceUnbindFromStub(virPCIDevicePtr dev)
/* do not do it again */
dev->unbind_from_stub = false;
dev->remove_slot = false;
-dev->reprobe = false;
VIR_FREE(drvdir);
VIR_FREE(path);
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/8] Wait for vfio-pci device cleanups before reassinging the device to host driver
Before unbind from stub driver libvirt should be sure the guest is not using
the device anymore. The libvirt today waits for pci-stub driver alone in
/proc/iommu.
The same wait is needed for vfio devices too.
Signed-off-by: Shivaprasad G Bhat
---
src/util/virhostdev.c |7 +++
1 file changed, 7 insertions(+)
diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index 91f28e9..6247477 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -756,6 +756,13 @@ virHostdevReattachPCIDevice(virPCIDevicePtr dev,
virHostdevManagerPtr mgr)
usleep(100*1000);
retries--;
}
+} else if (STREQ(virPCIDeviceGetStubDriver(dev), "vfio-pci")) {
+int retries = 100;
+while (virPCIDeviceWaitForCleanup(dev, "vfio-pci")
+ && retries) {
+usleep(100*1000);
+retries--;
+}
}
if (virPCIDeviceReattach(dev, mgr->activePCIHostdevs,
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/8] Refuse to reattach from vfio if the iommu group is in use by any domain
It is incorrect to attempt the device reattach of a function,
when some other domain is using some functions belonging to the same iommu
group.
Signed-off-by: Shivaprasad G Bhat
---
src/util/virhostdev.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index de029a0..91f28e9 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -1590,6 +1590,7 @@ virHostdevPCINodeDeviceReAttach(virHostdevManagerPtr
hostdev_mgr,
virPCIDevicePtr pci)
{
virPCIDeviceAddressPtr devAddr = NULL;
+bool usesVfio = STREQ_NULLABLE(virPCIDeviceGetStubDriver(pci), "vfio-pci");
struct virHostdevIsPCINodeDeviceUsedData data = {hostdev_mgr, NULL,
false};
int ret = -1;
@@ -1600,8 +1601,16 @@ virHostdevPCINodeDeviceReAttach(virHostdevManagerPtr
hostdev_mgr,
if (!(devAddr = virPCIDeviceGetAddress(pci)))
goto out;
-if (virHostdevIsPCINodeDeviceUsed(devAddr, &data))
+if (usesVfio) {
+/* Doesn't matter which function. If any domain is actively using the
+ iommu group, refuse to reattach */
+if (virPCIDeviceAddressIOMMUGroupIterate(devAddr,
+ virHostdevIsPCINodeDeviceUsed,
+ &data) < 0)
+goto out;
+} else if (virHostdevIsPCINodeDeviceUsed(devAddr, &data)) {
goto out;
+}
virPCIDeviceReattachInit(pci);
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/8] Add iommu group number info to virPCIDevice
The iommu group number need not be fetched from the sysfs
everytime as it remains constant. Fetch it once during
allocation
Signed-off-by: Shivaprasad G Bhat
---
src/util/virpci.c |5 +
1 file changed, 5 insertions(+)
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 5acf486..eba285a 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -75,6 +75,7 @@ struct _virPCIDevice {
bool has_pm_reset;
bool managed;
char *stubDriver;
+unsigned int iommuGroup;
/* used by reattach function */
bool unbind_from_stub;
@@ -1565,6 +1566,8 @@ virPCIDeviceNew(unsigned int domain,
char *product = NULL;
char *drvpath = NULL;
char *driver = NULL;
+virPCIDeviceAddress devAddr = { domain, bus,
+slot, function };
if (VIR_ALLOC(dev) < 0)
return NULL;
@@ -1618,6 +1621,8 @@ virPCIDeviceNew(unsigned int domain,
if (virPCIIsAKnownStub(driver))
dev->stubDriver = driver;
+dev->iommuGroup = virPCIDeviceAddressGetIOMMUGroupNum(&devAddr);
+
VIR_DEBUG("%s %s: initialized", dev->id, dev->name);
cleanup:
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/8] Initialize the stubDriver of pci devices if bound to a valid one
The stubDriver name can be used to make useful decisions if readily available.
Set it if bound to a valid one during initialisation.
Signed-off-by: Shivaprasad G Bhat
---
src/util/virpci.c | 36 ++--
1 file changed, 26 insertions(+), 10 deletions(-)
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 35b1459..5acf486 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -1080,6 +1080,22 @@ static const char *virPCIKnownStubs[] = {
NULL
};
+static bool virPCIIsAKnownStub(char *driver)
+{
+const char **stubTest;
+bool ret = false;
+
+for (stubTest = virPCIKnownStubs; *stubTest != NULL; stubTest++) {
+if (STREQ_NULLABLE(driver, *stubTest)) {
+ret = true;
+VIR_DEBUG("Found stub driver %s", *stubTest);
+break;
+}
+}
+
+return ret;
+}
+
static int
virPCIDeviceUnbindFromStub(virPCIDevicePtr dev)
{
@@ -1087,8 +1103,6 @@ virPCIDeviceUnbindFromStub(virPCIDevicePtr dev)
char *drvdir = NULL;
char *path = NULL;
char *driver = NULL;
-const char **stubTest;
-bool isStub = false;
/* If the device is currently bound to one of the "well known"
* stub drivers, then unbind it, otherwise ignore it.
@@ -1105,14 +1119,7 @@ virPCIDeviceUnbindFromStub(virPCIDevicePtr dev)
goto remove_slot;
/* If the device isn't bound to a known stub, skip the unbind. */
-for (stubTest = virPCIKnownStubs; *stubTest != NULL; stubTest++) {
-if (STREQ(driver, *stubTest)) {
-isStub = true;
-VIR_DEBUG("Found stub driver %s", *stubTest);
-break;
-}
-}
-if (!isStub)
+if (!virPCIIsAKnownStub(driver))
goto remove_slot;
if (virPCIDeviceUnbind(dev, dev->reprobe) < 0)
@@ -1556,6 +1563,8 @@ virPCIDeviceNew(unsigned int domain,
virPCIDevicePtr dev;
char *vendor = NULL;
char *product = NULL;
+char *drvpath = NULL;
+char *driver = NULL;
if (VIR_ALLOC(dev) < 0)
return NULL;
@@ -1603,9 +1612,16 @@ virPCIDeviceNew(unsigned int domain,
goto error;
}
+if (virPCIDeviceGetDriverPathAndName(dev, &drvpath, &driver) < 0)
+goto cleanup;
+
+if (virPCIIsAKnownStub(driver))
+dev->stubDriver = driver;
+
VIR_DEBUG("%s %s: initialized", dev->id, dev->name);
cleanup:
+VIR_FREE(drvpath);
VIR_FREE(product);
VIR_FREE(vendor);
return dev;
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] gobject: Add wrapper virDomainSetTime()
---
This version:
* Replaces the seconds and nseconds arguments by a GDateTime.
* Drops the use of flags argument, since caller can specify the only flag
currently possible (VIR_DOMAIN_TIME_SYNC) by simply passing a NULL as the
GDateTime argument.
* Add some needed articles to doc comment.
libvirt-gobject/libvirt-gobject-domain.c | 121 +++
libvirt-gobject/libvirt-gobject-domain.h | 15 +++-
libvirt-gobject/libvirt-gobject.sym | 9 +++
3 files changed, 144 insertions(+), 1 deletion(-)
diff --git a/libvirt-gobject/libvirt-gobject-domain.c
b/libvirt-gobject/libvirt-gobject-domain.c
index 34eb7ca..debae2d 100644
--- a/libvirt-gobject/libvirt-gobject-domain.c
+++ b/libvirt-gobject/libvirt-gobject-domain.c
@@ -1886,3 +1886,124 @@ gboolean
gvir_domain_get_has_current_snapshot(GVirDomain *dom,
return TRUE;
}
+
+/**
+ * gvir_domain_set_time:
+ * @dom: the domain
+ * @date_time: (allow-none)(transfer none): the time to set as #GDateTime.
+ * @flags: Unused, Pass 0.
+ *
+ * This function tries to set guest time to the given value. The passed
+ * time must in UTC.
+ *
+ * If @date_time is %NULL, the time is reset using the domain's RTC.
+ *
+ * Please note that some hypervisors may require guest agent to be configured
+ * and running in order for this function to work.
+ */
+gboolean gvir_domain_set_time(GVirDomain *dom,
+ GDateTime *date_time,
+ guint flags G_GNUC_UNUSED,
+ GError **err)
+{
+GVirDomainPrivate *priv;
+int ret;
+GTimeVal tv;
+gint64 seconds;
+guint nseconds;
+guint settime_flags;
+
+g_return_val_if_fail(GVIR_IS_DOMAIN(dom), FALSE);
+g_return_val_if_fail(err == NULL || *err == NULL, FALSE);
+
+if (date_time != NULL) {
+if (!g_date_time_to_timeval(date_time, &tv)) {
+gvir_set_error_literal(err, GVIR_DOMAIN_ERROR,
+ 0,
+ "Failed to parse given time argument");
+return FALSE;
+}
+
+seconds = tv.tv_sec;
+nseconds = tv.tv_usec * 1000;
+settime_flags = 0;
+} else {
+seconds = 0;
+nseconds = 0;
+settime_flags = VIR_DOMAIN_TIME_SYNC;
+}
+
+priv = dom->priv;
+ret = virDomainSetTime(priv->handle, seconds, nseconds, settime_flags);
+if (ret < 0) {
+gvir_set_error_literal(err, GVIR_DOMAIN_ERROR,
+ 0,
+ "Unable to set domain time");
+return FALSE;
+}
+
+return TRUE;
+}
+
+static void
+gvir_domain_set_time_helper(GTask *task,
+gpointer object,
+gpointer task_data,
+GCancellable *cancellable G_GNUC_UNUSED)
+{
+GVirDomain *dom = GVIR_DOMAIN(object);
+GDateTime *date_time = (GDateTime *) task_data;
+GError *err = NULL;
+
+if (!gvir_domain_set_time(dom, date_time, 0, &err))
+g_task_return_error(task, err);
+else
+g_task_return_boolean(task, TRUE);
+}
+
+/**
+ * gvir_domain_set_time_async:
+ * @dom: the domain
+ * @date_time: (allow-none)(transfer none): the time to set as #GDateTime.
+ * @flags: bitwise-OR of #GVirDomainSetTimeFlags.
+ * @cancellable: (allow-none)(transfer none): cancellation object
+ * @callback: (scope async): completion callback
+ * @user_data: (closure): opaque data for callback
+ *
+ * Asynchronous variant of #gvir_domain_set_time.
+ */
+void gvir_domain_set_time_async(GVirDomain *dom,
+GDateTime *date_time,
+guint flags G_GNUC_UNUSED,
+GCancellable *cancellable,
+GAsyncReadyCallback callback,
+gpointer user_data)
+{
+GTask *task;
+
+g_return_if_fail(GVIR_IS_DOMAIN(dom));
+g_return_if_fail((cancellable == NULL) || G_IS_CANCELLABLE(cancellable));
+
+task = g_task_new(G_OBJECT(dom),
+ cancellable,
+ callback,
+ user_data);
+if (date_time != NULL)
+g_task_set_task_data(task,
+ g_date_time_ref(date_time),
+ (GDestroyNotify)g_date_time_unref);
+g_task_run_in_thread(task, gvir_domain_set_time_helper);
+
+g_object_unref(task);
+}
+
+gboolean gvir_domain_set_time_finish(GVirDomain *dom,
+ GAsyncResult *result,
+ GError **err)
+{
+g_return_val_if_fail(GVIR_IS_DOMAIN(dom), FALSE);
+g_return_val_if_fail(g_task_is_valid(result, G_OBJECT(dom)), FALSE);
+g_return_val_if_fail(err == NULL || *err == NULL, FALSE);
+
+return g_task_propagate_boolean(G_TASK(result), err);
+}
diff --git a/libvirt-gobject/libvirt-gobject-domain.h
b/libvirt-gobject/libvirt-gobject-domain.h
index
[libvirt] [libvirt-glib 3/3] gobject: Port to GTask API
Drop usage of deprecated GSimpleAsyncResult API.
---
libvirt-gobject/libvirt-gobject-domain.c| 270 ++-
libvirt-gobject/libvirt-gobject-input-stream.c | 76 +++
libvirt-gobject/libvirt-gobject-output-stream.c | 75 +++
libvirt-gobject/libvirt-gobject-storage-pool.c | 281 ++--
libvirt-gobject/libvirt-gobject-stream.c| 46 ++--
5 files changed, 326 insertions(+), 422 deletions(-)
diff --git a/libvirt-gobject/libvirt-gobject-domain.c
b/libvirt-gobject/libvirt-gobject-domain.c
index 34eb7ca..dda9268 100644
--- a/libvirt-gobject/libvirt-gobject-domain.c
+++ b/libvirt-gobject/libvirt-gobject-domain.c
@@ -380,18 +380,19 @@ static void domain_start_data_free(DomainStartData *data)
}
static void
-gvir_domain_start_helper(GSimpleAsyncResult *res,
- GObject *object,
+gvir_domain_start_helper(GTask *task,
+ gpointer source_object,
+ gpointer task_data,
GCancellable *cancellable G_GNUC_UNUSED)
{
-GVirDomain *dom = GVIR_DOMAIN(object);
-DomainStartData *data;
+GVirDomain *dom = GVIR_DOMAIN(source_object);
+DomainStartData *data = (DomainStartData *) task_data;
GError *err = NULL;
-data = g_simple_async_result_get_op_res_gpointer(res);
-
if (!gvir_domain_start(dom, data->flags, &err))
-g_simple_async_result_take_error(res, err);
+g_task_return_error(task, err);
+else
+g_task_return_boolean(task, TRUE);
}
/**
@@ -410,7 +411,7 @@ void gvir_domain_start_async(GVirDomain *dom,
GAsyncReadyCallback callback,
gpointer user_data)
{
-GSimpleAsyncResult *res;
+GTask *task;
DomainStartData *data;
g_return_if_fail(GVIR_IS_DOMAIN(dom));
@@ -419,16 +420,13 @@ void gvir_domain_start_async(GVirDomain *dom,
data = g_slice_new0(DomainStartData);
data->flags = flags;
-res = g_simple_async_result_new(G_OBJECT(dom),
-callback,
-user_data,
-gvir_domain_start_async);
-g_simple_async_result_set_op_res_gpointer (res, data,
(GDestroyNotify)domain_start_data_free);
-g_simple_async_result_run_in_thread(res,
-gvir_domain_start_helper,
-G_PRIORITY_DEFAULT,
-cancellable);
-g_object_unref(res);
+task = g_task_new(G_OBJECT(dom),
+ cancellable,
+ callback,
+ user_data);
+g_task_set_task_data(task, data, (GDestroyNotify)domain_start_data_free);
+g_task_run_in_thread(task, gvir_domain_start_helper);
+g_object_unref(task);
}
gboolean gvir_domain_start_finish(GVirDomain *dom,
@@ -436,13 +434,10 @@ gboolean gvir_domain_start_finish(GVirDomain *dom,
GError **err)
{
g_return_val_if_fail(GVIR_IS_DOMAIN(dom), FALSE);
-g_return_val_if_fail(g_simple_async_result_is_valid(result, G_OBJECT(dom),
gvir_domain_start_async), FALSE);
+g_return_val_if_fail(g_task_is_valid(result, dom), FALSE);
g_return_val_if_fail(err == NULL || *err == NULL, FALSE);
-if (g_simple_async_result_propagate_error(G_SIMPLE_ASYNC_RESULT(result),
err))
-return FALSE;
-
-return TRUE;
+return g_task_propagate_boolean(G_TASK(result), err);
}
/**
@@ -472,15 +467,18 @@ gboolean gvir_domain_resume(GVirDomain *dom,
}
static void
-gvir_domain_resume_helper(GSimpleAsyncResult *res,
- GObject *object,
+gvir_domain_resume_helper(GTask *task,
+ gpointer source_object,
+ gpointer task_data G_GNUC_UNUSED,
GCancellable *cancellable G_GNUC_UNUSED)
{
-GVirDomain *dom = GVIR_DOMAIN(object);
+GVirDomain *dom = GVIR_DOMAIN(source_object);
GError *err = NULL;
if (!gvir_domain_resume(dom, &err))
-g_simple_async_result_take_error(res, err);
+g_task_return_error(task, err);
+else
+g_task_return_boolean(task, TRUE);
}
/**
@@ -497,20 +495,17 @@ void gvir_domain_resume_async(GVirDomain *dom,
GAsyncReadyCallback callback,
gpointer user_data)
{
-GSimpleAsyncResult *res;
+GTask *task;
g_return_if_fail(GVIR_IS_DOMAIN(dom));
g_return_if_fail((cancellable == NULL) || G_IS_CANCELLABLE(cancellable));
-res = g_simple_async_result_new(G_OBJECT(dom),
-callback,
-user_data,
-gvir_domain_resume_async);
-g_simple_async_result_run_in_thread(res,
-gvir_domain_resume_helper,
-G_P
[libvirt] [libvirt-glib 1/3] gobject: Drop redundant glib compatibility code
We already require and use glib >= 2.36 so there is no reason to keep
around code to ensure compatibility with glib oler than that.
---
libvirt-gobject/libvirt-gobject-compat.c | 87
libvirt-gobject/libvirt-gobject-compat.h | 73 ---
2 files changed, 160 deletions(-)
diff --git a/libvirt-gobject/libvirt-gobject-compat.c
b/libvirt-gobject/libvirt-gobject-compat.c
index 14b5eb3..e91b018 100644
--- a/libvirt-gobject/libvirt-gobject-compat.c
+++ b/libvirt-gobject/libvirt-gobject-compat.c
@@ -17,93 +17,6 @@
#include "libvirt-gobject-compat.h"
-#if !GLIB_CHECK_VERSION(2,28,0)
-/**
- * g_simple_async_result_take_error: (skip)
- * @simple: a #GSimpleAsyncResult
- * @error: a #GError
- *
- * Sets the result from @error, and takes over the caller's ownership
- * of @error, so the caller does not need to free it any more.
- *
- * Since: 2.28
- **/
-G_GNUC_INTERNAL void
-g_simple_async_result_take_error (GSimpleAsyncResult *simple,
- GError *error)
-{
-/* this code is different from upstream */
-/* we can't avoid extra copy/free, since the simple struct is
- opaque */
-g_simple_async_result_set_from_error (simple, error);
-g_error_free (error);
-}
-
-/**
- * g_simple_async_result_new_take_error: (skip)
- * @source_object: (allow-none): a #GObject, or %NULL
- * @callback: (scope async): a #GAsyncReadyCallback
- * @user_data: (closure): user data passed to @callback
- * @error: a #GError
- *
- * Creates a #GSimpleAsyncResult from an error condition, and takes over the
- * caller's ownership of @error, so the caller does not need to free it
anymore.
- *
- * Returns: a #GSimpleAsyncResult
- *
- * Since: 2.28
- **/
-GSimpleAsyncResult *
-g_simple_async_result_new_take_error(GObject *source_object,
- GAsyncReadyCallback callback,
- gpointer user_data,
- GError *error)
-{
-GSimpleAsyncResult *simple;
-
-g_return_val_if_fail(!source_object || G_IS_OBJECT(source_object), NULL);
-
-simple = g_simple_async_result_new(source_object,
- callback,
- user_data, NULL);
-g_simple_async_result_take_error(simple, error);
-
-return simple;
-}
-
-/**
- * g_simple_async_report_take_gerror_in_idle: (skip)
- * @object: (allow-none): a #GObject, or %NULL
- * @callback: a #GAsyncReadyCallback.
- * @user_data: user data passed to @callback.
- * @error: the #GError to report
- *
- * Reports an error in an idle function. Similar to
- * g_simple_async_report_gerror_in_idle(), but takes over the caller's
- * ownership of @error, so the caller does not have to free it any more.
- *
- * Since: 2.28
- **/
-void
-g_simple_async_report_take_gerror_in_idle(GObject *object,
- GAsyncReadyCallback callback,
- gpointer user_data,
- GError *error)
-{
-GSimpleAsyncResult *simple;
-
-g_return_if_fail(!object || G_IS_OBJECT(object));
-g_return_if_fail(error != NULL);
-
-simple = g_simple_async_result_new_take_error(object,
- callback,
- user_data,
- error);
-g_simple_async_result_complete_in_idle(simple);
-g_object_unref(simple);
-}
-#endif
-
GMutex *gvir_mutex_new(void)
{
GMutex *mutex;
diff --git a/libvirt-gobject/libvirt-gobject-compat.h
b/libvirt-gobject/libvirt-gobject-compat.h
index 2e87966..27fa305 100644
--- a/libvirt-gobject/libvirt-gobject-compat.h
+++ b/libvirt-gobject/libvirt-gobject-compat.h
@@ -35,77 +35,4 @@ GMutex *gvir_mutex_new(void);
#endif
-#if !GLIB_CHECK_VERSION(2,26,0)
-#define G_DEFINE_BOXED_TYPE(TypeName, type_name, copy_func, free_func)
G_DEFINE_BOXED_TYPE_WITH_CODE (TypeName, type_name, copy_func, free_func, {})
-#define G_DEFINE_BOXED_TYPE_WITH_CODE(TypeName, type_name, copy_func,
free_func, _C_) _G_DEFINE_BOXED_TYPE_BEGIN (TypeName, type_name, copy_func,
free_func) {_C_;} _G_DEFINE_TYPE_EXTENDED_END()
-#if __GNUC__ > 2 || (__GNUC__ == 2 && __GNUC_MINOR__ >= 7)
-#define _G_DEFINE_BOXED_TYPE_BEGIN(TypeName, type_name, copy_func, free_func) \
-GType \
-type_name##_get_type (void) \
-{ \
- static volatile gsize g_define_type_id__volatile = 0; \
- if (g_once_init_enter (&g_define_type_id__volatile)) \
-{ \
- GType (* _g_register_boxed) \
-(const gchar *, \
- union \
- { \
- TypeName * (*do_copy_type) (TypeName *); \
- TypeName * (*do_const_copy_type) (const TypeName *); \
- GBoxedCopyFunc do_copy_boxed; \
- } __attribute__((__transparent_union__)), \
- union \
- { \
- void (* do_free_type) (Ty
[libvirt] [libvirt-glib 2/3] gconfig: Drop redundant glib compatibility code
We already require and use glib >= 2.36 so there is no reason to keep
around code to ensure compatibility with glib oler than that.
---
libvirt-gconfig/libvirt-gconfig-compat.h | 20
1 file changed, 20 deletions(-)
diff --git a/libvirt-gconfig/libvirt-gconfig-compat.h
b/libvirt-gconfig/libvirt-gconfig-compat.h
index fbf552c..c9ac645 100644
--- a/libvirt-gconfig/libvirt-gconfig-compat.h
+++ b/libvirt-gconfig/libvirt-gconfig-compat.h
@@ -25,26 +25,6 @@
#include
-#if !GLIB_CHECK_VERSION(2,32,0)
-
-#if__GNUC__ > 3 || (__GNUC__ == 3 && __GNUC_MINOR__ >= 1)
-#define G_DEPRECATED __attribute__((__deprecated__))
-#elif defined(_MSC_VER) && (_MSC_VER >= 1300)
-#define G_DEPRECATED __declspec(deprecated)
-#else
-#define G_DEPRECATED
-#endif
-
-#if__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 5)
-#define G_DEPRECATED_FOR(f) __attribute__((__deprecated__("Use '" #f "'
instead")))
-#elif defined(_MSC_FULL_VER) && (_MSC_FULL_VER > 140050320)
-#define G_DEPRECATED_FOR(f) __declspec(deprecated("is deprecated. Use '" #f "'
instead"))
-#else
-#define G_DEPRECATED_FOR(f) G_DEPRECATED
-#endif
-
-#endif
-
#if GLIB_CHECK_VERSION(2, 35, 0)
#define g_type_init()
#endif
--
2.5.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Drop deprecated API use + redundant compat. code
Couldn't come up with a better summary here, sorry but these patches simply * Drop usage of deprecated GSimpleAsyncResult API. * Drop redundant compatibility API. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: set error if DAD is not finished
On 10/29/2015 12:15 PM, Laine Stump wrote: > > On my system, these are set to 1, 1, and 1000, and I've found that DAD > takes something between 5.7 and 6.8 seconds to complete (it was at the > lower end with a single IP address, and at the high end with 70 IP > addresses (or also with 20 IPs, so I don't think it's going to get > substantially higher). (Too bad I didn't do this *before* I pushed, > rather than relying on a successful build and reports of proper > operation on your system :-/) > > Based on that, I think it makes sense to push a patch that sets the > timeout to 20 seconds (and push Luyao's patch ragardless). Since the > timeout is meant to catch an "infinite" wait, I think 20 seconds is > okay; I don't want to add yet another tunable parameter unless we really > need to. > > I'm pushing Luyao's patch now, but will wait for an ACK to push the > patch I've attached here. Anyone? > Looks reasonable to me, and safe for inclusion in the current release. ACK -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] RAM backend and guest ABI (was Re: [Qemu-devel] [PATCH v2] pc: memhp: enforce minimal 128Mb) alignment for pc-dimm
(CCing Michal and libvir-list, so libvirt team is aware of this
restriction)
On Thu, Oct 29, 2015 at 02:36:37PM +0100, Igor Mammedov wrote:
> On Tue, 27 Oct 2015 14:36:35 -0200
> Eduardo Habkost wrote:
>
> > On Tue, Oct 27, 2015 at 10:14:56AM +0100, Igor Mammedov wrote:
> > > On Tue, 27 Oct 2015 10:53:08 +0200
> > > "Michael S. Tsirkin" wrote:
> > >
> > > > On Tue, Oct 27, 2015 at 09:48:37AM +0100, Igor Mammedov wrote:
> > > > > On Tue, 27 Oct 2015 10:31:21 +0200
> > > > > "Michael S. Tsirkin" wrote:
> > > > >
> > > > > > On Mon, Oct 26, 2015 at 02:24:32PM +0100, Igor Mammedov wrote:
> > > > > > > Yep it's workaround but it works around QEMU's broken virtio
> > > > > > > implementation in a simple way without need for guest side
> > > > > > > changes.
> > > > > > >
> > > > > > > Without foreseeable virtio fix it makes memory hotplug unusable
> > > > > > > and even
> > > > > > > more so if there were a virtio fix it won't fix old guests since
> > > > > > > you've
> > > > > > > said that virtio fix would require changes of both QEMU and guest
> > > > > > > sides.
> > > > > >
> > > > > > What makes it not foreseeable?
> > > > > > Apparently only the fact that we have a work-around in place so no
> > > > > > one
> > > > > > works on it. I can code it up pretty quickly, but I'm flat out of
> > > > > > time
> > > > > > for testing as I'm going on vacation soon, and hard freeze is pretty
> > > > > > close.
> > > > > I can lend a hand for testing part.
> > > > >
> > > > > >
> > > > > > GPA space is kind of cheap, but wasting it in chunks of 512M
> > > > > > seems way too aggressive.
> > > > > hotplug region is sized with 1Gb alignment reserve per DIMM so we
> > > > > aren't
> > > > > actually wasting anything here.
> > > > >
> > > >
> > > > If I allocate two 1G DIMMs, what will be the gap size? 512M? 1G?
> > > > It's too much either way.
> > > minimum would be 512, and if backend is 1Gb-hugepage gap will be
> > > backend's natural alignment (i.e. 1Gb).
> >
> > Is backend configuration even allowed to affect the machine ABI? We need
> > to be able to change backend configuration when migrating the VM to
> > another host.
> for now, one has to use the same type of backend on both sides
> i.e. if source uses 1Gb huge pages backend then target also
> need to use it.
>
The page size of the backend don't even depend on QEMU arguments, but on
the kernel command-line or hugetlbfs mount options. So it's possible to
have exactly the same QEMU command-line on source and destination (with
an explicit versioned machine-type), and get a VM that can't be
migrated? That means we are breaking our guarantees about migration and
guest ABI.
> We could change this for the next machine type to always force
> max alignment (1Gb), then it would be possible to change
> between backends with different alignments.
I'm not sure what's the best solution here. If always using 1GB is too
aggressive, we could require management to ask for an explicit alignment
as a -machine option if they know they will need a specific backend page
size.
BTW, are you talking about the behavior introduced by
aa8580cddf011e8cedcf87f7a0fdea7549fc4704 ("pc: memhp: force gaps between
DIMM's GPA") only, or the backend page size was already affecting GPA
allocation before that commit?
--
Eduardo
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: set error if DAD is not finished
On 10/29/2015 08:32 AM, Maxim Perevedentsev wrote:
On 10/29/2015 12:47 PM, Luyao Huang wrote:
If DAD not finished in 5 seconds, user will get an
unknown error like this:
# virsh net-start ipv6
error: Failed to start network ipv6
error: An error occurred, but the cause is unknown
Call virReportError to set an error.
Signed-off-by: Luyao Huang
---
I found the DAD will take 7 seconds
on my machine, and i cannot create a network which
use ipv6 now :( . Can we offer a way allow user to change this
timeout ? maybe add a configuration file option in
libvirtd.conf.
Could you please send the related records of your sysctl -a?
Max DAD timeout should be
rand() % net.ipv6.conf.default.router_solicitation_delay +
net.ipv6.conf.default.dad_transmits *
net.ipv6.neigh.default.retrans_time_ms
I wonder whether my calculations were faulty or it is your specific
configuration.
On my system, these are set to 1, 1, and 1000, and I've found that DAD
takes something between 5.7 and 6.8 seconds to complete (it was at the
lower end with a single IP address, and at the high end with 70 IP
addresses (or also with 20 IPs, so I don't think it's going to get
substantially higher). (Too bad I didn't do this *before* I pushed,
rather than relying on a successful build and reports of proper
operation on your system :-/)
Based on that, I think it makes sense to push a patch that sets the
timeout to 20 seconds (and push Luyao's patch ragardless). Since the
timeout is meant to catch an "infinite" wait, I think 20 seconds is
okay; I don't want to add yet another tunable parameter unless we really
need to.
I'm pushing Luyao's patch now, but will wait for an ACK to push the
patch I've attached here. Anyone?
>From 559f9dbd72e7ca8c252134b87a6d0f57b4b2227b Mon Sep 17 00:00:00 2001
From: Laine Stump
Date: Thu, 29 Oct 2015 14:09:59 -0400
Subject: [PATCH] util: set max wait for IPv6 DAD to 20 seconds
This was originally set to 5 seconds, but times of 5.5 to 7 seconds
were experienced. Since it's an arbitrary number intended to prevent
an infinite hang, having it a bit too high won't hurt anything, and 20
seconds looks to be adequate (i.e. I think/hope we don't need to make
it tunable in libvirtd.conf)
---
src/util/virnetdev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index b84437e..7bb7b15 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -96,7 +96,7 @@ VIR_LOG_INIT("util.netdev");
# define FEATURE_BIT_IS_SET(blocks, index, field)\
(FEATURE_WORD(blocks, index, field) & FEATURE_FIELD_FLAG(index))
#endif
-#define VIR_DAD_WAIT_TIMEOUT 5 /* seconds */
+#define VIR_DAD_WAIT_TIMEOUT 20 /* seconds */
typedef enum {
VIR_MCAST_TYPE_INDEX_TOKEN,
--
2.4.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC: Improve performance of macvtap device creation
On 10/29/2015 12:49 PM, Tony Krowiak wrote: For a guest domain defined with a large number of macvtap devices, it takes an exceedingly long time to boot the guest. In a test of a guest domain configured with 82 macvtap devices, it took over two minutes for the guest to boot. An strace of the ioctl calls during guest start up showed the SIOCGIFFLAGS ioctl literally being invoked 3,403 times. I was able to isolate the source of the ioctl calls to the*virNetDevMacVLanCreateWithVPortProfile* function in*virnetdevmacvlan.c*. The macvtap interface name is created by looping over a counter variable, starting with zero, and appending the counter value to 'macvtap'. I've wondered ever since the first time I saw that code why it was done that way, and why there had never been any performance complaints. Lacking any complaints, I promptly forgot about it (until the next time I went past the code for some other tangentially related reason.) Since you're the first to complain, you have the honor of fixing it :-) With each iteration, a call is made to*virNetDevExists* (SIOCGIFFLAGS ioctl) to determine if a device with that name already exists, until a unique name is created. In the test case cited above, to create an interface name for the 82nd macvtap device, the*virNetDevExists* function will be called for interface names 'macvtap0' to 'macvtap80' before it is determined that 'mavtap81' can be used. So if N is the number of macvtap interfaces defined for a guest, the SIOCGIFFLAGS ioctl will be invoked (N x N + N)/2 times to find an unused macvtap device names. That's assuming only one guest is being started, who knows how many times the ioctl may have to be called in an installation running a large number of guests defined with macvtap devices. I was able to reduce the amount of time for starting a guest domain defined with 82 macvtap devices from over 2 minutes to about 14 seconds by keeping track of the interface name suffixes previously used. I defined two static bit maps (virBitmap), one each for macvtap and macvlan device name suffixes. When a macvtap/macvlan device is created, the index of the next clear bit (virBitmapNextClearBit) is retrieved to create the name. If an interface with that name does not exist, the device is created and the bit at the index used to create the interface name is set (virBitmapSetBit). When a macvtap/macvlan device is deleted, if the interface name has the pattern 'macvtap%d' or 'macvlan%d', the suffix is parsed into a bit index and used to clear the (virBitMapClearBit) bit in the respective bitmap. This sounds fine, as long as 1) you recreate the bitmap whenever libvirtd is restarted (while scanning through all the interfaces of every domain; there is already code being executed in exactly the right place - look for qemu_process.c:qemuProcessNotifyNets() and add appropriate code inside the loop there), and 2) you retry some number of times if a supposedly unused device name is actually in use (to account for processes other than libvirt using the same naming convention). I am not sure that is the best design because there is no way to track interface names used to create macvtap devices outside of libvirt, for example using the ip command. If you wanted to get *really* complicated, you could use netlink to get a list of all network devices, or even monitor netlink traffic to maintain your own cache, but I think that's serious overkill (until proven otherwise). There may also be other issues I've not contemplated. I included a couple of additional ideas below and am looking for comments or other suggestions that I have not considered. * Define a global counter variable initialized to 0, that gets incremented each time an interface name is created, to keep track of the last used interface name suffix. At some maximum value, the counter will be set back to 0. There could be some merit to this, as it is simpler and likely faster. You would need to maintain the counter somewhere in persistent storage so it could be retrieved when libvirtd is restarted though. * Append a random number to 'macvlan' or 'macvtap' when creating the interface name. Of course, the number of digits would have to be limited so the interface name would not exceed the maximum allowed. Well, that has the advantage that no persistent state information is required. * Create the interface name in code that has more knowledge of the environment and pass the name into the *virNetDevMacVLanCreateWithVPortProfile* function via the *tgifname* parameter. For example, the *qemuBuildCommandLine* function in *qemu_command.c* contains the loop that iterates over the network devices defined for the guest domain that ultimately get created via the *virNetDevMacVLanCreateWithVPortProfile* function. That function has access to the network device configuration and at the very least could ensure
[libvirt] RFC: Improve performance of macvtap device creation
For a guest domain defined with a large number of macvtap devices, it takes an exceedingly long time to boot the guest. In a test of a guest domain configured with 82 macvtap devices, it took over two minutes for the guest to boot. An strace of the ioctl calls during guest start up showed the SIOCGIFFLAGS ioctl literally being invoked 3,403 times. I was able to isolate the source of the ioctl calls to the*virNetDevMacVLanCreateWithVPortProfile* function in*virnetdevmacvlan.c*. The macvtap interface name is created by looping over a counter variable, starting with zero, and appending the counter value to 'macvtap'. With each iteration, a call is made to*virNetDevExists* (SIOCGIFFLAGS ioctl) to determine if a device with that name already exists, until a unique name is created. In the test case cited above, to create an interface name for the 82nd macvtap device, the*virNetDevExists* function will be called for interface names 'macvtap0' to 'macvtap80' before it is determined that 'mavtap81' can be used. So if N is the number of macvtap interfaces defined for a guest, the SIOCGIFFLAGS ioctl will be invoked (N x N + N)/2 times to find an unused macvtap device names. That's assuming only one guest is being started, who knows how many times the ioctl may have to be called in an installation running a large number of guests defined with macvtap devices. I was able to reduce the amount of time for starting a guest domain defined with 82 macvtap devices from over 2 minutes to about 14 seconds by keeping track of the interface name suffixes previously used. I defined two static bit maps (virBitmap), one each for macvtap and macvlan device name suffixes. When a macvtap/macvlan device is created, the index of the next clear bit (virBitmapNextClearBit) is retrieved to create the name. If an interface with that name does not exist, the device is created and the bit at the index used to create the interface name is set (virBitmapSetBit). When a macvtap/macvlan device is deleted, if the interface name has the pattern 'macvtap%d' or 'macvlan%d', the suffix is parsed into a bit index and used to clear the (virBitMapClearBit) bit in the respective bitmap. I am not sure that is the best design because there is no way to track interface names used to create macvtap devices outside of libvirt, for example using the ip command. There may also be other issues I've not contemplated. I included a couple of additional ideas below and am looking for comments or other suggestions that I have not considered. * Define a global counter variable initialized to 0, that gets incremented each time an interface name is created, to keep track of the last used interface name suffix. At some maximum value, the counter will be set back to 0. * Append a random number to 'macvlan' or 'macvtap' when creating the interface name. Of course, the number of digits would have to be limited so the interface name would not exceed the maximum allowed. * Create the interface name in code that has more knowledge of the environment and pass the name into the *virNetDevMacVLanCreateWithVPortProfile* function via the *tgifname* parameter. For example, the *qemuBuildCommandLine* function in *qemu_command.c* contains the loop that iterates over the network devices defined for the guest domain that ultimately get created via the *virNetDevMacVLanCreateWithVPortProfile* function. That function has access to the network device configuration and at the very least could ensure none of the names previously defined for the guest aren't used. I believe it would be matter of creating a macvtap interface name - e.g., maybe a call to some function in *virnetdevmacvlan.c* - and setting the name in the virDomainNetDef structure prior to invoking *qemuBuildInterfaceCommandLine*? There are shortcomings in all of these ideas, so if you have a better one, feel free to present it. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Again cgroups and isolcpus
Hi folks, i already started a discussion on the interaction of cgroups and isolcpus a while ago. But now i believe i have got a better understanding of how the two interact and i can describe problems that arise from that. The scenario: A machine that runs realtime tasks on pcpus reserved with isolcpus. It also runs VMs with the help of libvirt. It might also run realtime VMs with the help of libvirt. Moving a task into a new cgroup/cpuset and some modifications of the cpus in that set imply a setaffinity by the kernel. That affinity setting will ignore isolcpus. The result is possible "interference by" or "starvation of" these tasks. Now let me describe one scenario where that implicit setaffinity becomes a problem for our realtime system. libvirt creates a superset called the machine.slice and subsets called emulator and vpuX. By default the machine.slice inherits from the root which contains all pcpus, also the isolated ones. Now moving a task into that superset will place that task on isolcpus where it might interfere or simply starve. Turns out that a fresh qemu actually is put into that superset. That is a bug that should be fixed but let me address that one in another mail. My current point of view is that we need a strong mechanism to isolate cpus. isolcpus just is not good enough. The measure of choice probably is cpusets as well, and this time with the exclusive flag turned on. That will stop every other cpuset user from messing around with those cpus by accident. I am thinking of one or more cpusets where isolated cpus are parked and not used within this cgroup. Anyone wanting to use one of them will have to take it out there and explictily put it into their a new set. Now if libvirt makes the mistake to have tasks running in supersets these tasks will spread to the newly added rt-cpu. Or new tasks that run in supersets will end up on rt-cpus already in use. But at least we have containment in libvirt and the VMs it spawned. For alloc and free of rt-cpus i am planning to use libvirt hooks to begin with, from what i read they should enable me to do what i need. What do you guys think about the general idea to address the described problem? I will implement a prototype of the alloc-free of rt-cpus. My current hope is that libvirt hooks can be abused for that. I am thinking that at some point libvirt should be able to do that without hooks. It should get a notion of reserved ressources that are currently parked in other cgroups. My current suspicion is that the cpusets might just be the tip of the iceberg. -- for now i am running libvirt without cgroups to keep my isolcpus free cgroups/cpusets offer a switch to make a cpu exclusive to a set. That switch is great because it will act as an assert, a second line of defense. Having seen how cpusets and migration mess around with affinities i guess for realtime people have to insist on that second line of defense. Especially in times where cgroups are all over the place. In openstack one would actually say that a pcpu should be "dedicated". That will result in a vcpupin on exactly one pcpu. Unfortunately one meaning of "dedicated" gets lost in translation. It could otherwise be used by libvirtd to set cpuset.cpu_exclusive in the vcpu-cgroup. And i am bringing that up here because i do not think libvirt allows me to influence the cpu_exclusive flag for my vcpu cgroups. Henning -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [sandbox] Weird apparmor problems
Hi all, I'm seeing weird apparmor errors when running virt-sandbox here. Here are the log entries: apparmor="ALLOWED" operation="mknod" parent=1 profile="libvirt-634ed189-cca0-4126-830c-4e4a76846b25" name="/var/lib/libvirt/qemu/sandbox.monitor" pid=2251 comm="qemu-system-x86" requested_mask="c" denied_mask="c" fsuid=493 ouid=493 apparmor="ALLOWED" operation="open" parent=1 profile="libvirt-634ed189-cca0-4126-830c-4e4a76846b25" name="/dev/ptmx" pid=2251 comm="qemu-system-x86" requested_mask="w" denied_mask="w" fsuid=493 ouid=0 apparmor="ALLOWED" operation="open" parent=1 profile="libvirt-634ed189-cca0-4126-830c-4e4a76846b25" name="/dev/pts/2" pid=2251 comm="qemu-system-x86" requested_mask="w" denied_mask="w" fsuid=493 ouid=493 apparmor="ALLOWED" operation="file_perm" parent=1 profile="libvirt-634ed189-cca0-4126-830c-4e4a76846b25" name="/var/log/libvirt/qemu/sandbox.log" pid=2251 comm="qemu-system-x86" requested_mask="w" denied_mask="w" fsuid=493 ouid=0 apparmor="ALLOWED" operation="open" parent=1 profile="libvirt-634ed189-cca0-4126-830c-4e4a76846b25" name="/dev/ptmx" pid=2251 comm="qemu-system-x86" requested_mask="w" denied_mask="w" fsuid=493 ouid=0 apparmor="ALLOWED" operation="open" parent=1 profile="libvirt-634ed189-cca0-4126-830c-4e4a76846b25" name="/dev/pts/3" pid=2251 comm="qemu-system-x86" requested_mask="w" denied_mask="w" fsuid=493 ouid=493 apparmor="ALLOWED" operation="file_perm" parent=1 profile="libvirt-634ed189-cca0-4126-830c-4e4a76846b25" name="/var/log/libvirt/qemu/sandbox.log" pid=2251 comm="qemu-system-x86" requested_mask="w" denied_mask="w" fsuid=493 ouid=0 apparmor="ALLOWED" operation="open" parent=1 profile="libvirt-634ed189-cca0-4126-830c-4e4a76846b25" name="/dev/kvm" pid=2251 comm="qemu-system-x86" requested_mask="w" denied_mask="w" fsuid=493 ouid=0 The weird thing is that /dev/kvm, /var/log/libvirt/qemu/sandbox.log and /var/lib/libvirt/qemu/sandbox.monitor already have rules. And I'm wondering if it's normal to have write access to /dev/pts/* and /dev/ptmx. Any idea? -- Cedric -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v6 03/13] virstoragefile: Add virStorageSourceSetBackingStore
As explained for virStorageSourceGetBackingStore, quorum allows
multiple backing store, this make the operation to set bs complex
because we have to check if the backingStore is used as an array
or a pointer, and set it differently in both case.
In order to help the manipulation of backing store, I've added a
function virStorageSourceSetBackingStore.
For now virStorageSourceSetBackingStore don't handle the case where
we have more than one backing store in virStorageSource.
Signed-off-by: Matthias Gatto
Signed-off-by: John Ferlan
---
src/libvirt_private.syms | 1 +
src/util/virstoragefile.c | 10 ++
src/util/virstoragefile.h | 4
3 files changed, 15 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 8854b26..4aa923a 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2173,6 +2173,7 @@ virStorageSourceParseRBDColonString;
virStorageSourcePoolDefFree;
virStorageSourcePoolModeTypeFromString;
virStorageSourcePoolModeTypeToString;
+virStorageSourceSetBackingStore;
virStorageSourceUpdateBlockPhysicalSize;
virStorageTypeFromString;
virStorageTypeToString;
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index cb8e248..731e0c3 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1817,6 +1817,16 @@ virStorageSourceGetBackingStore(const virStorageSource
*src,
}
+bool
+virStorageSourceSetBackingStore(virStorageSourcePtr src,
+virStorageSourcePtr backingStore,
+size_t pos ATTRIBUTE_UNUSED)
+{
+src->backingStore = backingStore;
+return !!src->backingStore;
+}
+
+
/**
* virStorageSourcePtr:
*
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 8cd4854..5c5c29d 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -289,6 +289,10 @@ struct _virStorageSource {
# define DEV_BSIZE 512
# endif
+bool virStorageSourceSetBackingStore(virStorageSourcePtr src,
+ virStorageSourcePtr backingStore,
+ size_t pos);
+
virStorageSourcePtr virStorageSourceGetBackingStore(const virStorageSource
*src,
size_t pos);
--
2.6.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirt guest-agent-command error
On 28.10.2015 12:11, Vasiliy Tolstov wrote:
> Hi. I'm debug some issues with guest-agent inside vm and find this
> bug, that affected me
> https://bugzilla.redhat.com/show_bug.cgi?id=1090551
>
> Does anybody knows how to properly fix this ?
>
If qemu-ga fails to reply within desired timeout I guess there's not
much that libvirt can do.
Previously, when qemu did not expose whether there's somebody listening
inside the guest for guest agent commands, I've came up with ping
algorithm. Prior to each command a harmless ping is sent. If the agent
does not reply within 30 seconds, we consider it broken and don't even
try issuing the real command. So if the qemu-ga is stuck somewhere on
breakpoint, this mechanism works exactly as intended.
I can imagine it can be painful when debugging, therefore you can do
this locally to disable the ping:
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 5735ed8..b8b87aa 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -934,7 +934,7 @@ static int qemuAgentSend(qemuAgentPtr mon,
* Returns: 0 on success,
* -1 otherwise
*/
-static int
+static int ATTRIBUTE_UNUSED
qemuAgentGuestSync(qemuAgentPtr mon)
{
int ret = -1;
@@ -1121,9 +1121,6 @@ qemuAgentCommand(qemuAgentPtr mon,
return -1;
}
-if (qemuAgentGuestSync(mon) < 0)
-return -1;
-
memset(&msg, 0, sizeof(msg));
if (!(cmdstr = virJSONValueToString(cmd, false)))
Happy hacking!
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] qemu-agent-command via isa-serial for freebsd
On 28.10.2015 13:10, Vasiliy Tolstov wrote: > Hi! I'm need to control freebsd guest via qemu-ga, as i see qemu-ga > already supports isa-serial connection. > How i need to configure domain via libvirt to able virsh > qemu-agent-command to this guest vm? > You need to have a virtio channel whose target name is "org.qemu.guest_agent.0". The source does not matter to libvirt - we can connect both to an unix socket or pty. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] Qemu: add CMT support
On Thu, Oct 29, 2015 at 02:02:29PM +0800, Qiaowei Ren wrote:
> One RFC in
> https://www.redhat.com/archives/libvir-list/2015-June/msg01509.html
>
> CMT (Cache Monitoring Technology) can be used to measure the
> usage of cache by VM running on the host. This patch will
> extend the bulk stats API (virDomainListGetStats) to add this
> field. Applications based on libvirt can use this API to achieve
> cache usage of VM. Because CMT implementation in Linux kernel
> is based on perf mechanism, this patch will enable perf event
> for CMT when VM is created and disable it when VM is destroyed.
>
> Signed-off-by: Qiaowei Ren
Thanks for re-sending this patchset, it has reminded me of the
concerns / questions I had around this previously.
Just ignoring the code for a minute, IIUC the design is
- Open a file handle to the kernel perf system for each running VM
- Associate that perf event file handle with the QEMU VM PID
- Enable recording of the CMT perf event on that file handle
- Report the CMT event values in the virDomainGetStats() API
call when VIR_DOMAIN_STATS_CACHE is requested
My two primary concerns are
1. Do we want to have a perf event FD open for every running
VM all the time.
2. Is the virDomainGetStats() integration the right API approach
For item 1, my concern is that the CMT event is only ever going
to be consumed by OpenStack, and even then, only OpenStack installs
which have the schedular plugin that cares about the CMT event
data. It feels undesirable to have this perf system enabled for
all libvirt VMs, when perhaps < 1 % of libvirt users actually
want this data. It feels like we need some mechanism to decide
when this event is enabled
For item 2, my concern is first when virDomainGetStats is the
right API. I think it probably *is* the right API, since I can't
think of a better way.
Should we however, be having a very special case VIR_DOMAIN_STATS_CACHE
group, or should we have something more generic.
For example, if I run 'perf event' I see
List of pre-defined events (to be used in -e):
branch-instructions OR branches[Hardware event]
branch-misses [Hardware event]
bus-cycles [Hardware event]
cache-misses [Hardware event]
cache-references [Hardware event]
cpu-cycles OR cycles [Hardware event]
instructions [Hardware event]
ref-cycles [Hardware event]
stalled-cycles-frontend OR idle-cycles-frontend[Hardware event]
alignment-faults [Software event]
context-switches OR cs [Software event]
cpu-clock [Software event]
cpu-migrations OR migrations [Software event]
dummy [Software event]
emulation-faults [Software event]
major-faults [Software event]
minor-faults [Software event]
...any many many more...
Does it make sense to extend the virDomainStats API to *only* deal with
reporting of 1 specific perf event that you care about right now. It
feels like it might be better if we did something more general purpose.
eg what if something wants to get 'major-faults' data in future ?
So we add a VIR_DOMAIN_STATS_MAJOR_FAULT enum item, etc.
Combining these two concerns, I think we might need 2 things
- A new API to turn on/off collection of specific perf events
This could be something like
virDomainGetPerfEvents(virDOmainPtr dom,
virTypedParameter params);
This would fill virTypedParameters with one entry for each
perf event, using the VIR_TYPED_PARAM_BOOLEAN type. A 'true'
value would indicate that event is enabled for the VM. A
corresponding
virDomainSetPerfEvents(virDOmainPtr dom,
virTypedParameter params);
would enable you to toggle the flag, to enable/disable the
particular list of perf events you care about.
With that, we could have a 'VIR_DOMAIN_STATS_PERF_EVENT' enum
item for virDomainStats which causes reporting of all previously
enabled perf events
This would avoid us needing to have the perf event enabled for
all VMs all the time. Only applications using libvirt which
actually need the data would turn it on. It would also be now
scalable to all types of perf event, instead of just one specific
event
> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
> index 54e1e7b..31bce33 100644
> --- a/src/qemu/qemu_domain.h
> +++ b/src/qemu/qemu_domain.h
> @@ -196,6 +196,9 @@ struct _qemuDomainObjPrivate {
>
> bool hookRun; /* true if there was a hook run over this domain */
>
> +int cmt_fd; /* perf handl
Re: [libvirt] [PATCH v2 3/3] virsh.pod: update and improve a attach-interface section
On 10/29/2015 04:42 AM, Pavel Hrdina wrote: > On Tue, Oct 27, 2015 at 06:53:55PM -0400, John Ferlan wrote: >> >> >> On 10/27/2015 11:01 AM, Pavel Hrdina wrote: >>> Rewrite the attach-interface section in man page to be more readable and >>> add the new hostdev functionality. >>> >>> Signed-off-by: Pavel Hrdina >>> --- >>> tools/virsh.pod | 82 >>> +++-- >>> 1 file changed, 50 insertions(+), 32 deletions(-) >>> >> >> Why a separate patch? It's related to the previous one and if anyone >> ever (ahem) backed ported the other one, they could miss this one... No >> strong feeling either way - just curious. > > It's a rewrite of the attach-interface part of the man page, so I thought, > that > would be better to do it in a separate patch. Maybe I can at first do the > rewrite without adding anything about the new feature and than have the update > of man page together with previous patch. > Sure that works too - although I would think mostly unnecessary, but I know that's the norm to separate rewrite from feature/bug fix. >> >>> diff --git a/tools/virsh.pod b/tools/virsh.pod >>> index 0212e7a..843c293 100644 >>> --- a/tools/virsh.pod >>> +++ b/tools/virsh.pod >>> @@ -2507,51 +2507,69 @@ Likewise, I<--shareable> is an alias for I<--mode >>> shareable>. >>> [[[I<--live>] [I<--config>] | [I<--current>]] | [I<--persistent>]] >>> [I<--target target>] [I<--mac mac>] [I<--script script>] [I<--model model>] >>> [I<--inbound average,peak,burst,floor>] [I<--outbound average,peak,burst>] >>> -[I<--print-xml>] >>> - >>> -Attach a new network interface to the domain. I can be >>> -I to indicate connection via a libvirt virtual network, or >>> -I to indicate connection via a bridge device on the host, or >>> -I to indicate connection directly to one of the host's network >>> -interfaces or bridges. I indicates the source of the >>> -connection (the name of a network, or of a bridge device, or the >>> -host's network interfaces or bridges). I is used to specify >>> -the tap/macvtap device to be used to connect the domain to the >>> -source. Names starting with 'vnet' are considered as auto-generated >>> -and are blanked out/regenerated each time the interface is attached. >>> -I specifies the MAC address of the network interface; if a MAC >>> +[I<--managed>] [I<--print-xml>] >>> + >>> +Attach a new network interface to the domain. >>> + >>> +B<--type> can be one of the: I to indicate connection via a >>> libvirt >>> +virtual network, I to indicate connection via a bridge device >>> +on the host, I to indicate connection directly to one of the host's >>> +network interfaces or bridges, I to indicate connection using a >>> +passthrough of PCI device on the host. >> >> Using a ':' I think is unnecessary unless you somehow generate a real >> list such as via "=item * " entries. In that case you'd have the >> following: >> > > I've considered this format before writing the patch and it used a lot of > space. > I agree, that it would be better arranged. I'll update it. > I contend virsh.pod is a conglomeration of writing and formatting styles. I like your use of B<> instead of I<>, but that is "different". I think separating switches and better/longer descriptions are better, but that's not always the case. The whole file could use some amount of adjustment for consistency in format/style. >> +B<--type> can be one of the following: >> + >> +=over 4 >> + >> +=item * Use I to indicate connection via a libvirt virtual network >> + >> +=item * Use I to indicate connection via a bridge device on the >> host >> + >> + >> +=item * Use I to indicate connection directly to one of the host's >> +network interfaces or bridges >> + >> +=item * Use I to indicate connection using a passthrough of >> PCI device >> +on the host. >> + >> +=back >> >> NB: Whether the '--' is required on the type is perhaps a matter of >> opinion... Since the command line shown doesn't list --type shouldn't >> this still be an 'B'? > > Oh, You're right, there is no '--type' in the man page. In that case, it > should > be also referenced the same way. I've used it probably because you can write > something like this "attach-interface --domain test --type hostdev ...". > See this is one of those damned if you do and damned if you don't. One doesn't have to provide "--type" as long as the order is correct. However, providing --type can allow for a different argument order. I don't believe it's ever really described that ARGs can either be specified in the order for each COMMAND, but if not supplied in order, then the --ARG must be used. It's one of those things you just get used to while using virsh. >> >>> + >>> +B<--source> indicates the source of the connection. The source depends >>> +on the type of the interface: I name of the virtual network, >>> +I the name of the bridge device, I the name of the host's >>> +interface or bridge, I the PCI address of the host's interface >>>
[libvirt] [PATCH v6 11/11] quorum: Block snapshot operation with RAID
For now we block all snapshot operations with quorum, because it would require
a lot more code, espacially because Qemu doesn't really suport it.
I guess, we can use node-name, and manually snapshoot all qcow from a
virStorageSource and use this as a quorum's snapshot,
but libvirt doesn't support node-name, and we don't need node-name
anymore to use a quorum in qemu.
I actually have some patchs which could partially support quorum snapshot
on my computer, but nothing suitable to be upstream, so I prefer to have
a stable versions of quorum inside libvirt before dealling with
snapshot.
Signed-off-by: Matthias Gatto
---
src/qemu/qemu_driver.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 25be0d9..0e43966 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14674,6 +14674,12 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
goto cleanup;
+if (virDomainDefHasRAID(vm->def)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("Snapshot does not support domain with RAID(like
quorum) yet"));
+goto cleanup;
+}
+
if (qemuProcessAutoDestroyActive(driver, vm)) {
virReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("domain is marked for auto destroy"));
--
2.6.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v6 08/13] domain_conf: Read and Write quorum config
Add the capabiltty to libvirt to parse and format the quorum syntax
as described here:
http://www.redhat.com/archives/libvir-list/2014-May/msg00533.html
As explain Peter Krempa in the V5, we need a different index for each child to
manipulate individually each child of a quorum,
this tack is done in this patch.
Sadly this versions is a little buggy: if one day we allow a quorum child
to have a backing store, we would be unable to made a difference
between a quorum child and a backing store, worst than that,
if we have a quorum, with a quorum as a child, we have no way to
use the index for quorum child manipulation.
For now, this serie of patch forbid all actions which need
to use indexes with quorum.
Therefore even if the index manipulation is buggy, this should not be a problem
because the buggy code should never be call.
Signed-off-by: Matthias Gatto
---
src/conf/domain_conf.c | 178 ++---
1 file changed, 126 insertions(+), 52 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ce8edef..363ef5d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6607,20 +6607,56 @@ virDomainDiskSourceParse(xmlNodePtr node,
}
+static bool
+virDomainDiskThresholdParse(virStorageSourcePtr src,
+xmlNodePtr node)
+{
+char *threshold = virXMLPropString(node, "threshold");
+int ret;
+
+if (!threshold) {
+virReportError(VIR_ERR_XML_ERROR,
+ "%s", _("missing threshold in quorum"));
+return false;
+}
+ret = virStrToLong_ul(threshold, NULL, 10, &src->threshold);
+if (ret < 0 || src->threshold < 2) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unexpected threshold %s"),
+ "threshold must be a decimal number superior to 2 "
+ "and inferior to the number of children");
+VIR_FREE(threshold);
+return false;
+}
+VIR_FREE(threshold);
+return true;
+}
+
+
static int
virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
- virStorageSourcePtr src)
+ virStorageSourcePtr src,
+ xmlNodePtr node,
+ size_t pos)
{
virStorageSourcePtr backingStore = NULL;
xmlNodePtr save_ctxt = ctxt->node;
-xmlNodePtr source;
+xmlNodePtr source = NULL;
char *type = NULL;
char *format = NULL;
int ret = -1;
-if (!(ctxt->node = virXPathNode("./backingStore[*]", ctxt))) {
-ret = 0;
-goto cleanup;
+if (virStorageSourceIsRAID(src)) {
+if (!node) {
+ret = 0;
+goto cleanup;
+}
+ctxt->node = node;
+} else {
+if (!(ctxt->node = virXPathNode("./backingStore[*]", ctxt))) {
+ret = 0;
+goto cleanup;
+}
}
if (VIR_ALLOC(backingStore) < 0)
@@ -6652,17 +6688,36 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
goto cleanup;
}
-if (!(source = virXPathNode("./source", ctxt))) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
- _("missing disk backing store source"));
-goto cleanup;
-}
+if (virStorageSourceIsRAID(backingStore)) {
+xmlNodePtr cur = NULL;
-if (virDomainDiskSourceParse(source, ctxt, backingStore) < 0 ||
-virDomainDiskBackingStoreParse(ctxt, backingStore) < 0)
-goto cleanup;
+if (!virDomainDiskThresholdParse(backingStore, node))
+goto cleanup;
-if (!virStorageSourceSetBackingStore(src, backingStore, 0))
+for (cur = node->children; cur != NULL; cur = cur->next) {
+if (xmlStrEqual(cur->name, BAD_CAST "backingStore")) {
+if ((virDomainDiskBackingStoreParse(ctxt,
+backingStore,
+cur,
+
backingStore->nBackingStores) < 0)) {
+goto cleanup;
+}
+}
+}
+} else {
+
+if (!(source = virXPathNode("./source", ctxt))) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("missing disk backing store source"));
+goto cleanup;
+}
+
+if (virDomainDiskSourceParse(source, ctxt, backingStore) < 0 ||
+virDomainDiskBackingStoreParse(ctxt, backingStore, NULL, 0) < 0)
+goto cleanup;
+}
+
+if (!virStorageSourceSetBackingStore(src, backingStore, pos))
goto cleanup;
ret = 0;
@@ -7177,6 +7232,10 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
}
} else if (xmlStrEqual(cur->name, BAD_CAST "boot")) {
/* boot is parsed as part of virDomainDeviceInfoParseXML */
+} else if (xmlStrEqual
[libvirt] [PATCH v6 11/13] qemu: Block snapshot operation with RAID
For now we block all snapshot operations with quorum,
because it would require a lot more code,
especially because Qemu doesn't really suport it.
I guess, we can use node-name, and manually snapshot all qcow
from a virStorageSource and use this as a quorum's snapshot,
but libvirt doesn't support node-name, and we don't need
node-name anymore to use a quorum in qemu.
I have some patchs which could partially support quorum snapshot
on my computer, but nothing suitable to be upstream, so I prefer to have
a stable versions of quorum inside libvirt before dealing with
snapshot.
Signed-off-by: Matthias Gatto
---
src/qemu/qemu_driver.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 193c25d..1ec0cf2 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14674,6 +14674,12 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
goto cleanup;
+if (virDomainDefHasRAID(vm->def)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("Snapshot does not support domain with RAID(like
quorum) yet"));
+goto cleanup;
+}
+
if (qemuProcessAutoDestroyActive(driver, vm)) {
virReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("domain is marked for auto destroy"));
--
2.6.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v6 12/13] qemu: qemuDomainGetBlockInfo quorum support
Signed-off-by: Matthias Gatto
---
src/qemu/qemu_driver.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1ec0cf2..f70f1dd 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11826,7 +11826,8 @@ qemuDomainGetBlockInfo(virDomainPtr dom,
goto endjob;
}
-if (virStorageSourceIsEmpty(disk->src)) {
+if (!virStorageSourceIsRAID(disk->src) &&
+virStorageSourceIsEmpty(disk->src)) {
virReportError(VIR_ERR_INVALID_ARG,
_("disk '%s' does not currently have a source
assigned"),
path);
--
2.6.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v6 13/13] qemu: qemuDiskPathToAlias quorum support
By adding quorum support to qemuDiskPathToAlias, we're adding support to
qemuDomainGetBlkioParameters, which was returning an error when the domain
was active.
Signed-off-by: Matthias Gatto
---
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 f70f1dd..50d90b5 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16115,7 +16115,7 @@ qemuDiskPathToAlias(virDomainObjPtr vm, const char
*path, int *idxret)
if (idxret)
*idxret = idx;
-if (virDomainDiskGetSource(disk)) {
+if (virDomainDiskGetSource(disk) || virStorageSourceIsRAID(disk->src)) {
if (virAsprintf(&ret, "drive-%s", disk->info.alias) < 0)
return NULL;
}
--
2.6.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v6 07/13] virstoragefile: Add quorum in virstoragefile
Add VIR_STORAGE_TYPE_QUORUM in virStorageType. Add VIR_STORAGE_FILE_QUORUM in virStorageFileFormat. Add threshold value in _virStorageSource Signed-off-by: Matthias Gatto --- docs/formatdomain.html.in | 23 --- docs/schemas/domaincommon.rng | 21 - docs/schemas/storagecommon.rng | 1 + docs/schemas/storagevol.rng| 1 + src/conf/domain_conf.c | 2 ++ src/qemu/qemu_command.c| 1 + src/qemu/qemu_driver.c | 4 src/qemu/qemu_migration.c | 1 + src/util/virstoragefile.c | 25 + src/util/virstoragefile.h | 3 +++ 10 files changed, 70 insertions(+), 12 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index c88b032..a52b60b 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1958,8 +1958,9 @@ Valid values are "file", "block", "dir" (since 0.7.5), -"network" (since 0.8.7), or -"volume" (since 1.0.5) +"network" (since 0.8.7), +"volume" (since 1.0.5), or +"quorum" (since 1.2.21) and refer to the underlying source for the disk. device attribute @@ -2025,6 +2026,14 @@ snapshot='yes' with a transient disk generally does not make sense. + threshold attribute + since 1.2.21 + +Only use with a quorum disk. +Indicate the minimum of positive vote a quorum must have to validate +a data to be write. The minimum value is "2". The number of backingStores +contain by the quorum must be superior to the threshold. + source @@ -2102,6 +2111,11 @@ +type='quorum' +since 1.2.21 + + A quorum contain no source element, but a serie of backingStores instead. + With "file", "block", and "volume", one or more optional sub-elements seclabel, described @@ -2241,7 +2255,10 @@ property, but using existing external files for snapshot or block copy operations requires the end user to pre-create the file correctly). The following attributes and sub-elements are -supported in backingStore: +supported in. +Since 1.2.21. This elements is used to +describe a quorum child. +backingStore: type attribute diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index f196177..067140b 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1240,9 +1240,15 @@ + + + + + + - + @@ -1284,9 +1290,22 @@ + + + + +quorum + + + + + + + + diff --git a/docs/schemas/storagecommon.rng b/docs/schemas/storagecommon.rng index 7c04462..0ebc2ef 100644 --- a/docs/schemas/storagecommon.rng +++ b/docs/schemas/storagecommon.rng @@ -76,6 +76,7 @@ fat vhd ploop + quorum diff --git a/docs/schemas/storagevol.rng b/docs/schemas/storagevol.rng index 7450547..a718576 100644 --- a/docs/schemas/storagevol.rng +++ b/docs/schemas/storagevol.rng @@ -20,6 +20,7 @@ dir network netdir +quorum diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 101cfeb..ce8edef 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6583,6 +6583,7 @@ virDomainDiskSourceParse(xmlNodePtr node, if (virDomainDiskSourcePoolDefParse(node, &src->srcpool) < 0) goto cleanup; break; +case VIR_STORAGE_TYPE_QUORUM: case VIR_STORAGE_TYPE_NONE: case VIR_STORAGE_TYPE_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, @@ -18837,6 +18838,7 @@ virDomainDiskSourceFormatInternal(virBufferPtr buf, skipSeclabels); break; +case VIR_STORAGE_TYPE_QUORUM: case VIR_STORAGE_TYPE_NONE: case VIR_STORAGE_TYPE_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 8824541..50cf8cc 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3534,6 +3534,7 @@ qemuGetDriveSourceString(virStorageSourcePtr src, goto cleanup; break; +case VIR_STORAGE_TYPE_QUORUM: case VIR_STORAGE_TYPE_VOLUME: case VIR_STORAGE_TYPE_NONE: case VIR_STORAGE_TYPE_LAST: diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index cbc508c..193c25d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13782,6 +13782,7 @@ qemu
[libvirt] [PATCH v6 02/13] virstoragefile: Always use virStorageSourceGetBackingStore to get backing store
Uniformize backing store usage by calling virStorageSourceGetBackingStore
instead of setting backing store manually.
Signed-off-by: Matthias Gatto
Signed-off-by: John Ferlan
---
src/conf/domain_conf.c| 7 ---
src/conf/storage_conf.c | 6 +++---
src/qemu/qemu_cgroup.c| 4 ++--
src/qemu/qemu_domain.c| 2 +-
src/qemu/qemu_driver.c| 18 ++
src/qemu/qemu_monitor_json.c | 4 +++-
src/security/security_dac.c | 2 +-
src/security/security_selinux.c | 4 ++--
src/security/virt-aa-helper.c | 2 +-
src/storage/storage_backend.c | 20
src/storage/storage_backend_fs.c | 31 +--
src/storage/storage_backend_gluster.c | 8 +---
src/storage/storage_backend_logical.c | 10 ++
src/util/virstoragefile.c | 20 ++--
tests/virstoragetest.c| 14 +++---
15 files changed, 84 insertions(+), 68 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9a0c7fc..e0befc3 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -18897,7 +18897,7 @@ virDomainDiskBackingStoreFormat(virBufferPtr buf,
/* We currently don't output seclabels for backing chain element */
if (virDomainDiskSourceFormatInternal(buf, backingStore, 0, 0, true) < 0 ||
virDomainDiskBackingStoreFormat(buf,
-backingStore->backingStore,
+
virStorageSourceGetBackingStore(backingStore, 0),
backingStore->backingStoreRaw,
idx + 1) < 0)
return -1;
@@ -19018,7 +19018,8 @@ virDomainDiskDefFormat(virBufferPtr buf,
/* Don't format backingStore to inactive XMLs until the code for
* persistent storage of backing chains is ready. */
if (!(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE) &&
-virDomainDiskBackingStoreFormat(buf, def->src->backingStore,
+virDomainDiskBackingStoreFormat(buf,
+
virStorageSourceGetBackingStore(def->src, 0),
def->src->backingStoreRaw, 1) < 0)
return -1;
@@ -23317,7 +23318,7 @@ virDomainDiskDefForeachPath(virDomainDiskDefPtr disk,
}
}
-for (tmp = disk->src; tmp; tmp = tmp->backingStore) {
+for (tmp = disk->src; tmp; tmp = virStorageSourceGetBackingStore(tmp, 0)) {
int actualType = virStorageSourceGetActualType(tmp);
/* execute the callback only for local storage */
if (actualType != VIR_STORAGE_TYPE_NETWORK &&
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 9b8abea..d048e39 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -1330,7 +1330,7 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
if (virStorageSize(unit, capacity, &ret->target.capacity) < 0)
goto error;
} else if (!(flags & VIR_VOL_XML_PARSE_NO_CAPACITY) &&
- !((flags & VIR_VOL_XML_PARSE_OPT_CAPACITY) &&
ret->target.backingStore)) {
+ !((flags & VIR_VOL_XML_PARSE_OPT_CAPACITY) &&
virStorageSourceGetBackingStore(&ret->target, 0))) {
virReportError(VIR_ERR_XML_ERROR, "%s", _("missing capacity element"));
goto error;
}
@@ -1644,9 +1644,9 @@ virStorageVolDefFormat(virStoragePoolDefPtr pool,
&def->target, "target") < 0)
goto cleanup;
-if (def->target.backingStore &&
+if (virStorageSourceGetBackingStore(&def->target, 0) &&
virStorageVolTargetDefFormat(options, &buf,
- def->target.backingStore,
+
virStorageSourceGetBackingStore(&def->target, 0),
"backingStore") < 0)
goto cleanup;
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index a8e0b8c..f93782b 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -121,7 +121,7 @@ qemuSetupDiskCgroup(virDomainObjPtr vm,
virStorageSourcePtr next;
bool forceReadonly = false;
-for (next = disk->src; next; next = next->backingStore) {
+for (next = disk->src; next; next = virStorageSourceGetBackingStore(next,
0)) {
if (qemuSetImageCgroupInternal(vm, next, false, forceReadonly) < 0)
return -1;
@@ -139,7 +139,7 @@ qemuTeardownDiskCgroup(virDomainObjPtr vm,
{
virStorageSourcePtr next;
-for (next = disk->src; next; next = next->backingStore) {
+for (next = disk->src; next; next = virStorageSourceGetBackingStore(next,
0)) {
if (qemuSetImageCgroup(vm, next, true) < 0)
return -1;
}
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 890d8ed..1298e44 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/q
[libvirt] [PATCH v6 00/13] qemu: Add quorum support to libvirt
The purpose of these patches is to introduce quorum for libvirt I've try to follow this proposal: http://www.redhat.com/archives/libvir-list/2014-May/msg00533.html This feature ask for 6 task: 1) Allow a _virStorageSource to contain more than one backing store. Because all the actual libvirt code use the backingStore field as a pointer and we needs want to change that, I've decide to encapsulate the backingStore field to simplifie the array manipulation. 2) Add the missing field a quorum need in _virStorageSource and the VIR_STORAGE_TYPE_QUORUM and VIR_STORAGE_FILE_QUORUM in their respectives enums. 3) Parse and format the xml Because a quorum allows to have more than one backing store at the same level we need to change virDomainDiskDefFormat and virDomainDiskDefParseXML to call virDomainDiskBackingStoreFormat and virDomainDiskBackingStoreParse in a loop. virDomainDiskBackingStoreFormat and virDomainDiskBackingStoreParse can call themself recursively in a loop because a quorum can contain another quorum 4) Build qemu string As for the xml, we have to call the function which create quorum recursively. But this task have the problem explained here: http://www.redhat.com/archives/libvir-list/2014-October/msg00529.html The _virStorageSource missing some informations that can be passed to a child, and therefore this version of quorum is incomplet. 5) Allow to hotplug/change a disk in a quorum This part is not present in these patches because for this task we have to use blockdev-add, and currently libvirt use device_add for hotpluging that doesn't allow to hotplug quorum childs. There is 3 way to handle this problem: 1) create a virDomainBlockDevAdd function in libvirt witch call blockdev-add. 2) use blockdev-add instead of device_add in qemuMonitorJSONAddDevice 3) write a hack which uses blockdev-add only when attaching a quorum child V2: -Rebase on master -Add Documentation V3: -Transforme the backingStore field in virStorageSource into an array of pointer instead of a pointer -Modify virStorageSourceSetBackingStore to allow it to expand the backingStore size. V4: -Rebase on master V5: -Rebase on master -patch 1-4/9: use patchs from John Ferlan -patch 4/9: check return of virStorageSourceSetBackingStore -patch 5/9: report type of error on virStorageSourceSetBackingStore v6 note: First at all, I'm sorry for the time between v5 and v6, I had other projects to work on and was unable to work at full time on libvirt, moreover I've try at first to support all snapshot and block jobs operations for quorum, but encounter a lot a problems. At the end I had a versions which was only handling some few operations, so I've block all unsupported operations for quorum, I plan to continue working on the quorum unsupported operations, and send it when it will be ready, but in the meantime I hope to upstream this serie of patch which should provide a very basic(but stable) quorum suport. v6 modifications: -Rebase on master -Remove node-name patch -patch 06/13: Add virStorageSourceIsRAID -patch 10/13: Add virDomainDefHasRAID -patch 11-13/13: Block all unsupported operations Matthias Gatto (13): virstoragefile: Add virStorageSourceGetBackingStore virstoragefile: Always use virStorageSourceGetBackingStore to get backing store virstoragefile: Add virStorageSourceSetBackingStore virstoragefile: Always use virStorageSourceSetBackingStore to set backing store virstoragefile: change backingStore to backingStores. virstoragefile: Add virStorageSourceIsRAID virstoragefile: Add quorum in virstoragefile domain_conf: Read and Write quorum config qemu: Add quorum support in qemuBuildDriveDevStr domain: add virDomainDefHasRAID qemu: Block snapshot operation with RAID qemu: qemuDomainGetBlockInfo quorum support qemu: qemuDiskPathToAlias quorum support docs/formatdomain.html.in | 23 +++- docs/schemas/domaincommon.rng | 21 +++- docs/schemas/storagecommon.rng| 1 + docs/schemas/storagevol.rng | 1 + src/conf/domain_conf.c| 196 +- src/conf/domain_conf.h| 1 + src/conf/storage_conf.c | 23 ++-- src/libvirt_private.syms | 4 + src/qemu/qemu_cgroup.c| 4 +- src/qemu/qemu_command.c | 94 src/qemu/qemu_domain.c| 2 +- src/qemu/qemu_driver.c| 50 ++--- src/qemu/qemu_migration.c | 1 + src/qemu/qemu_monitor_json.c | 4 +- src/security/security_dac.c | 2 +- src/security/security_selinux.c | 4 +- src/security/virt-aa-helper.c | 2 +- src/storage/storage_backend.c | 22 ++--
[libvirt] [PATCH v6 10/13] domain: Add virDomainDefHasRAID
This function check if a domain has a RAID as a disk.
This function is useful to block snapshot operation on domain
which contain quorum.
Signed-off-by: Matthias Gatto
---
src/conf/domain_conf.c | 13 +
src/conf/domain_conf.h | 1 +
src/libvirt_private.syms | 1 +
3 files changed, 15 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 363ef5d..9b947a6 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2711,6 +2711,19 @@ virDomainDefNewFull(const char *name,
}
+bool
+virDomainDefHasRAID(virDomainDefPtr def)
+{
+size_t i;
+
+for (i = 0; i < def->ndisks; ++i) {
+if (virStorageSourceIsRAID(def->disks[i]->src))
+return true;
+}
+return true;
+}
+
+
void virDomainObjAssignDef(virDomainObjPtr domain,
virDomainDefPtr def,
bool live,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index fd4ef82..bf768d8 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2556,6 +2556,7 @@ void virDomainDefClearPCIAddresses(virDomainDefPtr def);
void virDomainDefClearCCWAddresses(virDomainDefPtr def);
void virDomainDefClearDeviceAliases(virDomainDefPtr def);
void virDomainTPMDefFree(virDomainTPMDefPtr def);
+bool virDomainDefHasRAID(virDomainDefPtr def);
typedef int (*virDomainDeviceInfoCallback)(virDomainDefPtr def,
virDomainDeviceDefPtr dev,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index efcea49..1f5b106 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -218,6 +218,7 @@ virDomainDefGetMemoryInitial;
virDomainDefGetSecurityLabelDef;
virDomainDefHasDeviceAddress;
virDomainDefHasMemoryHotplug;
+virDomainDefHasRAID;
virDomainDefMaybeAddController;
virDomainDefMaybeAddInput;
virDomainDefNeedsPlacementAdvice;
--
2.6.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v6 04/13] virstoragefile: Always use virStorageSourceSetBackingStore to set backing store
Replace the parts of the code where a backing store is set manually
with virStorageSourceSetBackingStore
Signed-off-by: Matthias Gatto
Signed-off-by: John Ferlan
---
src/conf/domain_conf.c| 3 ++-
src/conf/storage_conf.c | 17 ++---
src/qemu/qemu_driver.c| 17 +++--
src/storage/storage_backend_fs.c | 7 +--
src/storage/storage_backend_gluster.c | 5 +++--
src/storage/storage_backend_logical.c | 9 ++---
src/storage/storage_driver.c | 3 ++-
src/util/virstoragefile.c | 8 +---
tests/virstoragetest.c| 4 ++--
9 files changed, 46 insertions(+), 27 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index e0befc3..101cfeb 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6661,7 +6661,8 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
virDomainDiskBackingStoreParse(ctxt, backingStore) < 0)
goto cleanup;
-src->backingStore = backingStore;
+if (!virStorageSourceSetBackingStore(src, backingStore, 0))
+goto cleanup;
ret = 0;
cleanup:
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index d048e39..183c80c 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -1259,6 +1259,7 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
char *capacity = NULL;
char *unit = NULL;
char *backingStore = NULL;
+virStorageSourcePtr backingStorePtr;
xmlNodePtr node;
xmlNodePtr *nodes = NULL;
size_t i;
@@ -1295,20 +1296,22 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
}
if ((backingStore = virXPathString("string(./backingStore/path)", ctxt))) {
-if (VIR_ALLOC(ret->target.backingStore) < 0)
+if (VIR_ALLOC(backingStorePtr) < 0)
goto error;
-ret->target.backingStore->path = backingStore;
+if (!virStorageSourceSetBackingStore(&ret->target, backingStorePtr, 0))
+goto error;
+backingStorePtr->path = backingStore;
backingStore = NULL;
if (options->formatFromString) {
char *format =
virXPathString("string(./backingStore/format/@type)", ctxt);
if (format == NULL)
-ret->target.backingStore->format = options->defaultFormat;
+backingStorePtr->format = options->defaultFormat;
else
-ret->target.backingStore->format =
(options->formatFromString)(format);
+backingStorePtr->format = (options->formatFromString)(format);
-if (ret->target.backingStore->format < 0) {
+if (backingStorePtr->format < 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("unknown volume format type %s"), format);
VIR_FREE(format);
@@ -1317,9 +1320,9 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
VIR_FREE(format);
}
-if (VIR_ALLOC(ret->target.backingStore->perms) < 0)
+if (VIR_ALLOC(backingStorePtr->perms) < 0)
goto error;
-if (virStorageDefParsePerms(ctxt, ret->target.backingStore->perms,
+if (virStorageDefParsePerms(ctxt, backingStorePtr->perms,
"./backingStore/permissions") < 0)
goto error;
}
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ac45d56..cbc508c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14253,13 +14253,18 @@
qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
/* Update vm in place to match changes. */
need_unlink = false;
-newDiskSrc->backingStore = disk->src;
-disk->src = newDiskSrc;
+if (!virStorageSourceSetBackingStore(newDiskSrc, disk->src, 0)) {
+ret = -1;
+goto cleanup;
+}
newDiskSrc = NULL;
if (persistDisk) {
-persistDiskSrc->backingStore = persistDisk->src;
-persistDisk->src = persistDiskSrc;
+if (!virStorageSourceSetBackingStore(persistDiskSrc,
+ persistDisk->src, 0)) {
+ret = -1;
+goto cleanup;
+}
persistDiskSrc = NULL;
}
@@ -14302,13 +14307,13 @@
qemuDomainSnapshotUndoSingleDiskActive(virQEMUDriverPtr driver,
/* Update vm in place to match changes. */
tmp = disk->src;
disk->src = virStorageSourceGetBackingStore(tmp, 0);
-tmp->backingStore = NULL;
+ignore_value(virStorageSourceSetBackingStore(tmp, NULL, 0));
virStorageSourceFree(tmp);
if (persistDisk) {
tmp = persistDisk->src;
persistDisk->src = virStorageSourceGetBackingStore(tmp, 0);
-tmp->backingStore = NULL;
+ignore_value(virStorageSourceSetBackingStore(tmp, NULL, 0));
virStorageSourceFree(tmp);
}
}
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_back
Re: [libvirt] [PATCH v2 2/3] virsh-domain: update attach-interface to support type=hostdev
On 10/29/2015 03:08 AM, Pavel Hrdina wrote: > On Tue, Oct 27, 2015 at 06:16:33PM -0400, John Ferlan wrote: >> [...] >> >> What happens if someone provides --managed with some other 'typ'? >> >> IOW: If it's only being supported by , then after the switch, a >> check should probably occur for "if (managed && typ != >> VIR_DOMAIN_NET_TYPE_HOSTDEV), message, and fail. > > The check was there, but then I removed it because other arguments doesn't > check > the usability too. We don't need to error out, because libvirt just ignore > the "managed='yes'" in the XML. > >> >> I'm not fully clear after reading the bz and the previous review whether >> this patch resolves the bz - something to be worked out in the bz for >> history's sake though > > I think, that the main issue with the BZ is that there was no easy way how to > attach *hostdev* interface without manually creating the XML for that > interface. > We established with Laine, that there is not need for translating netdev name > to > PCI address. > >> >> I think with the adjustment to check whether managed is supplied for non >> hostdev, then you have an ACK for what's changed here. > > Reconsider the 'managed' check, we can be strict and check whether it's used > only with hosted type or not, but it's not necessary. > As I read the docs/code, I see managed is only parsed for types, so yes from that aspect you're correct. I usually err on the side of the extra check so that if some day the parsing code gets changed you don't run into issues. The formatting code certainly only writes out managed='yes' if type is hostdev, so we're safe from the issue of managed='yes' being written into the guest XML... I guess it's the longer way of say ACK for what's here unless you want to be extra paranoid. John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v6 11/11] quorum: Block snapshot operation with RAID
On Thu, Oct 29, 2015 at 2:43 PM, Matthias Gatto
wrote:
> For now we block all snapshot operations with quorum, because it would require
> a lot more code, espacially because Qemu doesn't really suport it.
>
> I guess, we can use node-name, and manually snapshoot all qcow from a
> virStorageSource and use this as a quorum's snapshot,
> but libvirt doesn't support node-name, and we don't need node-name
> anymore to use a quorum in qemu.
>
> I actually have some patchs which could partially support quorum snapshot
> on my computer, but nothing suitable to be upstream, so I prefer to have
> a stable versions of quorum inside libvirt before dealling with
> snapshot.
>
> Signed-off-by: Matthias Gatto
> ---
> src/qemu/qemu_driver.c | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 25be0d9..0e43966 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -14674,6 +14674,12 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
> if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
> goto cleanup;
>
> +if (virDomainDefHasRAID(vm->def)) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("Snapshot does not support domain with RAID(like
> quorum) yet"));
> +goto cleanup;
> +}
> +
> if (qemuProcessAutoDestroyActive(driver, vm)) {
> virReportError(VIR_ERR_OPERATION_INVALID,
> "%s", _("domain is marked for auto destroy"));
> --
> 2.6.1
>
Oups, I've send this patch twice, sorry for my mistake.
This one is not the good one, the good one is this one:
http://www.redhat.com/archives/libvir-list/2015-October/msg00863.html
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v6 05/13] virstoragefile: change backingStore to backingStores.
The backingStore field was a virStorageSourcePtr.
Because a quorum can contain more that one backingStore at the same level,
it's now an array of 'virStorageSourcePtr'.
This patch rename src->backingStore to src->backingStores,
Made the necessary changes to virStorageSourceSetBackingStore
and virStorageSourceGetBackingStore.
virStorageSourceSetBackingStore can now expand the size of src->backingStores.
Signed-off-by: Matthias Gatto
---
src/storage/storage_backend.c| 2 +-
src/storage/storage_backend_fs.c | 2 +-
src/util/virstoragefile.c| 48 +---
src/util/virstoragefile.h| 3 ++-
4 files changed, 44 insertions(+), 11 deletions(-)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index 3b504e9..f6ed91a 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -497,7 +497,7 @@ virStorageBackendCreateRaw(virConnectPtr conn
ATTRIBUTE_UNUSED,
goto cleanup;
}
-if (vol->target.backingStore) {
+if (vol->target.backingStores) {
virReportError(VIR_ERR_NO_SUPPORT, "%s",
_("backing storage not supported for raw volumes"));
goto cleanup;
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index d065a5f..ef86ecd 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -1097,7 +1097,7 @@ static int createFileDir(virConnectPtr conn
ATTRIBUTE_UNUSED,
return -1;
}
-if (vol->target.backingStore) {
+if (vol->target.backingStores) {
virReportError(VIR_ERR_NO_SUPPORT, "%s",
_("backing storage not supported for directories
volumes"));
return -1;
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index cb96c5b..44bce91 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1809,21 +1809,48 @@ virStorageSourcePoolDefCopy(const
virStorageSourcePoolDef *src)
}
+/**
+ * virStorageSourceGetBackingStore:
+ * @src: virStorageSourcePtr containing the backing stores
+ * @pos: position of the backing store to get
+ *
+ * return the backingStore at the position of @pos
+ */
virStorageSourcePtr
-virStorageSourceGetBackingStore(const virStorageSource *src,
-size_t pos ATTRIBUTE_UNUSED)
+virStorageSourceGetBackingStore(const virStorageSource *src, size_t pos)
{
-return src->backingStore;
+if (!src || !src->backingStores || pos >= src->nBackingStores)
+return NULL;
+return src->backingStores[pos];
}
+/**
+ * virStorageSourceSetBackingStore:
+ * @src: virStorageSourcePtr to hold @backingStore
+ * @backingStore: backingStore to store
+ * @pos: position of the backing store to store
+ *
+ * Set @backingStore at @pos in src->backingStores.
+ * If src->backingStores don't have the space to contain
+ * @backingStore, we expand src->backingStores
+ */
bool
virStorageSourceSetBackingStore(virStorageSourcePtr src,
virStorageSourcePtr backingStore,
-size_t pos ATTRIBUTE_UNUSED)
+size_t pos)
{
-src->backingStore = backingStore;
-return !!src->backingStore;
+if (!src)
+return false;
+
+if (pos >= src->nBackingStores) {
+int nbr = pos - src->nBackingStores + 1;
+if (VIR_EXPAND_N(src->backingStores, src->nBackingStores, nbr) < 0)
+return false;
+}
+
+src->backingStores[pos] = backingStore;
+return true;
}
@@ -2038,6 +2065,8 @@ virStorageSourceIsEmpty(virStorageSourcePtr src)
void
virStorageSourceBackingStoreClear(virStorageSourcePtr def)
{
+size_t i;
+
if (!def)
return;
@@ -2045,8 +2074,11 @@ virStorageSourceBackingStoreClear(virStorageSourcePtr
def)
VIR_FREE(def->backingStoreRaw);
/* recursively free backing chain */
-virStorageSourceFree(virStorageSourceGetBackingStore(def, 0));
-virStorageSourceSetBackingStore(def, NULL, 0);
+for (i = 0; i < def->nBackingStores; ++i)
+virStorageSourceFree(virStorageSourceGetBackingStore(def, i));
+if (def->nBackingStores > 0)
+VIR_FREE(def->backingStores);
+def->nBackingStores = 0;
}
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 5c5c29d..5f76b4b 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -270,7 +270,8 @@ struct _virStorageSource {
bool shared;
/* backing chain of the storage source */
-virStorageSourcePtr backingStore;
+virStorageSourcePtr *backingStores;
+size_t nBackingStores;
/* metadata for storage driver access to remote and local volumes */
virStorageDriverDataPtr drv;
--
2.6.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v6 09/13] qemu: Add quorum support in qemuBuildDriveDevStr
Allow libvirt to build the quorum string used by quemu.
Add 2 static functions: qemuBuildRAIDStr and
qemuBuildRAIDFileSourceStr.
qemuBuildRAIDStr is made because a quorum can have another quorum
as a child, so we may need to call qemuBuildRAIDStr recursively.
qemuBuildRAIDFileSourceStr was basically made to share
the code use to build the source between qemuBuildRAIDStr and
qemuBuildDriveStr, but there is some difference betwin the syntax
use by libvirt to declare a disk and the one qemu need to build a quorum:
a quorum need a syntaxe like:
"domaine_name.children.X.file.filename=filename"
where libvirt don't use "file.filename=" but directly "file=".
Therfore I use this function only for quorum.
Signed-off-by: Matthias Gatto
---
src/qemu/qemu_command.c | 93 +
1 file changed, 93 insertions(+)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 50cf8cc..4ca0011 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3612,6 +3612,93 @@ qemuCheckDiskConfig(virDomainDiskDefPtr disk)
return -1;
}
+static bool
+qemuBuildRAIDFileSourceStr(virConnectPtr conn,
+ virStorageSourcePtr src,
+ virBuffer *opt,
+ const char *toAppend)
+{
+char *source = NULL;
+int actualType = virStorageSourceGetActualType(src);
+
+if (qemuGetDriveSourceString(src, conn, &source) < 0)
+return false;
+
+if (source) {
+virBufferStrcat(opt, ",", toAppend, "filename=", NULL);
+
+if (actualType == VIR_STORAGE_TYPE_DIR) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unsupported disk driver type for '%s'"),
+ virStorageFileFormatTypeToString(src->format));
+return false;
+}
+virBufferAdd(opt, source, -1);
+}
+
+return true;
+}
+
+
+static bool
+qemuBuildRAIDStr(virConnectPtr conn,
+ virDomainDiskDefPtr disk,
+ virStorageSourcePtr src,
+ virBuffer *opt,
+ const char *toAppend)
+{
+char *tmp = NULL;
+int ret;
+virStorageSourcePtr backingStore;
+size_t i;
+
+if (virStorageSourceGetActualType(src) == VIR_STORAGE_TYPE_QUORUM) {
+if (!src->threshold) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("threshold missing in the quorum configuration"));
+return false;
+}
+if (src->nBackingStores < 2) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("a quorum must have at last 2 children"));
+return false;
+}
+if (src->threshold > src->nBackingStores) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("threshold must not exceed the number of
children"));
+return false;
+}
+virBufferAsprintf(opt, ",%svote-threshold=%lu",
+ toAppend, src->threshold);
+}
+
+for (i = 0; i < src->nBackingStores; ++i) {
+backingStore = virStorageSourceGetBackingStore(src, i);
+ret = virAsprintf(&tmp, "%schildren.%lu.file.", toAppend, i);
+if (ret < 0)
+return false;
+
+virBufferAsprintf(opt, ",%schildren.%lu.driver=%s",
+ toAppend, i,
+
virStorageFileFormatTypeToString(backingStore->format));
+
+if (qemuBuildRAIDFileSourceStr(conn, backingStore, opt, tmp) == false)
+goto error;
+
+/* This operation avoid us to made another copy */
+tmp[ret - sizeof("file")] = '\0';
+if (virStorageSourceIsRAID(backingStore)) {
+if (!qemuBuildRAIDStr(conn, disk, backingStore, opt, tmp))
+goto error;
+}
+VIR_FREE(tmp);
+}
+return true;
+ error:
+VIR_FREE(tmp);
+return false;
+}
+
/* Check whether the device address is using either 'ccw' or default s390
* address format and whether that's "legal" for the current qemu and/or
@@ -3781,6 +3868,7 @@ qemuBuildDriveStr(virConnectPtr conn,
goto error;
if (source &&
+!virStorageSourceIsRAID(disk->src) &&
!((disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY ||
disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) &&
disk->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN)) {
@@ -4132,6 +4220,11 @@ qemuBuildDriveStr(virConnectPtr conn,
disk->blkdeviotune.size_iops_sec);
}
+if (virStorageSourceIsRAID(disk->src)) {
+if (!qemuBuildRAIDStr(conn, disk, disk->src, &opt, ""))
+goto error;
+}
+
if (virBufferCheckError(&opt) < 0)
goto error;
--
2.6.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v6 06/13] virstoragefile: Add virStorageSourceIsRAID
Add a new function which return true if a virStorageSourcePtr is
a RAID.
For now, quorum is the only RAID we have.
This function is usefull, because, a lot of code access directly
to a virStorageSource internal member (like path) with some functions
like "virDomainDiskGetSource".
This beavious won't work with Quorum, and so we need to add
exeptions for these functions, but I'm not convinced by the idea to add a lot
of "disk->format == QUORUM" in all the code that deserve
exeption for Quorum, so I've add a generic function for this.
Signed-off-by: Matthias Gatto
---
src/libvirt_private.syms | 1 +
src/util/virstoragefile.c | 27 +++
src/util/virstoragefile.h | 2 ++
3 files changed, 30 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 4aa923a..efcea49 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2168,6 +2168,7 @@ virStorageSourceGetSecurityLabelDef;
virStorageSourceInitChainElement;
virStorageSourceIsEmpty;
virStorageSourceIsLocalStorage;
+virStorageSourceIsRAID;
virStorageSourceNewFromBacking;
virStorageSourceParseRBDColonString;
virStorageSourcePoolDefFree;
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 44bce91..a9cc0c8 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1808,6 +1808,33 @@ virStorageSourcePoolDefCopy(const
virStorageSourcePoolDef *src)
return NULL;
}
+/**
+ * virStorageSourceIsRAID:
+ * return true if the backingStores field inside @src is use
+ * as a child of a contener
+ */
+bool virStorageSourceIsRAID(virStorageSourcePtr src)
+{
+virStorageType type;
+
+if (!src)
+return false;
+type = virStorageSourceGetActualType(src);
+switch (type) {
+case VIR_STORAGE_TYPE_NONE:
+case VIR_STORAGE_TYPE_FILE:
+case VIR_STORAGE_TYPE_BLOCK:
+case VIR_STORAGE_TYPE_DIR:
+case VIR_STORAGE_TYPE_NETWORK:
+case VIR_STORAGE_TYPE_VOLUME:
+case VIR_STORAGE_TYPE_LAST:
+return false;
+
+case VIR_STORAGE_TYPE_QUORUM:
+return true;
+}
+return false;
+}
/**
* virStorageSourceGetBackingStore:
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 5f76b4b..c9f2afa 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -290,6 +290,8 @@ struct _virStorageSource {
# define DEV_BSIZE 512
# endif
+bool virStorageSourceIsRAID(virStorageSourcePtr src);
+
bool virStorageSourceSetBackingStore(virStorageSourcePtr src,
virStorageSourcePtr backingStore,
size_t pos);
--
2.6.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v6 01/13] virstoragefile: Add virStorageSourceGetBackingStore
Create virStorageSourceGetBackingStore function in
preparation for quorum:
Actually, if we want to get a backing store inside a virStorageSource
we have to do it manually(src->backingStore = backingStore).
The problem is that with a quorum, a virStorageSource
can contain multiple backing stores, and src->backingStore can
be treated as a virStorageSourcePtr or a virStorageSourcePtrPtr.
Due to these reason, we need to simplify the manipulation of
virStorageSource, and create a function to get the asked
backingStore in a virStorageSource
For now, this function only return the backingStore field
Signed-off-by: Matthias Gatto
---
src/libvirt_private.syms | 1 +
src/util/virstoragefile.c | 8
src/util/virstoragefile.h | 3 +++
3 files changed, 12 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 4b7e165..8854b26 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2163,6 +2163,7 @@ virStorageSourceClear;
virStorageSourceCopy;
virStorageSourceFree;
virStorageSourceGetActualType;
+virStorageSourceGetBackingStore;
virStorageSourceGetSecurityLabelDef;
virStorageSourceInitChainElement;
virStorageSourceIsEmpty;
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 2aa1d90..3fad323 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1809,6 +1809,14 @@ virStorageSourcePoolDefCopy(const
virStorageSourcePoolDef *src)
}
+virStorageSourcePtr
+virStorageSourceGetBackingStore(const virStorageSource *src,
+size_t pos ATTRIBUTE_UNUSED)
+{
+return src->backingStore;
+}
+
+
/**
* virStorageSourcePtr:
*
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index b98fe25..8cd4854 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -289,6 +289,9 @@ struct _virStorageSource {
# define DEV_BSIZE 512
# endif
+virStorageSourcePtr virStorageSourceGetBackingStore(const virStorageSource
*src,
+size_t pos);
+
int virStorageFileProbeFormat(const char *path, uid_t uid, gid_t gid);
int virStorageFileProbeFormatFromBuf(const char *path,
char *buf,
--
2.6.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] wireshark: Install to generic plugin directory
On Thu, Oct 29, 2015 at 01:38:44PM +0100, Michal Privoznik wrote:
There has been a report on the list [1] that we are not
installing the wireshark dissector into the correct plugin
directory. And in fact we are not. The problem is, the plugin
directory path is constructed at compile time. However, it's
dependent on the wireshark version, e.g.
/usr/lib/wireshark/1.12.6/plugins/
This should be plugins/1.12.6, I believe this is a problem originally
caused by me, in case you are using my wireshark.pc on your machine.
ACK with that changed, as this is the cleanest way we can do right
now, I believe.
This is rather unfortunate, because if libvirt RPMs were built
with one version, but installed on a system with newer one, the
plugins are not really loaded. This problem lead fedora packagers
to unify plugin path to:
/usr/lib/wireshark/plugins/
Cool! But this was enabled just in wireshark-1.12.6-4. Therefore,
we must require at least that version.
And while at it, on some distributions, the wireshark.pc file
already has a variable that defines where plugin dir is. Use that
if possible.
1: https://www.redhat.com/archives/libvirt-users/2015-October/msg00063.html
Signed-off-by: Michal Privoznik
---
libvirt.spec.in | 6 --
m4/virt-wireshark.m4 | 7 ++-
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 9dff994..469bfca 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1170,7 +1170,7 @@ virtualization capabilities of recent versions of Linux
(and other OSes).
%package wireshark
Summary: Wireshark dissector plugin for libvirt RPC transactions
Group: Development/Libraries
-Requires: wireshark
+Requires: wireshark >= 1.12.6-4
Requires: %{name}-client = %{version}-%{release}
%description wireshark
@@ -1561,6 +1561,8 @@ rm -f
$RPM_BUILD_ROOT%{_libdir}/libvirt/connection-driver/*.a
%endif
%if %{with_wireshark}
rm -f $RPM_BUILD_ROOT%{_libdir}/wireshark/plugins/*/libvirt.la
+mv $RPM_BUILD_ROOT%{_libdir}/wireshark/plugins/*/libvirt.so \
+ $RPM_BUILD_ROOT%{_libdir}/wireshark/plugins/libvirt.so
%endif
# Temporarily get rid of not-installed libvirt-admin.so
@@ -2279,7 +2281,7 @@ exit 0
%if %{with_wireshark}
%files wireshark
-%{_libdir}/wireshark/plugins/*/libvirt.so
+%{_libdir}/wireshark/plugins/libvirt.so
%endif
%if %{with_lxc}
diff --git a/m4/virt-wireshark.m4 b/m4/virt-wireshark.m4
index 47204ed..199317e 100644
--- a/m4/virt-wireshark.m4
+++ b/m4/virt-wireshark.m4
@@ -28,7 +28,12 @@ AC_DEFUN([LIBVIRT_CHECK_WIRESHARK],[
dnl Check for system location of wireshark plugins
if test "x$with_wireshark_dissector" != "xno" ; then
if test "x$with_ws_plugindir" = "xcheck" ; then
- ws_plugindir="$libdir/wireshark/plugins/$($PKG_CONFIG --modversion
wireshark)"
+ ws_plugindir="$($PKG_CONFIG --variable plugindir wireshark)"
+ if test "x$ws_plugindir" = "x" ; then
+dnl On some systems the plugindir variable may not be stored within
pkg config.
+dnl Fall back to older style of constructing the plugin dir path.
+ws_plugindir="$libdir/wireshark/plugins/$($PKG_CONFIG --modversion
wireshark)"
+ fi
elif test "x$with_ws_plugindir" = "xno" || test "x$with_ws_plugindir" =
"xyes"; then
AC_MSG_ERROR([ws-plugindir must be used only with valid path])
else
--
2.4.10
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] wireshark: Install to generic plugin directory
There has been a report on the list [1] that we are not
installing the wireshark dissector into the correct plugin
directory. And in fact we are not. The problem is, the plugin
directory path is constructed at compile time. However, it's
dependent on the wireshark version, e.g.
/usr/lib/wireshark/1.12.6/plugins/
This is rather unfortunate, because if libvirt RPMs were built
with one version, but installed on a system with newer one, the
plugins are not really loaded. This problem lead fedora packagers
to unify plugin path to:
/usr/lib/wireshark/plugins/
Cool! But this was enabled just in wireshark-1.12.6-4. Therefore,
we must require at least that version.
And while at it, on some distributions, the wireshark.pc file
already has a variable that defines where plugin dir is. Use that
if possible.
1: https://www.redhat.com/archives/libvirt-users/2015-October/msg00063.html
Signed-off-by: Michal Privoznik
---
libvirt.spec.in | 6 --
m4/virt-wireshark.m4 | 7 ++-
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 9dff994..469bfca 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1170,7 +1170,7 @@ virtualization capabilities of recent versions of Linux
(and other OSes).
%package wireshark
Summary: Wireshark dissector plugin for libvirt RPC transactions
Group: Development/Libraries
-Requires: wireshark
+Requires: wireshark >= 1.12.6-4
Requires: %{name}-client = %{version}-%{release}
%description wireshark
@@ -1561,6 +1561,8 @@ rm -f
$RPM_BUILD_ROOT%{_libdir}/libvirt/connection-driver/*.a
%endif
%if %{with_wireshark}
rm -f $RPM_BUILD_ROOT%{_libdir}/wireshark/plugins/*/libvirt.la
+mv $RPM_BUILD_ROOT%{_libdir}/wireshark/plugins/*/libvirt.so \
+ $RPM_BUILD_ROOT%{_libdir}/wireshark/plugins/libvirt.so
%endif
# Temporarily get rid of not-installed libvirt-admin.so
@@ -2279,7 +2281,7 @@ exit 0
%if %{with_wireshark}
%files wireshark
-%{_libdir}/wireshark/plugins/*/libvirt.so
+%{_libdir}/wireshark/plugins/libvirt.so
%endif
%if %{with_lxc}
diff --git a/m4/virt-wireshark.m4 b/m4/virt-wireshark.m4
index 47204ed..199317e 100644
--- a/m4/virt-wireshark.m4
+++ b/m4/virt-wireshark.m4
@@ -28,7 +28,12 @@ AC_DEFUN([LIBVIRT_CHECK_WIRESHARK],[
dnl Check for system location of wireshark plugins
if test "x$with_wireshark_dissector" != "xno" ; then
if test "x$with_ws_plugindir" = "xcheck" ; then
- ws_plugindir="$libdir/wireshark/plugins/$($PKG_CONFIG --modversion
wireshark)"
+ ws_plugindir="$($PKG_CONFIG --variable plugindir wireshark)"
+ if test "x$ws_plugindir" = "x" ; then
+dnl On some systems the plugindir variable may not be stored within
pkg config.
+dnl Fall back to older style of constructing the plugin dir path.
+ws_plugindir="$libdir/wireshark/plugins/$($PKG_CONFIG --modversion
wireshark)"
+ fi
elif test "x$with_ws_plugindir" = "xno" || test "x$with_ws_plugindir" =
"xyes"; then
AC_MSG_ERROR([ws-plugindir must be used only with valid path])
else
--
2.4.10
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: set error if DAD is not finished
On 10/29/2015 12:47 PM, Luyao Huang wrote: If DAD not finished in 5 seconds, user will get an unknown error like this: # virsh net-start ipv6 error: Failed to start network ipv6 error: An error occurred, but the cause is unknown Call virReportError to set an error. Signed-off-by: Luyao Huang --- I found the DAD will take 7 seconds on my machine, and i cannot create a network which use ipv6 now :( . Can we offer a way allow user to change this timeout ? maybe add a configuration file option in libvirtd.conf. Could you please send the related records of your sysctl -a? Max DAD timeout should be rand() % net.ipv6.conf.default.router_solicitation_delay + net.ipv6.conf.default.dad_transmits * net.ipv6.neigh.default.retrans_time_ms I wonder whether my calculations were faulty or it is your specific configuration. -- Your sincerely, Maxim Perevedentsev -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-users] virsh can't support VM offline blockcommit
Hi Kashyap Chamarthy: thank you very much for answer my question: 一： lead to VM filesystem becoming read-only 1： test case it lead to VM filesystem becoming read-only test case as follows: we want to snapshot for VM , to obtain VM incremental data，and use virsh blockcommit，qemu-img commit，qemu-img rebase to shorten snapshot chain. Details are as follows（when VM running state， we perform the following operations）: (1) the host machine control VM test virsh snapshot-create-as mix snap1 --diskspec vda,file=/tmp/mul/loop-mix-commit-rebase/snap1-mix.img --disk-only --atomic --quiesce virsh snapshot-create-as mix snap2 --diskspec vda,file=/tmp/mul/loop-mix-commit-rebase/snap2-mix.img --disk-only --atomic --quiesce virsh snapshot-create-as mix snap3 --diskspec vda,file=/tmp/mul/loop-mix-commit-rebase/snap3-mix.img --disk-only --atomic --quiesce virsh snapshot-create-as mix snap4 --diskspec vda,file=/tmp/mul/loop-mix-commit-rebase/snap4-mix.img --disk-only --atomic --quiesce virsh blockcommit mix vda --base /tmp/mul/loop-mix-commit-rebase/snap1-mix.img --top /tmp/mul/loop-mix-commit-rebase/snap2-mix.img --wait --verbose qemu-img commit -f qcow2 /tmp/mul/loop-mix-commit-rebase/snap1-mix.img qemu-img rebase -f qcow2 /tmp/mul/loop-mix-commit-rebase/snap3-mix.img -b /tmp/mul/loop-mix-commit-rebase/mix.img -F qcow2 -p virsh snapshot-delete mix snap1 --metadata rm -fr /tmp/mul/loop-mix-commit-rebase/snap1-mix.img virsh snapshot-delete mix snap2 --metadata rm -fr /tmp/mul/loop-mix-commit-rebase/snap2-mix.img virsh snapshot-create-as mix snap5 --diskspec vda,file=/tmp/mul/loop-mix-commit-rebase/snap5-mix.img --disk-only --atomic --quiesce virsh snapshot-create-as mix snap6 --diskspec vda,file=/tmp/mul/loop-mix-commit-rebase/snap6-mix.img --disk-only --atomic --quiesce virsh snapshot-create-as mix snap7 --diskspec vda,file=/tmp/mul/loop-mix-commit-rebase/snap7-mix.img --disk-only --atomic --quiesce virsh snapshot-create-as mix snap8 --diskspec vda,file=/tmp/mul/loop-mix-commit-rebase/snap8-mix.img --disk-only --atomic --quiesce virsh blockcommit mix vda --base /tmp/mul/loop-mix-commit-rebase/snap3-mix.img --top /tmp/mul/loop-mix-commit-rebase/snap6-mix.img --wait --verbose qemu-img commit -f qcow2 /tmp/mul/loop-mix-commit-rebase/snap3-mix.img qemu-img rebase -f qcow2 /tmp/mul/loop-mix-commit-rebase/snap7-mix.img -b /tmp/mul/loop-mix-commit-rebase/mix.img -F qcow2 -p virsh snapshot-delete mix snap3 --metadata rm -fr /tmp/mul/loop-mix-commit-rebase/snap3-mix.img virsh snapshot-delete mix snap4 --metadata rm -fr /tmp/mul/loop-mix-commit-rebase/snap4-mix.img virsh snapshot-delete mix snap5 --metadata rm -fr /tmp/mul/loop-mix-commit-rebase/snap5-mix.img virsh snapshot-delete mix snap6 --metadata rm -fr /tmp/mul/loop-mix-commit-rebase/snap6-mix.img virsh snapshot-create-as mix snap9 --diskspec vda,file=/tmp/mul/loop-mix-commit-rebase/snap9-mix.img --disk-only --atomic --quiesce virsh snapshot-create-as mix snap10 --diskspec vda,file=/tmp/mul/loop-mix-commit-rebase/snap10-mix.img --disk-only --atomic --quiesce virsh snapshot-create-as mix snap11 --diskspec vda,file=/tmp/mul/loop-mix-commit-rebase/snap11-mix.img --disk-only --atomic --quiesce virsh snapshot-create-as mix snap12 --diskspec vda,file=/tmp/mul/loop-mix-commit-rebase/snap12-mix.img --disk-only --atomic --quiesce virsh blockcommit mix vda --base /tmp/mul/loop-mix-commit-rebase/snap7-mix.img --top /tmp/mul/loop-mix-commit-rebase/snap10-mix.img --wait --verbose qemu-img commit -f qcow2 /tmp/mul/loop-mix-commit-rebase/snap7-mix.img qemu-img rebase -f qcow2 /tmp/mul/loop-mix-commit-rebase/snap11-mix.img -b /tmp/mul/loop-mix-commit-rebase/mix.img -F qcow2 -p virsh snapshot-delete mix snap7 --metadata rm -fr /tmp/mul/loop-mix-commit-rebase/snap7-mix.img virsh snapshot-delete mix snap8 --metadata rm -fr /tmp/mul/loop-mix-commit-rebase/snap8-mix.img virsh snapshot-delete mix snap9 --metadata rm -fr /tmp/mul/loop-mix-commit-rebase/snap9-mix.img virsh snapshot-delete mix snap10 --metadata rm -fr /tmp/mul/loop-mix-commit-rebase/snap10-mix.img virsh snapshot-create-as mix snap13 --diskspec vda,file=/tmp/mul/loop-mix-commit-rebase/snap13-mix.img --disk-only --atomic --quiesce virsh snapshot-create-as mix snap14 --diskspec vda,file=/tmp/mul/loop-mix-commit-rebase/snap14-mix.img --disk-only --atomic --quiesce virsh snapshot-create-as mix snap15 --diskspec vda,file=/tmp/mul/loop-mix-commit-rebase/snap15-mix.img --disk-only --atomic --quiesce virsh snapshot-create-as mix snap16 --diskspec vda,file=/tmp/mul/loop-mix-commit-rebase/snap16-mix.img --disk-only --atomic --quiesce virsh blockcommit mix vda --base /tmp/mul/loop-mix-commit-rebase/snap11-mix.img --top /tmp/mul/loop-mix-commit-rebase/snap14-mix.img --wait --verbose qemu-img commit -f qcow2 /tmp/mul/loop-mix-commit-rebase/snap11-mix.img qemu-img rebase -f qcow2 /tmp/mul/loop-mix-commit-rebase/snap15-mix.img -b /tmp/mul/loop-mix-commit-rebase/mix.img -F qcow2 -p virsh snapshot-delete
Re: [libvirt] [PATCH 2/3] Qemu: add CMT support
On Thu, Oct 29, 2015 at 14:02:29 +0800, Qiaowei Ren wrote:
> One RFC in
> https://www.redhat.com/archives/libvir-list/2015-June/msg01509.html
>
> CMT (Cache Monitoring Technology) can be used to measure the
> usage of cache by VM running on the host. This patch will
> extend the bulk stats API (virDomainListGetStats) to add this
> field. Applications based on libvirt can use this API to achieve
> cache usage of VM. Because CMT implementation in Linux kernel
> is based on perf mechanism, this patch will enable perf event
> for CMT when VM is created and disable it when VM is destroyed.
>
> Signed-off-by: Qiaowei Ren
> ---
> include/libvirt/libvirt-domain.h | 1 +
> src/qemu/qemu_domain.h | 3 ++
> src/qemu/qemu_driver.c | 48 ++
> src/qemu/qemu_process.c | 86
>
> 4 files changed, 138 insertions(+)
>
> diff --git a/include/libvirt/libvirt-domain.h
> b/include/libvirt/libvirt-domain.h
> index e8202cf..fb5e1f4 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -1764,6 +1764,7 @@ typedef enum {
> VIR_DOMAIN_STATS_VCPU = (1 << 3), /* return domain virtual CPU info */
> VIR_DOMAIN_STATS_INTERFACE = (1 << 4), /* return domain interfaces info
> */
> VIR_DOMAIN_STATS_BLOCK = (1 << 5), /* return domain block info */
> +VIR_DOMAIN_STATS_CACHE = (1 << 6), /* return domain block info */
This flag is not documented anywhere and the comment is completely
wrong.
> } virDomainStatsTypes;
>
> typedef enum {
> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
> index 54e1e7b..31bce33 100644
> --- a/src/qemu/qemu_domain.h
> +++ b/src/qemu/qemu_domain.h
> @@ -196,6 +196,9 @@ struct _qemuDomainObjPrivate {
>
> bool hookRun; /* true if there was a hook run over this domain */
>
> +int cmt_fd; /* perf handler for CMT */
So you declare cmt_fd as int...
> +
> +
Why two empty lines?
> /* Bitmaps below hold data from the auto NUMA feature */
> virBitmapPtr autoNodeset;
> virBitmapPtr autoCpuset;
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 4cfae03..8c678c9 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -19320,6 +19320,53 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver,
>
> #undef QEMU_ADD_COUNT_PARAM
>
> +static int
> +qemuDomainGetStatsCache(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
> +virDomainObjPtr dom,
> +virDomainStatsRecordPtr record,
> +int *maxparams,
> +unsigned int privflags ATTRIBUTE_UNUSED)
> +{
> +qemuDomainObjPrivatePtr priv = dom->privateData;
> +FILE *fd;
> +unsigned long long cache = 0;
> +int scaling_factor = 0;
> +
> +if (priv->cmt_fd <= 0)
Shouldn't this only check for cmt_fd < 0?
> +return -1;
> +
> +if (read(priv->cmt_fd, &cache, sizeof(uint64_t)) < 0) {
Shouldn't cache be declared as uint64_t then?
> +virReportSystemError(errno, "%s",
> + _("Unable to read cache data"));
> +return -1;
> +}
> +
> +fd = fopen("/sys/devices/intel_cqm/events/llc_occupancy.scale", "r");
> +if (!fd) {
> +virReportSystemError(errno, "%s",
> + _("Unable to open CMT scale file"));
> +return -1;
> +}
> +if (fscanf(fd, "%d", &scaling_factor) != 1) {
> +virReportSystemError(errno, "%s",
> + _("Unable to read CMT scale file"));
> +VIR_FORCE_FCLOSE(fd);
> +return -1;
> +}
> +VIR_FORCE_FCLOSE(fd);
> +
> +cache *= scaling_factor;
> +
> +if (virTypedParamsAddULLong(&record->params,
> +&record->nparams,
> +maxparams,
> +"cache.current",
> +cache) < 0)
Any documentation of this new statistics is missing. The commit message
doesn't really help understanding it either.
> +return -1;
> +
> +return 0;
> +}
> +
> typedef int
> (*qemuDomainGetStatsFunc)(virQEMUDriverPtr driver,
>virDomainObjPtr dom,
> @@ -19340,6 +19387,7 @@ static struct qemuDomainGetStatsWorker
> qemuDomainGetStatsWorkers[] = {
> { qemuDomainGetStatsVcpu, VIR_DOMAIN_STATS_VCPU, false },
> { qemuDomainGetStatsInterface, VIR_DOMAIN_STATS_INTERFACE, false },
> { qemuDomainGetStatsBlock, VIR_DOMAIN_STATS_BLOCK, true },
> +{ qemuDomainGetStatsCache, VIR_DOMAIN_STATS_CACHE, false },
> { NULL, 0, false }
> };
>
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index ba84182..00b889d 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -25,8 +25,11 @@
> #include
> #include
> #include
> +#include
> +#include
> #if defined(__linux__)
> # include
> +# include
What if there's
Re: [libvirt] [PATCH 3/3] cmt: add virsh support
On Thu, Oct 29, 2015 at 14:02:30 +0800, Qiaowei Ren wrote:
> This patch update domstats command to support CMT feature based on
> extended bulk stats API virDomainListGetStats.
>
> Signed-off-by: Qiaowei Ren
> ---
> tools/virsh-domain-monitor.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
> index 1d4dc25..28f7bf8 100644
> --- a/tools/virsh-domain-monitor.c
> +++ b/tools/virsh-domain-monitor.c
> @@ -2013,6 +2013,10 @@ static const vshCmdOptDef opts_domstats[] = {
> .type = VSH_OT_BOOL,
> .help = N_("report domain block device statistics"),
> },
> +{.name = "cache",
> + .type = VSH_OT_BOOL,
> + .help = N_("report domain cache statistics"),
> +},
> {.name = "list-active",
> .type = VSH_OT_BOOL,
> .help = N_("list only active domains"),
> @@ -2123,6 +2127,9 @@ cmdDomstats(vshControl *ctl, const vshCmd *cmd)
> if (vshCommandOptBool(cmd, "block"))
> stats |= VIR_DOMAIN_STATS_BLOCK;
>
> +if (vshCommandOptBool(cmd, "cache"))
> +stats |= VIR_DOMAIN_STATS_CACHE;
> +
> if (vshCommandOptBool(cmd, "list-active"))
> flags |= VIR_CONNECT_GET_ALL_DOMAINS_STATS_ACTIVE;
You need to document the option in virsh manpage.
Jirka
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] util: set error if DAD is not finished
If DAD not finished in 5 seconds, user will get an
unknown error like this:
# virsh net-start ipv6
error: Failed to start network ipv6
error: An error occurred, but the cause is unknown
Call virReportError to set an error.
Signed-off-by: Luyao Huang
---
I found the DAD will take 7 seconds
on my machine, and i cannot create a network which
use ipv6 now :( . Can we offer a way allow user to change this
timeout ? maybe add a configuration file option in
libvirtd.conf.
src/util/virnetdev.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 9789e93..c8861e9 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -1398,7 +1398,13 @@ virNetDevWaitDadFinish(virSocketAddrPtr *addrs, size_t
count)
VIR_FREE(resp);
}
/* Check timeout. */
-ret = dad ? -1 : 0;
+if (dad) {
+virReportError(VIR_ERR_SYSTEM_ERROR,
+ _("Duplicate Address Detection "
+ "not finished in %d seconds"), VIR_DAD_WAIT_TIMEOUT);
+} else {
+ret = 0;
+}
cleanup:
VIR_FREE(resp);
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Remove new lines from debug messages
On Tue, 2015-10-27 at 19:21 +0100, Jiri Denemark wrote:
> > Looks good, I would squash in the following:
> >
> > diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
> > index f85bd3e..6e00107 100644
> > --- a/src/util/virnetdevmacvlan.c
> > +++ b/src/util/virnetdevmacvlan.c
> > @@ -565,8 +565,7 @@ virNetDevMacVLanVPortProfileCallback(struct
> > nlmsghdr *hdr,
> > }
> > if (nla_parse_nested(tb3, IFLA_PORT_MAX, tb_vf_ports,
> > ifla_port_policy)) {
> > -VIR_DEBUG("nested parsing on level 2"
> > - " failed");
> > +VIR_DEBUG("nested parsing on level 2 failed");
> > }
> > if (tb3[IFLA_PORT_VF]) {
> > VIR_DEBUG("IFLA_PORT_VF = %d",
> >
> > and note in the commit message that you're also removing
> > trailing full stops from the messages.
>
> I dropped this hunk completely, removing full stops deserves a separate
> patch since we have a lot of them.
Fair enough.
> > Are those exceptions or did you just miss them? If the
> > latter, adding a syntax-check for this is probably a good
> > idea :)
>
> Unfortunately, they are exceptions and that's why I didn't add a
> syntax-check rule.
Too bad :(
Cheers.
--
Andrea Bolognani
Software Engineer - Virtualization Team
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] Remove new lines from debug messages
On Tue, 2015-10-27 at 19:26 +0100, Jiri Denemark wrote:
> VIR_DEBUG will automatically add a new line to the message, having
> "\n"
> at the end or at the beginning of the message results in empty lines.
>
> Signed-off-by: Jiri Denemark
> ---
> src/nwfilter/nwfilter_dhcpsnoop.c | 2 +-
> src/nwfilter/nwfilter_gentech_driver.c | 2 +-
> src/nwfilter/nwfilter_learnipaddr.c| 10 +-
> src/rpc/virnetsocket.c | 2 +-
> src/util/virfile.c | 2 +-
> src/util/virhash.c | 2 +-
> src/util/virnetdevmacvlan.c| 26
> src/util/virprocess.c | 2 +-
> src/xen/xend_internal.c| 2 +-
> tests/virhostdevtest.c | 36 +---
> --
> tests/virnetsockettest.c | 2 +-
> tests/virtimetest.c| 2 +-
> 12 files changed, 45 insertions(+), 45 deletions(-)
I believe you missed these:
---8<---
diff --git a/src/libvirt.c b/src/libvirt.c
index 2602dde..25a0040 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -1125,7 +1125,7 @@ do_open(const char *name,
" server %s\n"
" user %s\n"
" port %d\n"
- " path %s\n",
+ " path %s",
alias ? alias : name,
NULLSTR(ret->uri->scheme), NULLSTR(ret->uri->server),
NULLSTR(ret->uri->user), ret->uri->port,
diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c
b/src/nwfilter/nwfilter_dhcpsnoop.c
index bd6d25f..7dbf467 100644
--- a/src/nwfilter/nwfilter_dhcpsnoop.c
+++ b/src/nwfilter/nwfilter_dhcpsnoop.c
@@ -1509,7 +1509,7 @@ virNWFilterDHCPSnoopThread(void *req0)
if (last_displayed_queue - time(0) > 10) {
last_displayed_queue = time(0);
VIR_WARN("Worker thread for interface '%s' has a "
- "job queue that is too long\n",
+ "job queue that is too long",
req->ifname);
}
continue;
--->9---
and since you're changing warning messages as well, you
should probably include that information in the commit
message.
ACK with that fixed.
Cheers.
--
Andrea Bolognani
Software Engineer - Virtualization Team
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] cpu_map.xml: add cmt feature to x86
On Thu, Oct 29, 2015 at 14:02:28 +0800, Qiaowei Ren wrote: > Some Intel processor families (e.g. the Intel Xeon processor E5 v3 > family) introduced CMT (Cache Monitoring Technology) to measure the > usage of cache by applications running on the platform. This patch > add it into x86 part of cpu_map.xml. When sending a series of patches, please use --cover-letter to create a 0/n patch where you describe what the series is trying to achieve. As a nice side effect , individual patches will be sent as replies to the cover letter, which is much better than sending 2..n patches as replies to the first one. > Signed-off-by: Qiaowei Ren > --- > .gnulib | 2 +- > src/cpu/cpu_map.xml | 3 +++ > 2 files changed, 4 insertions(+), 1 deletion(-) > > diff --git a/.gnulib b/.gnulib > index f39477d..106a386 16 > --- a/.gnulib > +++ b/.gnulib > @@ -1 +1 @@ > -Subproject commit f39477dba778e99392948dd3dd19ec0d46aee932 > +Subproject commit 106a3866d01f9dd57ab4f10dbeb0d5a8db73a9f7 As Peter said, this hunk should be removed. > diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml > index b9e95cf..14ccbd8 100644 > --- a/src/cpu/cpu_map.xml > +++ b/src/cpu/cpu_map.xml > @@ -317,6 +317,9 @@ > > > > + > + > + > > > This looks like it makes sense, but it really doesn't. This patches causes libvirt to report cmt feature on host CPUs which support it, but what's the point since we are apparently not interested in exposing the feature to guests? Not to mention that even if the host CPU supports cmt, the host kernel does not have to supported so advertising the CPU feature doesn't really say whether it's usable or not. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] cpu_map.xml: add cmt feature to x86
> -Original Message- > From: Peter Krempa [mailto:pkre...@redhat.com] > Sent: Thursday, October 29, 2015 4:43 PM > To: Ren, Qiaowei > Cc: libvir-list@redhat.com; Feng, Shaohe > Subject: Re: [libvirt] [PATCH 1/3] cpu_map.xml: add cmt feature to x86 > > On Thu, Oct 29, 2015 at 14:02:28 +0800, Qiaowei Ren wrote: > > Some Intel processor families (e.g. the Intel Xeon processor E5 v3 > > family) introduced CMT (Cache Monitoring Technology) to measure the > > usage of cache by applications running on the platform. This patch add > > it into x86 part of cpu_map.xml. > > > > Signed-off-by: Qiaowei Ren > > --- > > .gnulib | 2 +- > > src/cpu/cpu_map.xml | 3 +++ > > 2 files changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/.gnulib b/.gnulib > > index f39477d..106a386 16 > > --- a/.gnulib > > +++ b/.gnulib > > @@ -1 +1 @@ > > -Subproject commit f39477dba778e99392948dd3dd19ec0d46aee932 > > +Subproject commit 106a3866d01f9dd57ab4f10dbeb0d5a8db73a9f7 > > This hunk should not be here. Gnulib versions are changed separately. > Also I doubt that it's necessary. > Yes. This should be not necessary here. Thanks, Qiaowei -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 3/3] virsh.pod: update and improve a attach-interface section
On Tue, Oct 27, 2015 at 06:53:55PM -0400, John Ferlan wrote: > > > On 10/27/2015 11:01 AM, Pavel Hrdina wrote: > > Rewrite the attach-interface section in man page to be more readable and > > add the new hostdev functionality. > > > > Signed-off-by: Pavel Hrdina > > --- > > tools/virsh.pod | 82 > > +++-- > > 1 file changed, 50 insertions(+), 32 deletions(-) > > > > Why a separate patch? It's related to the previous one and if anyone > ever (ahem) backed ported the other one, they could miss this one... No > strong feeling either way - just curious. It's a rewrite of the attach-interface part of the man page, so I thought, that would be better to do it in a separate patch. Maybe I can at first do the rewrite without adding anything about the new feature and than have the update of man page together with previous patch. > > > diff --git a/tools/virsh.pod b/tools/virsh.pod > > index 0212e7a..843c293 100644 > > --- a/tools/virsh.pod > > +++ b/tools/virsh.pod > > @@ -2507,51 +2507,69 @@ Likewise, I<--shareable> is an alias for I<--mode > > shareable>. > > [[[I<--live>] [I<--config>] | [I<--current>]] | [I<--persistent>]] > > [I<--target target>] [I<--mac mac>] [I<--script script>] [I<--model model>] > > [I<--inbound average,peak,burst,floor>] [I<--outbound average,peak,burst>] > > -[I<--print-xml>] > > - > > -Attach a new network interface to the domain. I can be > > -I to indicate connection via a libvirt virtual network, or > > -I to indicate connection via a bridge device on the host, or > > -I to indicate connection directly to one of the host's network > > -interfaces or bridges. I indicates the source of the > > -connection (the name of a network, or of a bridge device, or the > > -host's network interfaces or bridges). I is used to specify > > -the tap/macvtap device to be used to connect the domain to the > > -source. Names starting with 'vnet' are considered as auto-generated > > -and are blanked out/regenerated each time the interface is attached. > > -I specifies the MAC address of the network interface; if a MAC > > +[I<--managed>] [I<--print-xml>] > > + > > +Attach a new network interface to the domain. > > + > > +B<--type> can be one of the: I to indicate connection via a > > libvirt > > +virtual network, I to indicate connection via a bridge device > > +on the host, I to indicate connection directly to one of the host's > > +network interfaces or bridges, I to indicate connection using a > > +passthrough of PCI device on the host. > > Using a ':' I think is unnecessary unless you somehow generate a real > list such as via "=item * " entries. In that case you'd have the > following: > I've considered this format before writing the patch and it used a lot of space. I agree, that it would be better arranged. I'll update it. > +B<--type> can be one of the following: > + > +=over 4 > + > +=item * Use I to indicate connection via a libvirt virtual network > + > +=item * Use I to indicate connection via a bridge device on the > host > + > + > +=item * Use I to indicate connection directly to one of the host's > +network interfaces or bridges > + > +=item * Use I to indicate connection using a passthrough of > PCI device > +on the host. > + > +=back > > NB: Whether the '--' is required on the type is perhaps a matter of > opinion... Since the command line shown doesn't list --type shouldn't > this still be an 'B'? Oh, You're right, there is no '--type' in the man page. In that case, it should be also referenced the same way. I've used it probably because you can write something like this "attach-interface --domain test --type hostdev ...". > > > + > > +B<--source> indicates the source of the connection. The source depends > > +on the type of the interface: I name of the virtual network, > > +I the name of the bridge device, I the name of the host's > > +interface or bridge, I the PCI address of the host's interface > > +formatted as domain:bus:slot.function. > > Similar comment/construct here and same comment about '--' for source. > > > + > > +B<--target> is used to specify the tap/macvtap device to be used to > > +connect the domain to the source. Names starting with 'vnet' are > > +considered as auto-generated and are blanked out/regenerated each > > +time the interface is attached. > > + > > +B<--mac> specifies the MAC address of the network interface; if a MAC > > B<--target> and B<--mac> seem OK... > > > address is not given, a new address will be automatically generated > > (and stored in the persistent configuration if "--config" is given on > > -the commandline). I
Re: [libvirt] [PATCH 1/3] cpu_map.xml: add cmt feature to x86
On Thu, Oct 29, 2015 at 14:02:28 +0800, Qiaowei Ren wrote: > Some Intel processor families (e.g. the Intel Xeon processor E5 v3 > family) introduced CMT (Cache Monitoring Technology) to measure the > usage of cache by applications running on the platform. This patch > add it into x86 part of cpu_map.xml. > > Signed-off-by: Qiaowei Ren > --- > .gnulib | 2 +- > src/cpu/cpu_map.xml | 3 +++ > 2 files changed, 4 insertions(+), 1 deletion(-) > > diff --git a/.gnulib b/.gnulib > index f39477d..106a386 16 > --- a/.gnulib > +++ b/.gnulib > @@ -1 +1 @@ > -Subproject commit f39477dba778e99392948dd3dd19ec0d46aee932 > +Subproject commit 106a3866d01f9dd57ab4f10dbeb0d5a8db73a9f7 This hunk should not be here. Gnulib versions are changed separately. Also I doubt that it's necessary. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-test-api][PATCH] Add config flag when calling setVcpusFlags with maximum flag
ACK and Pushed
On 10/29/2015 04:22 PM, Hongming Zhang wrote:
Flag 'VIR_DOMAIN_AFFECT_CONFIG' is required by flag 'VIR_DOMAIN_VCPU_MAXIMUM'
modified: set_vcpus_config.py
---
repos/setVcpus/set_vcpus_config.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/repos/setVcpus/set_vcpus_config.py
b/repos/setVcpus/set_vcpus_config.py
index 3bb3984..3ef0992 100644
--- a/repos/setVcpus/set_vcpus_config.py
+++ b/repos/setVcpus/set_vcpus_config.py
@@ -80,7 +80,7 @@ def set_vcpus_config(params):
return 1
if maxvcpu:
-flags = libvirt.VIR_DOMAIN_VCPU_MAXIMUM
+flags =
libvirt.VIR_DOMAIN_VCPU_MAXIMUM|libvirt.VIR_DOMAIN_AFFECT_CONFIG
logger.info("the given max vcpu number is %s" % maxvcpu)
logger.info("set domain maximum vcpu as %s with flag: %s" %
(maxvcpu, flags))
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-test-api][PATCH] Add config flag when calling setVcpusFlags with maximum flag
Flag 'VIR_DOMAIN_AFFECT_CONFIG' is required by flag 'VIR_DOMAIN_VCPU_MAXIMUM'
modified: set_vcpus_config.py
---
repos/setVcpus/set_vcpus_config.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/repos/setVcpus/set_vcpus_config.py
b/repos/setVcpus/set_vcpus_config.py
index 3bb3984..3ef0992 100644
--- a/repos/setVcpus/set_vcpus_config.py
+++ b/repos/setVcpus/set_vcpus_config.py
@@ -80,7 +80,7 @@ def set_vcpus_config(params):
return 1
if maxvcpu:
-flags = libvirt.VIR_DOMAIN_VCPU_MAXIMUM
+flags =
libvirt.VIR_DOMAIN_VCPU_MAXIMUM|libvirt.VIR_DOMAIN_AFFECT_CONFIG
logger.info("the given max vcpu number is %s" % maxvcpu)
logger.info("set domain maximum vcpu as %s with flag: %s" %
(maxvcpu, flags))
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-test-api][PATCH 0/2]Fix issues in the method of getting vcpu thread id of vcpupin_live and cpu_affinity
ACK to all of them BR, Luyao - Original Message - From: "Hongming Zhang" To: libvir-list@redhat.com Sent: Thursday, October 29, 2015 3:19:29 PM Subject: [libvirt] [libvirt-test-api][PATCH 0/2]Fix issues in the method of getting vcpu thread id of vcpupin_live and cpu_affinity Hongming Zhang (2): Fix the issues in checking method of vcpupin_live Modify the checking method via passing the vcpu thread id repos/domain/cpu_affinity.py | 16 +--- repos/setVcpus/vcpupin_live.py | 36 2 files changed, 29 insertions(+), 23 deletions(-) -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Entering freeze for libvirt-1.2.21
As pointed our on Tuesday it's time for a new release. I have tagged the release candidate 1 in git and pushed signed tarball and rpms to the usual place at: ftp://libvirt.org/libvirt/ Based on my limited testing this works just fine, but that's very limited and doesn't test portability at all, so please give it a try ! I will likely push rc2 on the week-end and the final version on Tuesday or Wednesday, please raise issues if you fine any, thanks, Daniel -- Daniel Veillard | Open Source and Standards, Red Hat veill...@redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-test-api][PATCH 0/2]Fix issues in the method of getting vcpu thread id of vcpupin_live and cpu_affinity
Hongming Zhang (2): Fix the issues in checking method of vcpupin_live Modify the checking method via passing the vcpu thread id repos/domain/cpu_affinity.py | 16 +--- repos/setVcpus/vcpupin_live.py | 36 2 files changed, 29 insertions(+), 23 deletions(-) -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-test-api][PATCH 1/2] Fix the issues in checking method of vcpupin_live
The new method will get the vcpu thread id via qemu-monitor.
Then get the Cpus_allowed_list value from /proc using the vcpu's
thread id
modified: repos/setVcpus/vcpupin_live.py
---
repos/setVcpus/vcpupin_live.py | 36
1 file changed, 20 insertions(+), 16 deletions(-)
diff --git a/repos/setVcpus/vcpupin_live.py b/repos/setVcpus/vcpupin_live.py
index c3dfe8e..9f21583 100644
--- a/repos/setVcpus/vcpupin_live.py
+++ b/repos/setVcpus/vcpupin_live.py
@@ -13,30 +13,34 @@ from utils import utils
required_params = ('guestname', 'vcpu', 'cpulist',)
optional_params = {}
-def vcpupin_check(guestname, vcpu, cpumap):
+def vcpupin_check(guestname, vcpu, cpulist):
"""check vcpu subprocess status of the running virtual machine
grep Cpus_allowed_list /proc/PID/task/*/status
"""
-tmp_str = ''
-cmd = "cat /var/run/libvirt/qemu/%s.pid" % guestname
-status, pid = utils.exec_cmd(cmd, shell=True)
+cmd_pid = "cat /var/run/libvirt/qemu/%s.pid" % guestname
+status, pid = utils.exec_cmd(cmd_pid, shell=True)
if status:
logger.error("failed to get the pid of domain %s" % guestname)
return 1
-cmd = "grep Cpus_allowed_list /proc/%s/task/*/status" % pid[0]
-status, output = utils.exec_cmd(cmd, shell=True)
-logger.debug("command '%s' output is:" % cmd)
-for i in range(len(output)):
-tmp_str += ''.join(output[i]) + '\n'
-logger.debug(tmp_str)
+cmd_vcpu_task_id = "virsh qemu-monitor-command %s --hmp info cpus|grep
'#%s'|cut -d '=' -f3"\
+% (guestname,vcpu)
+status, vcpu_task_id = utils.exec_cmd(cmd_vcpu_task_id, shell=True)
+if status:
+logger.error("failed to get the threadid of domain %s" % guestname)
+return 1
+
+logger.debug("vcpu id %s:" % vcpu_task_id[0])
+cmd_cpus_allowed_list = "grep Cpus_allowed_list /proc/%s/task/%s/status" %
(pid[0] , vcpu_task_id[0])
+status, output = utils.exec_cmd(cmd_cpus_allowed_list, shell=True)
+if status:
+logger.error("failed to get the cpu_allowed_list of vcpu %s")
+return 1
-task_list = output[1:]
-vcpu_task = task_list[int(vcpu)]
-cpulist = vcpu_task.split('\t')[1]
-ret = utils.param_to_tuple(cpulist, maxcpu)
+logger.debug("the output of command 'grep Cpus_allowed_list \
+ /proc/%s/task/%s/status' is %s" %
(pid[0],vcpu_task_id[0],output))
-if ret == cpumap:
+if output[0].split('\t')[1] == cpulist:
logger.info("vcpu process cpus allowed list is expected")
return 0
else:
@@ -92,7 +96,7 @@ def vcpupin_live(params):
return 1
logger.info("check vcpu pin status on host")
-ret = vcpupin_check(guestname, vcpu, cpumap)
+ret = vcpupin_check(guestname, vcpu, cpulist)
if ret:
logger.error("domain vcpu pin failed")
return 1
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-test-api][PATCH 2/2] Modify the checking method via passing the vcpu thread id
modified: repos/domain/cpu_affinity.py
---
repos/domain/cpu_affinity.py | 16 +---
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/repos/domain/cpu_affinity.py b/repos/domain/cpu_affinity.py
index e710968..8246938 100644
--- a/repos/domain/cpu_affinity.py
+++ b/repos/domain/cpu_affinity.py
@@ -142,16 +142,18 @@ def vcpu_affinity_check(domain_name, vcpu,
expected_pinned_cpu, hypervisor):
logger.error("failed to get the pid of \
the running virtual machine process")
return 1
-if 'el6' in host_kernel_version:
-cmd_get_task_list = "grep Cpus_allowed_list
/proc/%s/task/*/status" % pid
+if 'el6' or 'el7' in host_kernel_version:
+cmd_vcpu_task_id = "virsh qemu-monitor-command %s --hmp info
cpus|grep '#%s'|cut -d '=' -f3"\
+% (domain_name,vcpu)
+status, output = commands.getstatusoutput(cmd_vcpu_task_id)
+vcpu_task_id = output[:output.find("^")]
+logger.debug("vcpu id %s:" % vcpu_task_id)
+cmd_get_task_list = "grep Cpus_allowed_list
/proc/%s/task/%s/status" % (pid , vcpu_task_id)
status, output = commands.getstatusoutput(cmd_get_task_list)
-
logger.debug("the output of command 'grep Cpus_allowed_list \
- /proc/%s/task/*/status' is %s" % (pid, output))
+ /proc/%s/task/%s/status' is %s" %
(pid,vcpu_task_id,output))
+actual_pinned_cpu = int(output.split('\t')[1])
-task_list = output.split('\n')[1:]
-vcpu_task = task_list[int(vcpu)]
-actual_pinned_cpu = int(vcpu_task.split('\t')[1])
elif 'el5' in host_kernel_version:
cmd_get_task_list = "grep Cpus_allowed /proc/%s/task/*/status" %
pid
status, output = commands.getstatusoutput(cmd_get_task_list)
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 2/3] virsh-domain: update attach-interface to support type=hostdev
On Tue, Oct 27, 2015 at 06:16:33PM -0400, John Ferlan wrote:
>
>
> On 10/27/2015 11:01 AM, Pavel Hrdina wrote:
> > Adding this feature will allow users to easily attach a hostdev network
> > interface using PCI passthrough.
> >
> > The interface can be attached using --type=hostdev and PCI address or
> > as --source. This command also allows you to tell, whether the interface
> > should be managed.
> >
> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=997561
> >
> > Signed-off-by: Pavel Hrdina
> > ---
> > tools/virsh-domain.c | 34 --
> > 1 file changed, 32 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> > index 12e85e3..bd00785 100644
> > --- a/tools/virsh-domain.c
> > +++ b/tools/virsh-domain.c
> > @@ -56,6 +56,7 @@
> > #include "virtime.h"
> > #include "virtypedparam.h"
> > #include "virxml.h"
> > +#include "virsh-nodedev.h"
> >
> > /* Gnulib doesn't guarantee SA_SIGINFO support. */
> > #ifndef SA_SIGINFO
> > @@ -866,6 +867,10 @@ static const vshCmdOptDef opts_attach_interface[] = {
> > .type = VSH_OT_BOOL,
> > .help = N_("print XML document rather than attach the interface")
> > },
> > +{.name = "managed",
> > + .type = VSH_OT_BOOL,
> > + .help = N_("libvirt will automatically detach/attach the device
> > from/to host")
> > +},
> > {.name = NULL}
> > };
> >
> > @@ -931,6 +936,7 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
> > bool config = vshCommandOptBool(cmd, "config");
> > bool live = vshCommandOptBool(cmd, "live");
> > bool persistent = vshCommandOptBool(cmd, "persistent");
> > +bool managed = vshCommandOptBool(cmd, "managed");
> >
> > VSH_EXCLUSIVE_OPTIONS_VAR(persistent, current);
> >
> > @@ -983,7 +989,12 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
> > }
> >
> > /* Make XML of interface */
> > -virBufferAsprintf(&buf, "\n", type);
> > +virBufferAsprintf(&buf, " > +
> > +if (managed)
> > +virBufferAddLit(&buf, " managed='yes'>\n");
> > +else
> > +virBufferAddLit(&buf, ">\n");
> > virBufferAdjustIndent(&buf, 2);
> >
> > switch (typ) {
> > @@ -995,6 +1006,26 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
> > case VIR_DOMAIN_NET_TYPE_DIRECT:
> > virBufferAsprintf(&buf, "\n", source);
> > break;
> > +case VIR_DOMAIN_NET_TYPE_HOSTDEV:
> > +{
> > +struct PCIAddress pciAddr = {0, 0, 0, 0};
> > +
> > +if (str2PCIAddress(source, &pciAddr) < 0) {
> > +vshError(ctl, _("cannot parse pci address '%s' for network "
> > +"interface"), source);
> > +goto cleanup;
> > +}
> > +
> > +virBufferAddLit(&buf, "\n");
> > +virBufferAdjustIndent(&buf, 2);
> > +virBufferAsprintf(&buf, " > + " bus='0x%.2x' slot='0x%.2x'
> > function='0x%.1x'/>\n",
> > + pciAddr.domain, pciAddr.bus,
> > + pciAddr.slot, pciAddr.function);
> > +virBufferAdjustIndent(&buf, -2);
> > +virBufferAddLit(&buf, "\n");
> > +break;
> > +}
> >
> > case VIR_DOMAIN_NET_TYPE_USER:
> > case VIR_DOMAIN_NET_TYPE_ETHERNET:
> > @@ -1004,7 +1035,6 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
> > case VIR_DOMAIN_NET_TYPE_MCAST:
> > case VIR_DOMAIN_NET_TYPE_UDP:
> > case VIR_DOMAIN_NET_TYPE_INTERNAL:
> > -case VIR_DOMAIN_NET_TYPE_HOSTDEV:
> > case VIR_DOMAIN_NET_TYPE_LAST:
> > vshError(ctl, _("No support for %s in command 'attach-interface'"),
> > type);
> >
>
> What happens if someone provides --managed with some other 'typ'?
>
> IOW: If it's only being supported by , then after the switch, a
> check should probably occur for "if (managed && typ !=
> VIR_DOMAIN_NET_TYPE_HOSTDEV), message, and fail.
The check was there, but then I removed it because other arguments doesn't check
the usability too. We don't need to error out, because libvirt just ignore
the "managed='yes'" in the XML.
>
> I'm not fully clear after reading the bz and the previous review whether
> this patch resolves the bz - something to be worked out in the bz for
> history's sake though
I think, that the main issue with the BZ is that there was no easy way how to
attach *hostdev* interface without manually creating the XML for that interface.
We established with Laine, that there is not need for translating netdev name to
PCI address.
>
> I think with the adjustment to check whether managed is supplied for non
> hostdev, then you have an ACK for what's changed here.
Reconsider the 'managed' check, we can be strict and check whether it's used
only with hosted type or not, but it's not necessary.
Thanks,
Pavel
>
> John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvi
Re: [libvirt] [PATCH v2 1/3] virsh-nodedev: makes struct and functions for NodeDevice list available
On Tue, Oct 27, 2015 at 06:10:35PM -0400, John Ferlan wrote:
> $SUBJ: Expose virshNodeDeviceList{Collect|Free} and virshNodeDeviceList
> struct
>
> On 10/27/2015 11:01 AM, Pavel Hrdina wrote:
> > Next patch will use those function to collect NodeDevice list and find a
> > specific device. Make functions virshNodeDeviceListCollect() and
> > virshNodeDeviceListFree() together with struct virshNodeDeviceList
> > available to reuse existing code.
> >
>
> Exposing virshNodeDeviceListCollect, virshNodeDeviceListFree, and
> virshNodeDeviceList allows the data returned to be available to other
> virsh API's that may need them in the future.
>
> > Signed-off-by: Pavel Hrdina
> > ---
> > tools/virsh-nodedev.c | 16 +---
> > tools/virsh-nodedev.h | 11 +++
> > 2 files changed, 16 insertions(+), 11 deletions(-)
> >
>
> OK - all that said, but your future patches don't use these functions,
> so is there really any use for this patch yet? It seems your 2/3 has
> removed what was in the 3/4 in your prior series related to calling
> virshNodeDeviceListCollect (and noted in your cover letter as being
> removed).
>
> I don't oppose the change, but it doesn't seem necessary.
Thanks, I don't know why I didn't remove this patch from series, because it's
not required anymore. I'm not pushing this one.
Thanks,
Pavel
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
