Re: [libvirt] [PATCH 0/3] LXC guest network device name changes
On Wed, 2014-07-02 at 23:00 +0200, Richard Weinberger wrote: > On Wed, Jul 2, 2014 at 3:57 PM, Cédric Bosdonnat wrote: > > This patch series allows users to configure the network device name in the > > LXC container. I intentionaly didn't allow this for hostdev net interfaces > > as the NIC would be returned with a different name to the host and we have > > no way to guess under what name it was returned to the host namespace. > > > > This will help filling the feature gap with LXC WRT network configuration. > > We need this feature because you want to change the interface name and drop > CAP_NET_ADMIN? Well that is mostly the beginning of another patch series to help converting lxc.network.ipv[46] properties too. I have no feedback on how LXC users are using lxc.network.name in their containers. -- Cedric -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/5] Utilize virDomainDiskAuth for domain disk
On 07/02/2014 05:44 PM, John Ferlan wrote: > On 07/02/2014 09:10 AM, Ján Tomko wrote: >> On 06/27/2014 05:11 PM, John Ferlan wrote: >>> @@ -1802,7 +1790,7 @@ virStorageSourceClear(virStorageSourcePtr def) >>> VIR_FREE(def->timestamps); >>> >>> virStorageNetHostDefFree(def->nhosts, def->hosts); >>> -virStorageSourceAuthClear(def); >>> +virStorageAuthDefFree(def->auth); >> >> I don't like *Clear functions leaving pointers to freed memory behind, but >> this one is only called right before freeing the StorageSource and it already >> leaves def->hosts. >> > > I think you are asking for a 'def->auth = NULL;' right? Yes. > Similarly a 'def->hosts = NULL;' Of course it doesn't matter > since we're freeing def anyway. If you want it - I can put > it there. I think that's better left for a separate cleanup. I'll make a note on my TODO list :) Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Fwd: [PATCH] qemu: Add cmd_per_lun, max_sectors to virtio-scsi
On Wed, Jul 02, 2014 at 06:48:36PM -0600, Eric Blake wrote: > On 07/02/2014 06:28 PM, Nicholas A. Bellinger wrote: > > >> QEMU is not the only hypervisor that libvirt targets, so tieing libvirt > >> names to QEMU names is a non-goal. We pick the names that make most sense > >> in the context of libvirt. > >> > > > > Not sure I follow.. virtio-scsi is specific to QEMU/KVM, and per the > > comment in the original patch: > > > > 'Currently it only supports attribute queues ( > class="since">1.0.5, QEMU and KVM only)' > > > > would seem to indicate the parameter names are only used in the context > > of QEMU/KVM, no..? > > Just because qemu is the only hypervisor driver that _currently_ uses > the feature doesn't preclude the libxl hypervisor from _also_ gaining > support for the feature in a future libvirt release, at which point the > documentation would mention the new version number for the additional > use of the feature. Again, the name qemu chose is not necessarily the > best name compared to what it might map to in libxl or any other > hypervisor, so libvirt tries to pick names that are consistent with > other libvirt terms, even if they don't match underlying qemu names. > > > > > If the virtio-scsi parameters are intended to be used across > > hypervisors, then matching them to QEMU's own names doesn't really > > matter. But if they are specific to virtio-scsi and only used by > > QEMU/KVM instances, then renaming them to something arbitrary to libvirt > > is pointless and confusing. > > virtio is not necessarily a qemu-only concept. Indeed, virtio is already used outside of QEMU & Linux by the FreeBSD BHyve virt platform. 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
Re: [libvirt] [PATCH 00/12] OOM reporting cleanups
On 07/02/2014 11:03 PM, Jiri Denemark wrote: > On Wed, Jul 02, 2014 at 12:10:53 +0200, Jano Tomko wrote: >> Patches 1-3 don't touch any code >> 4-5 no functional change >> 6 introduces a new function >> 7-8 have no functional change unless virBuffer API is misused >> (that case resulted in an OOM error before, now it's an internal >>error) >> 9-10 add OOM error reporting to a few functions >> 11-12 remove double OOM error reporting >> >> Ján Tomko (12): >> Fix indentation in bridge driver >> More indentation fixes >> usb: Remove redundant comment >> Remove useless condition in networkRadvdConfContents >> Use virStringReplace instead of openvz_replace >> Introduce virBufferCheckError >> Report errors in virCapabilitiesFormatXML >> Use virBufferCheckError everywhere we report OOM error >> Set errno on OOM in lxcProcReadMeminfo >> Add OOM error reporting to a few fucntions >> Remove double OOM error reporting from JSON monitor >> Remove double OOM error reporting > > ACK series after fixing the small issue I pointed out in 7/12. > > Jirka I've fixed the issue and pushed the series. Thanks! Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] LXC guest network device name changes
Am 03.07.2014 09:15, schrieb Cedric Bosdonnat: > On Wed, 2014-07-02 at 23:00 +0200, Richard Weinberger wrote: >> On Wed, Jul 2, 2014 at 3:57 PM, Cédric Bosdonnat wrote: >>> This patch series allows users to configure the network device name in the >>> LXC container. I intentionaly didn't allow this for hostdev net interfaces >>> as the NIC would be returned with a different name to the host and we have >>> no way to guess under what name it was returned to the host namespace. >>> >>> This will help filling the feature gap with LXC WRT network configuration. >> >> We need this feature because you want to change the interface name and drop >> CAP_NET_ADMIN? > > Well that is mostly the beginning of another patch series to help > converting lxc.network.ipv[46] properties too. I have no feedback on how > LXC users are using lxc.network.name in their containers. Okay. I'd assume that ifrename does the job. :) Thanks, //richard -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH for 1.2.7 8/8] qemu: Implement virConnectGetDomainCapabilities
On 02.07.2014 16:56, Daniel P. Berrange wrote: On Mon, Jun 30, 2014 at 05:31:51PM +0200, Michal Privoznik wrote: So far only information on disks and host devices are exposed in the capabilities XML. Well, at least something. Even a new test is introduced. The qemu capabilities are stolen from already existing qemucapabilities test. There's one tricky point though. Functions that checks host's KVM and VFIO capabilities, are impossible to mock currently. So in the test, we are setting the capabilities by hand. Signed-off-by: Michal Privoznik --- src/libvirt_private.syms | 1 + src/qemu/qemu_capabilities.c | 90 ++ src/qemu/qemu_capabilities.h | 4 + src/qemu/qemu_driver.c | 101 + tests/Makefile.am | 5 + .../domaincaps-qemu_1.6.50-1.xml | 44 + tests/domaincapstest.c | 45 + 7 files changed, 290 insertions(+) create mode 100644 tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml +static void +virQEMUCapsFillDomainDeviceDiskCaps(virQEMUCapsPtr qemuCaps, +virDomainCapsDeviceDiskPtr disk) +{ +disk->device.supported = true; +/* QEMU supports all of these */ +VIR_DOMAIN_CAPS_ENUM_SET(disk->diskDevice, + VIR_DOMAIN_DISK_DEVICE_DISK, + VIR_DOMAIN_DISK_DEVICE_CDROM, + VIR_DOMAIN_DISK_DEVICE_FLOPPY); + +if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_BLK_SG_IO)) +VIR_DOMAIN_CAPS_ENUM_SET(disk->diskDevice, VIR_DOMAIN_DISK_DEVICE_LUN); + +VIR_DOMAIN_CAPS_ENUM_SET(disk->bus, + VIR_DOMAIN_DISK_BUS_IDE, + VIR_DOMAIN_DISK_BUS_FDC, + VIR_DOMAIN_DISK_BUS_SCSI, + VIR_DOMAIN_DISK_BUS_VIRTIO, + VIR_DOMAIN_DISK_BUS_SD); I have a feeling that 'SD' is not supported in all QEMU's we claim to work with, though perhaps we've never checked this before when building the CLI args. Well, we don't. I haven't found any code that checks for 'SD' in qemu driver. I can remove it until the time we have a resolution. + +if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_USB_STORAGE)) +VIR_DOMAIN_CAPS_ENUM_SET(disk->bus, VIR_DOMAIN_DISK_BUS_USB); +} + + + + Hmm, so that's claiming we don't support any values for the mode attribute, but we support subsys. Ouch, yeah. The problem is, I was not setting the correct struct member. Here's the fix: index 9fc58ff..6ed85a9 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -3603,7 +3603,7 @@ virQEMUCapsFillDomainDeviceHostdevCaps(virQEMUCapsPtr qemuCaps, hostdev->device.supported = true; /* VIR_DOMAIN_HOSTDEV_MODE_CAPABILITIES is for containers only */ -VIR_DOMAIN_CAPS_ENUM_SET(hostdev->subsysType, +VIR_DOMAIN_CAPS_ENUM_SET(hostdev->mode, VIR_DOMAIN_HOSTDEV_MODE_SUBSYS); VIR_DOMAIN_CAPS_ENUM_SET(hostdev->startupPolicy, diff --git a/tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml b/tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml index 562e2f4..b7d9c26 100644 --- a/tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml +++ b/tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml @@ -21,7 +21,9 @@ - + +subsystem + default mandatory Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH for 1.2.7 8/8] qemu: Implement virConnectGetDomainCapabilities
On Thu, Jul 03, 2014 at 11:55:44AM +0200, Michal Privoznik wrote: > On 02.07.2014 16:56, Daniel P. Berrange wrote: > >On Mon, Jun 30, 2014 at 05:31:51PM +0200, Michal Privoznik wrote: > >>So far only information on disks and host devices are exposed in the > >>capabilities XML. Well, at least something. Even a new test is > >>introduced. The qemu capabilities are stolen from already existing > >>qemucapabilities test. There's one tricky point though. Functions that > >>checks host's KVM and VFIO capabilities, are impossible to mock > >>currently. So in the test, we are setting the capabilities by hand. > >> > >>Signed-off-by: Michal Privoznik > >>--- > >> src/libvirt_private.syms | 1 + > >> src/qemu/qemu_capabilities.c | 90 > >> ++ > >> src/qemu/qemu_capabilities.h | 4 + > >> src/qemu/qemu_driver.c | 101 > >> + > >> tests/Makefile.am | 5 + > >> .../domaincaps-qemu_1.6.50-1.xml | 44 + > >> tests/domaincapstest.c | 45 + > >> 7 files changed, 290 insertions(+) > >> create mode 100644 tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml > >> > > > > > >>+static void > >>+virQEMUCapsFillDomainDeviceDiskCaps(virQEMUCapsPtr qemuCaps, > >>+virDomainCapsDeviceDiskPtr disk) > >>+{ > >>+disk->device.supported = true; > >>+/* QEMU supports all of these */ > >>+VIR_DOMAIN_CAPS_ENUM_SET(disk->diskDevice, > >>+ VIR_DOMAIN_DISK_DEVICE_DISK, > >>+ VIR_DOMAIN_DISK_DEVICE_CDROM, > >>+ VIR_DOMAIN_DISK_DEVICE_FLOPPY); > >>+ > >>+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_BLK_SG_IO)) > >>+VIR_DOMAIN_CAPS_ENUM_SET(disk->diskDevice, > >>VIR_DOMAIN_DISK_DEVICE_LUN); > >>+ > >>+VIR_DOMAIN_CAPS_ENUM_SET(disk->bus, > >>+ VIR_DOMAIN_DISK_BUS_IDE, > >>+ VIR_DOMAIN_DISK_BUS_FDC, > >>+ VIR_DOMAIN_DISK_BUS_SCSI, > >>+ VIR_DOMAIN_DISK_BUS_VIRTIO, > >>+ VIR_DOMAIN_DISK_BUS_SD); > > > >I have a feeling that 'SD' is not supported in all QEMU's we claim to > >work with, though perhaps we've never checked this before when > >building the CLI args. > > Well, we don't. I haven't found any code that checks for 'SD' in qemu > driver. I can remove it until the time we have a resolution. > > > > >>+ > >>+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_USB_STORAGE)) > >>+VIR_DOMAIN_CAPS_ENUM_SET(disk->bus, VIR_DOMAIN_DISK_BUS_USB); > >>+} > >>+ > >>+ > > > >>+ > >>+ > > > >Hmm, so that's claiming we don't support any values for > >the mode attribute, but we support subsys. > > Ouch, yeah. The problem is, I was not setting the correct struct member. > Here's the fix: > > index 9fc58ff..6ed85a9 100644 > --- a/src/qemu/qemu_capabilities.c > +++ b/src/qemu/qemu_capabilities.c > @@ -3603,7 +3603,7 @@ virQEMUCapsFillDomainDeviceHostdevCaps(virQEMUCapsPtr > qemuCaps, > > hostdev->device.supported = true; > /* VIR_DOMAIN_HOSTDEV_MODE_CAPABILITIES is for containers only */ > -VIR_DOMAIN_CAPS_ENUM_SET(hostdev->subsysType, > +VIR_DOMAIN_CAPS_ENUM_SET(hostdev->mode, > VIR_DOMAIN_HOSTDEV_MODE_SUBSYS); > > VIR_DOMAIN_CAPS_ENUM_SET(hostdev->startupPolicy, > diff --git a/tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml > b/tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml > index 562e2f4..b7d9c26 100644 > --- a/tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml > +++ b/tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml > @@ -21,7 +21,9 @@ > > > > - > + > +subsystem > + > > default > mandatory ACK 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 1/6] util: storage: Add helper to determine whether storage is local
There's a lot of places where we skip doing actions based on the locality of given storage type. The usual pattern is to skip it if: virStorageSourceGetActualType(src) == VIR_STORAGE_TYPE_NETWORK Add a simple helper to simplify the pattern to virStorageSourceIsLocalStorage(src) --- src/libvirt_private.syms | 1 + src/util/virstoragefile.c | 7 +++ src/util/virstoragefile.h | 1 + 3 files changed, 9 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index ed56103..067dcad 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1910,6 +1910,7 @@ virStorageSourceClear; virStorageSourceFree; virStorageSourceGetActualType; virStorageSourceGetSecurityLabelDef; +virStorageSourceIsLocalStorage; virStorageSourceNewFromBacking; virStorageSourcePoolDefFree; virStorageSourcePoolModeTypeFromString; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 0c50de1..7ab640f 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1550,6 +1550,13 @@ virStorageSourceGetActualType(virStorageSourcePtr def) } +bool +virStorageSourceIsLocalStorage(virStorageSourcePtr src) +{ +return virStorageSourceGetActualType(src) != VIR_STORAGE_TYPE_NETWORK; +} + + /** * virStorageSourceBackingStoreClear: * diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 48c7e02..6609023 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -320,6 +320,7 @@ void virStorageSourceAuthClear(virStorageSourcePtr def); void virStorageSourcePoolDefFree(virStorageSourcePoolDefPtr def); void virStorageSourceClear(virStorageSourcePtr def); int virStorageSourceGetActualType(virStorageSourcePtr def); +bool virStorageSourceIsLocalStorage(virStorageSourcePtr src); void virStorageSourceFree(virStorageSourcePtr def); void virStorageSourceBackingStoreClear(virStorageSourcePtr def); virStorageSourcePtr virStorageSourceNewFromBacking(virStorageSourcePtr parent); -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/6] conf: audit: Split out common steps to audit domain devices
Extract common operations done when creating an audit message to a separate generic function that can be reused and convert RNG, disk, FS and net audit to use it. --- src/conf/domain_audit.c | 175 1 file changed, 57 insertions(+), 118 deletions(-) diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index 91095b1..4c4290c 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -93,46 +93,73 @@ virDomainAuditChardevPath(virDomainChrSourceDefPtr chr) } -void -virDomainAuditDisk(virDomainObjPtr vm, - const char *oldDef, const char *newDef, - const char *reason, bool success) +static void +virDomainAuditGenericDev(virDomainObjPtr vm, + const char *type, + const char *oldsrcpath, + const char *newsrcpath, + const char *reason, + bool success) { +char *newdev = NULL; +char *olddev = NULL; char uuidstr[VIR_UUID_STRING_BUFLEN]; char *vmname; char *oldsrc = NULL; char *newsrc = NULL; const char *virt; -virUUIDFormat(vm->def->uuid, uuidstr); -if (!(vmname = virAuditEncode("vm", vm->def->name))) { -VIR_WARN("OOM while encoding audit message"); +/* if both new and old source aren't provided don't log anything */ +if (!newsrcpath && !oldsrcpath) return; -} + +if (virAsprintfQuiet(&newdev, "new-%s", type) < 0) +goto no_memory; + +if (virAsprintfQuiet(&olddev, "old-%s", type) < 0) +goto no_memory; + +virUUIDFormat(vm->def->uuid, uuidstr); + +if (!(vmname = virAuditEncode("vm", vm->def->name))) +goto no_memory; if (!(virt = virDomainVirtTypeToString(vm->def->virtType))) { -VIR_WARN("Unexpected virt type %d while encoding audit message", vm->def->virtType); +VIR_WARN("Unexpected virt type %d while encoding audit message", + vm->def->virtType); virt = "?"; } -if (!(oldsrc = virAuditEncode("old-disk", VIR_AUDIT_STR(oldDef { -VIR_WARN("OOM while encoding audit message"); -goto cleanup; -} -if (!(newsrc = virAuditEncode("new-disk", VIR_AUDIT_STR(newDef { -VIR_WARN("OOM while encoding audit message"); -goto cleanup; -} +if (!(newsrc = virAuditEncode(newdev, VIR_AUDIT_STR(newsrcpath +goto no_memory; + +if (!(oldsrc = virAuditEncode(olddev, VIR_AUDIT_STR(oldsrcpath +goto no_memory; VIR_AUDIT(VIR_AUDIT_RECORD_RESOURCE, success, - "virt=%s resrc=disk reason=%s %s uuid=%s %s %s", - virt, reason, vmname, uuidstr, - oldsrc, newsrc); + "virt=%s resrc=%s reason=%s %s uuid=%s %s %s", + virt, type, reason, vmname, uuidstr, oldsrc, newsrc); cleanup: +VIR_FREE(newdev); +VIR_FREE(olddev); VIR_FREE(vmname); VIR_FREE(oldsrc); VIR_FREE(newsrc); +return; + + no_memory: +VIR_WARN("OOM while encoding audit message"); +goto cleanup; +} + + +void +virDomainAuditDisk(virDomainObjPtr vm, + const char *oldDef, const char *newDef, + const char *reason, bool success) +{ +virDomainAuditGenericDev(vm, "disk", oldDef, newDef, reason, success); } @@ -141,13 +168,8 @@ virDomainAuditRNG(virDomainObjPtr vm, virDomainRNGDefPtr oldDef, virDomainRNGDefPtr newDef, const char *reason, bool success) { -char uuidstr[VIR_UUID_STRING_BUFLEN]; -char *vmname; const char *newsrcpath = NULL; const char *oldsrcpath = NULL; -char *oldsrc = NULL; -char *newsrc = NULL; -const char *virt; if (newDef) { switch ((virDomainRNGBackend) newDef->backend) { @@ -185,40 +207,7 @@ virDomainAuditRNG(virDomainObjPtr vm, } } -/* don't audit the RNG device if it doesn't use local resources */ -if (!oldsrcpath && !newsrcpath) -return; - -virUUIDFormat(vm->def->uuid, uuidstr); -if (!(vmname = virAuditEncode("vm", vm->def->name))) -goto no_memory; - -if (!(virt = virDomainVirtTypeToString(vm->def->virtType))) { -VIR_WARN("Unexpected virt type %d while encoding audit message", - vm->def->virtType); -virt = "?"; -} - -if (!(newsrc = virAuditEncode("new-rng", VIR_AUDIT_STR(newsrcpath -goto no_memory; - -if (!(oldsrc = virAuditEncode("old-rng", VIR_AUDIT_STR(oldsrcpath -goto no_memory; - -VIR_AUDIT(VIR_AUDIT_RECORD_RESOURCE, success, - "virt=%s resrc=rng reason=%s %s uuid=%s %s %s", - virt, reason, vmname, uuidstr, - oldsrc, newsrc); - - cleanup: -VIR_FREE(vmname); -VIR_FREE(oldsrc); -VIR_FREE(newsrc); -return; - - no_memory: -VIR_WARN("OOM while encoding audit message"); -goto cleanup; +virDomainAudit
[libvirt] [PATCH 0/6] audit: Improve disk auditing and add auditing of new device types
Peter Krempa (6): util: storage: Add helper to determine whether storage is local conf: audit: rng: Reorder new and old RNG device definitions conf: audit: Split out common steps to audit domain devices audit: disk: Refactor disk auditing to avoid auditing remote storage audit: Add auditing for serial/parallel/channel/console characted devs audit: Audit smartcard devices src/conf/domain_audit.c | 281 +- src/conf/domain_audit.h | 11 +- src/libvirt_private.syms | 2 + src/lxc/lxc_driver.c | 6 +- src/qemu/qemu_driver.c| 4 +- src/qemu/qemu_hotplug.c | 38 +++ src/util/virstoragefile.c | 7 ++ src/util/virstoragefile.h | 1 + 8 files changed, 198 insertions(+), 152 deletions(-) -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/6] audit: disk: Refactor disk auditing to avoid auditing remote storage
Pass the virStorageSource struct to the auditing function and check if storage is local before auditting. --- src/conf/domain_audit.c | 25 - src/conf/domain_audit.h | 4 ++-- src/lxc/lxc_driver.c| 6 +++--- src/qemu/qemu_driver.c | 4 ++-- src/qemu/qemu_hotplug.c | 21 - 5 files changed, 31 insertions(+), 29 deletions(-) diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index 4c4290c..c4dcfa5 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -156,10 +156,21 @@ virDomainAuditGenericDev(virDomainObjPtr vm, void virDomainAuditDisk(virDomainObjPtr vm, - const char *oldDef, const char *newDef, - const char *reason, bool success) + virStorageSourcePtr oldDef, + virStorageSourcePtr newDef, + const char *reason, + bool success) { -virDomainAuditGenericDev(vm, "disk", oldDef, newDef, reason, success); +const char *oldsrc = NULL; +const char *newsrc = NULL; + +if (oldDef && virStorageSourceIsLocalStorage(oldDef)) +oldsrc = oldDef->path; + +if (newDef && virStorageSourceIsLocalStorage(newDef)) +newsrc = newDef->path; + +virDomainAuditGenericDev(vm, "disk", oldsrc, newsrc, reason, success); } @@ -738,12 +749,8 @@ virDomainAuditStart(virDomainObjPtr vm, const char *reason, bool success) { size_t i; -for (i = 0; i < vm->def->ndisks; i++) { -const char *src = virDomainDiskGetSource(vm->def->disks[i]); - -if (src) /* Skips CDROM without media initially inserted */ -virDomainAuditDisk(vm, NULL, src, "start", true); -} +for (i = 0; i < vm->def->ndisks; i++) +virDomainAuditDisk(vm, NULL, vm->def->disks[i]->src, "start", true); for (i = 0; i < vm->def->nfss; i++) { virDomainFSDefPtr fs = vm->def->fss[i]; diff --git a/src/conf/domain_audit.h b/src/conf/domain_audit.h index 70b09e5..58d25a4 100644 --- a/src/conf/domain_audit.h +++ b/src/conf/domain_audit.h @@ -39,8 +39,8 @@ void virDomainAuditStop(virDomainObjPtr vm, const char *reason) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); void virDomainAuditDisk(virDomainObjPtr vm, -const char *oldDef, -const char *newDef, +virStorageSourcePtr oldDef, +virStorageSourcePtr newDef, const char *reason, bool success) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4); diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index a9a87ea..251817d 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -4120,7 +4120,7 @@ lxcDomainAttachDeviceDiskLive(virLXCDriverPtr driver, cleanup: if (src) -virDomainAuditDisk(vm, NULL, src, "attach", ret == 0); +virDomainAuditDisk(vm, NULL, def->src, "attach", ret == 0); VIR_FREE(file); return ret; } @@ -4608,10 +4608,10 @@ lxcDomainDetachDeviceDiskLive(virDomainObjPtr vm, } if (lxcDomainAttachDeviceUnlink(vm, dst) < 0) { -virDomainAuditDisk(vm, src, NULL, "detach", false); +virDomainAuditDisk(vm, def->src, NULL, "detach", false); goto cleanup; } -virDomainAuditDisk(vm, src, NULL, "detach", true); +virDomainAuditDisk(vm, def->src, NULL, "detach", true); if (virCgroupDenyDevicePath(priv->cgroup, src, VIR_CGROUP_DEVICE_RWM) != 0) VIR_WARN("cannot deny device %s for domain %s", diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d45a161..b39c405 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12969,7 +12969,7 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, } } -virDomainAuditDisk(vm, disk->src->path, source, "snapshot", ret >= 0); +virDomainAuditDisk(vm, disk->src, snap->src, "snapshot", ret >= 0); if (ret < 0) goto cleanup; @@ -15400,7 +15400,7 @@ qemuDomainBlockCopy(virDomainObjPtr vm, qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorDriveMirror(priv->mon, device, dest, format, bandwidth, flags); -virDomainAuditDisk(vm, NULL, dest, "mirror", ret >= 0); +virDomainAuditDisk(vm, NULL, mirror, "mirror", ret >= 0); qemuDomainObjExitMonitor(driver, vm); if (ret < 0) { qemuDomainPrepareDiskChainElementPath(driver, vm, disk, dest, diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 5e8aa4e..8d37813 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -154,9 +154,7 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, qemuDomainObjExitMonitor(driver, vm); } audit: -if (src) -virDomainAuditDisk(vm, virDomainDiskGetSource(origdisk), - src, "update", ret >= 0); +virDomainAuditDisk(vm, origdisk->src, disk->src, "update
[libvirt] [PATCH 2/6] conf: audit: rng: Reorder new and old RNG device definitions
The audit functions usually take the old definition before the new one in the argument list. Unify RNG device to use the same order. --- src/conf/domain_audit.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index 52c643d..91095b1 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -138,7 +138,7 @@ virDomainAuditDisk(virDomainObjPtr vm, static void virDomainAuditRNG(virDomainObjPtr vm, - virDomainRNGDefPtr newDef, virDomainRNGDefPtr oldDef, + virDomainRNGDefPtr oldDef, virDomainRNGDefPtr newDef, const char *reason, bool success) { char uuidstr[VIR_UUID_STRING_BUFLEN]; @@ -827,7 +827,7 @@ virDomainAuditStart(virDomainObjPtr vm, const char *reason, bool success) } if (vm->def->rng) -virDomainAuditRNG(vm, vm->def->rng, NULL, "start", true); +virDomainAuditRNG(vm, NULL, vm->def->rng, "start", true); if (vm->def->tpm) virDomainAuditTPM(vm, vm->def->tpm, "start", true); -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/6] audit: Add auditing for serial/parallel/channel/console characted devs
Add startup auditing and also hotplug auditing for said devices --- src/conf/domain_audit.c | 35 +++ src/conf/domain_audit.h | 7 +++ src/libvirt_private.syms | 1 + src/qemu/qemu_hotplug.c | 17 +++-- 4 files changed, 54 insertions(+), 6 deletions(-) diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index c4dcfa5..b7f8123 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -155,6 +155,29 @@ virDomainAuditGenericDev(virDomainObjPtr vm, void +virDomainAuditChardev(virDomainObjPtr vm, + virDomainChrDefPtr oldDef, + virDomainChrDefPtr newDef, + const char *reason, + bool success) +{ +virDomainChrSourceDefPtr oldsrc = NULL; +virDomainChrSourceDefPtr newsrc = NULL; + +if (oldDef) +oldsrc = &oldDef->source; + +if (newDef) +newsrc = &newDef->source; + +virDomainAuditGenericDev(vm, "chardev", + virDomainAuditChardevPath(oldsrc), + virDomainAuditChardevPath(newsrc), + reason, success); +} + + +void virDomainAuditDisk(virDomainObjPtr vm, virStorageSourcePtr oldDef, virStorageSourcePtr newDef, @@ -772,6 +795,18 @@ virDomainAuditStart(virDomainObjPtr vm, const char *reason, bool success) virDomainAuditRedirdev(vm, redirdev, "start", true); } +for (i = 0; i < vm->def->nserials; i++) +virDomainAuditChardev(vm, NULL, vm->def->serials[i], "start", true); + +for (i = 0; i < vm->def->nparallels; i++) +virDomainAuditChardev(vm, NULL, vm->def->parallels[i], "start", true); + +for (i = 0; i < vm->def->nchannels; i++) +virDomainAuditChardev(vm, NULL, vm->def->channels[i], "start", true); + +for (i = 0; i < vm->def->nconsoles; i++) +virDomainAuditChardev(vm, NULL, vm->def->consoles[i], "start", true); + if (vm->def->rng) virDomainAuditRNG(vm, NULL, vm->def->rng, "start", true); diff --git a/src/conf/domain_audit.h b/src/conf/domain_audit.h index 58d25a4..3434feb 100644 --- a/src/conf/domain_audit.h +++ b/src/conf/domain_audit.h @@ -111,4 +111,11 @@ void virDomainAuditRedirdev(virDomainObjPtr vm, bool success) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); +void virDomainAuditChardev(virDomainObjPtr vm, + virDomainChrDefPtr oldDef, + virDomainChrDefPtr newDef, + const char *reason, + bool success) +ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4); + #endif /* __VIR_DOMAIN_AUDIT_H__ */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 067dcad..b04b099 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -116,6 +116,7 @@ virDomainPCIAddressValidate; virDomainAuditCgroup; virDomainAuditCgroupMajor; virDomainAuditCgroupPath; +virDomainAuditChardev; virDomainAuditDisk; virDomainAuditFS; virDomainAuditHostdev; diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 8d37813..5451118 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1460,18 +1460,20 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver, qemuDomainObjEnterMonitor(driver, vm); if (qemuMonitorAttachCharDev(priv->mon, charAlias, &chr->source) < 0) { qemuDomainObjExitMonitor(driver, vm); -goto cleanup; +goto audit; } if (devstr && qemuMonitorAddDevice(priv->mon, devstr) < 0) { /* detach associated chardev on error */ qemuMonitorDetachCharDev(priv->mon, charAlias); qemuDomainObjExitMonitor(driver, vm); -goto cleanup; +goto audit; } qemuDomainObjExitMonitor(driver, vm); ret = 0; + audit: +virDomainAuditChardev(vm, NULL, chr, "attach", ret == 0); cleanup: if (ret < 0 && need_remove) qemuDomainChrRemove(vmdef, chr); @@ -2751,6 +2753,7 @@ qemuDomainRemoveChrDevice(virQEMUDriverPtr driver, char *charAlias = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; int ret = -1; +int rc; VIR_DEBUG("Removing character device %s from domain %p %s", chr->info.alias, vm, vm->def->name); @@ -2759,12 +2762,14 @@ qemuDomainRemoveChrDevice(virQEMUDriverPtr driver, goto cleanup; qemuDomainObjEnterMonitor(driver, vm); -if (qemuMonitorDetachCharDev(priv->mon, charAlias) < 0) { -qemuDomainObjExitMonitor(driver, vm); -goto cleanup; -} +rc = qemuMonitorDetachCharDev(priv->mon, charAlias); qemuDomainObjExitMonitor(driver, vm); +virDomainAuditChardev(vm, chr, NULL, "detach", rc == 0); + +if (rc < 0) +goto cleanup; + event = virDomainEventDeviceRemovedNewFromObj(vm, chr->info.alias); if (event) qemuDomainEventQueue(drive
[libvirt] [PATCH 6/6] audit: Audit smartcard devices
--- src/conf/domain_audit.c | 48 1 file changed, 48 insertions(+) diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index b7f8123..a906d88 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -177,6 +177,51 @@ virDomainAuditChardev(virDomainObjPtr vm, } +static void +virDomainAuditSmartcard(virDomainObjPtr vm, +virDomainSmartcardDefPtr def, +const char *reason, +bool success) +{ +const char *database = VIR_DOMAIN_SMARTCARD_DEFAULT_DATABASE; +size_t i; + +if (def) { +switch ((virDomainSmartcardType) def->type) { +case VIR_DOMAIN_SMARTCARD_TYPE_HOST: +virDomainAuditGenericDev(vm, "smartcard", + NULL, "nss-smartcard-device", + reason, success); +break; + +case VIR_DOMAIN_SMARTCARD_TYPE_HOST_CERTIFICATES: +for (i = 0; i < VIR_DOMAIN_SMARTCARD_NUM_CERTIFICATES; i++) { +virDomainAuditGenericDev(vm, "smartcard", NULL, + def->data.cert.file[i], + reason, success); +} + +if (def->data.cert.database) +database = def->data.cert.database; + +virDomainAuditGenericDev(vm, "smartcard", + NULL, database, + reason, success); +break; + +case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH: +virDomainAuditGenericDev(vm, "smartcard", NULL, + virDomainAuditChardevPath(&def->data.passthru), + reason, success); +break; + +case VIR_DOMAIN_SMARTCARD_TYPE_LAST: +break; +} +} +} + + void virDomainAuditDisk(virDomainObjPtr vm, virStorageSourcePtr oldDef, @@ -807,6 +852,9 @@ virDomainAuditStart(virDomainObjPtr vm, const char *reason, bool success) for (i = 0; i < vm->def->nconsoles; i++) virDomainAuditChardev(vm, NULL, vm->def->consoles[i], "start", true); +for (i = 0; i < vm->def->nsmartcards; i++) +virDomainAuditSmartcard(vm, vm->def->smartcards[i], "start", true); + if (vm->def->rng) virDomainAuditRNG(vm, NULL, vm->def->rng, "start", true); -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] xenapiConnectGetCapabilities: Remove unused 'cleanup' label
In the lastest rework (9e7ecabf) a cleanup label was left over which results in compilation error. Signed-off-by: Michal Privoznik --- Pushed under build-breaker rule. src/xenapi/xenapi_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c index e3bb77e..316e274 100644 --- a/src/xenapi/xenapi_driver.c +++ b/src/xenapi/xenapi_driver.c @@ -447,7 +447,7 @@ xenapiConnectGetCapabilities(virConnectPtr conn) virCapsPtr caps = ((struct _xenapiPrivate *)(conn->privateData))->caps; if (caps) return virCapabilitiesFormatXML(caps); - cleanup: + xenapiSessionErrorHandler(conn, VIR_ERR_INTERNAL_ERROR, _("Capabilities not available")); return NULL; -- 1.8.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] new openvz driver (bossonvz)
Fair enough. We have no problem to adapt to the new process. I don't know how libcontainer is designed internally and how low-level it is but having a thin user-space wrapper around the vzkernel would be wonderful. I wanted to use libvzctl in bossonvz to abstract the driver away from the low-level stuff but the library is just too much interconnected with the veconf parser. The current vz kernel API is not the nicest thing to work with to be honest and a shared user-space library would help both vzctl and bossonvz. If you are interested in my POV on the kernel API, I would say you should leave more work for the user-space. At the moment, when the user-space asks for a new container, the vzkernel magically creates a new environment and sets everything (esp. cgroups) by itself without any chance for the user-space to set anything. Because libvirt has its own cgroup hierarchy model and any libvirt driver should honor it, bossonvz should, idealy, create cgroups for the container under the libvirt hierarchy but it cannot since the kernel won't let it. As I see it, the driver has to do two separate things to successfully use the kernel API. There are the vz specific syscalls/ioctls (e.g. VZCTL_ENV_CREATE_DATA) and there is a preparatory work done in the user-space (preparing forked sub-processes, pipes, etc.). Is the new SDK going to cover only the low-level code (ioctls) or will I be able to use some sort of a high-level function like spawn_environment() and get PID of the newly created container's init? The amount of vz specific code in the bossonvz driver is not that big. The ioctls is just a single bossonvz_environment.c file. The harder part is the preparatory part which is located in bossonvz_container.c and is quite complex since it closely mimics the code path of vzctl (otherwise the driver would not work). -- David Fabian Cluster Design, s.r.o. Dne St 2. července 2014 19:41:15, Pavel Emelyanov napsal(a): > On 07/02/2014 04:15 PM, Bosson VZ wrote: > > Thanks in return for the acknowledgement. > > > > > > > > I understand that the long-term plan is to reduce the size of the vzkernel > > patch and to > > only use vanilla kernel primitives Daniel was talking about. That would be > > an ideal situation. > > Yes, at the kernel level the long term plan is to eventually > get rid of vz-specific API and switch to namespaces/cgroups > completely. Thus any new users that sit on the vzkernel API > make this process ... harder :) > > > The thing is that we simply cannot wait for this merge to happen. We have > > been using OpenVZ > > for many years and bossonvz is just a convenient tool for us right now and, > > at least, in the > > mid-term future. My question is how is the new libcontainer/PCS SDK library > > going to influence > > the existing vz-tools eco-system? > > We will keep the CLI (vzctl) and library (libvzctl) working on > new OpenVZ installations for some time (several years, I suppose) > by making them properly redirect calls to new API. > > But all the new features will be implemented within the new API, > libcontainer/PCS SDK. > > Thanks, > Pavel > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH]util:ovs:Delete port if it is exist when add port
If the ovs service stop abnormal, or host cold reboot, vm is destroyed after ovs service stop. The ovs port which connect to interface of vm will not be clear. When the ovs service restart, recover configuration from db, but the interface is no exist, port recovery failed, and then vm restart on the same host, libvirt add port again, but the port configuration is same as before, ovs will not connect the interface, only store the configuration in db. Below will trigger this problem, virsh start vm service openvswitch-switch stop virsh destroy vm service openvswitch-switch start virsh start vm Signed-off-by: Chunhe Li --- src/util/virnetdevopenvswitch.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index 9bcbfb1..dd601c0 100644 --- a/src/util/virnetdevopenvswitch.c +++ b/src/util/virnetdevopenvswitch.c @@ -84,7 +84,8 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname, cmd = virCommandNew(OVSVSCTL); -virCommandAddArgList(cmd, "--timeout=5", "--", "--may-exist", "add-port", +virCommandAddArgList(cmd, "--timeout=5", "--", "--if-exists", "del-port", +ifname, "--", "--may-exist", "add-port", brname, ifname, NULL); if (virtVlan && virtVlan->nTags > 0) { -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC]KVM VMs stay in pause state too long in finish phase during migration
Hi, I started a VM on KVM environment(libvirt1.2.6 qemu1.5.1). I found that the startup thread keeps vm lock too long. And this would cause other VMs paused(both on src and dest) too long during migration. Steps to Reproduce: 1. Define and start three VMs(VMA, VMB, VMC) on source host with 16 NICs for each. XML configuration for NIC: 2. Migrate the three VMs from source host to destination host concurrently. 3. On destination host, the three VMs may do the following operation: 1) VMA: do qemuProcessStart(get VM lock until end) -> .. -> virNetDevTapCreateInBridgePort(). This function costs time 0.28s per NIC, and 16 NICs costs about 4s. The following log shows time cost for creating NICs on my host. And it seems that the time of creating NICs is different between hosts. 2014-07-03 08:40:41.283+: 47007: info : remoteDispatchAuthList:2781 : Bypass polkit auth for privileged client pid:47635,uid:0 2014-07-03 08:40:41.285+: 47009: info : virNetDevProbeVnetHdr:203 : Enabling IFF_VNET_HDR 2014-07-03 08:40:41.560+: 47009: info : virNetDevProbeVnetHdr:203 : Enabling IFF_VNET_HDR 2014-07-03 08:40:41.852+: 47009: info : virNetDevProbeVnetHdr:203 : Enabling IFF_VNET_HDR 2014-07-03 08:40:42.144+: 47009: info : virNetDevProbeVnetHdr:203 : Enabling IFF_VNET_HDR 2014-07-03 08:40:42.464+: 47009: info : virNetDevProbeVnetHdr:203 : Enabling IFF_VNET_HDR 2014-07-03 08:40:42.756+: 47009: info : virNetDevProbeVnetHdr:203 : Enabling IFF_VNET_HDR 2014-07-03 08:40:43.076+: 47009: info : virNetDevProbeVnetHdr:203 : Enabling IFF_VNET_HDR 2014-07-03 08:40:43.372+: 47009: info : virNetDevProbeVnetHdr:203 : Enabling IFF_VNET_HDR 2014-07-03 08:40:43.680+: 47009: info : virNetDevProbeVnetHdr:203 : Enabling IFF_VNET_HDR 2014-07-03 08:40:43.972+: 47009: info : virNetDevProbeVnetHdr:203 : Enabling IFF_VNET_HDR 2014-07-03 08:40:44.268+: 47009: info : virNetDevProbeVnetHdr:203 : Enabling IFF_VNET_HDR 2014-07-03 08:40:44.560+: 47009: info : virNetDevProbeVnetHdr:203 : Enabling IFF_VNET_HDR 2014-07-03 08:40:44.720+: 47009: info : virNetDevProbeVnetHdr:203 : Enabling IFF_VNET_HDR 2014-07-03 08:40:44.804+: 47009: info : virNetDevProbeVnetHdr:203 : Enabling IFF_VNET_HDR 2014-07-03 08:40:44.888+: 47009: info : virNetDevProbeVnetHdr:203 : Enabling IFF_VNET_HDR 2014-07-03 08:40:45.040+: 47009: info : virNetDevProbeVnetHdr:203 : Enabling IFF_VNET_HDR 2014-07-03 08:40:45.369+: 47009: warning : qemuDomainObjTaint:1670 : Domain id=7 name='suse11sp3_test_7' uuid=94c4ac0b-3a6a-41aa-b16c-80aa7adbc6b8 is tainted: high-privileges 2014-07-03 08:40:45.708+: 47009: info : virSecurityDACSetOwnership:227 : Setting DAC user and group on '/home/wcj/DTS/suse11sp3_test_7' to '0:0' 2) VMB: do qemuMigrationPrepareAny() -> virDomainObjListAdd(). This function acquires driver->doms lock. And then virHashSearch() waits for VMA's vm lock which is hold by VMA's qemuProcessStart() thread. 3) VMC: do qemuDomainMigrateFinish3 -> virDomainObjListFindByName. This operation waits for driver->doms lock which is hold by VMB's qemuMigrationPrepareAny() thread. As VMC is in finish phase, it's in pause state on both source host and destination host. In the worst case, VMC may stay in pause state for about 4s during migration. And the pause time increased if we migrate more VMs concurrently. VirNetDevTapCreateInBridgePort() which holds vm lock costs a little long time. IMHO it would be nice to do some lock optimization for this case. (I think it makes little sense to optimize the time of creating net device. Because even though the time is reduced to 0.1s, if vm has 16 NICs, the vm lock will be held for more than 1.6s.) Any ideas? -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] tests: Avoid double linking some libraries
The problem is, since 614581f32b domaincapstest is linked with $(LDADDS) by default. Then, since 94e3f23e8a7 the test may be conditionally linked with $(qemu_LDADDS) which already contains $(LDADDS). And some linkers doesn't cope with this nicely: CCLD domaincapstest ../src/libvirt_probes.o:(.probes+0x0): multiple definition of `libvirt_event_poll_add_handle_semaphore' ../src/libvirt_probes.o:(.probes+0x0): first defined here ../src/libvirt_probes.o:(.probes+0x2): multiple definition of `libvirt_event_poll_update_handle_semaphore' ../src/libvirt_probes.o:(.probes+0x2): first defined here ../src/libvirt_probes.o:(.probes+0x4): multiple definition of `libvirt_event_poll_remove_handle_semaphore' ../src/libvirt_probes.o:(.probes+0x4): first defined here ../src/libvirt_probes.o:(.probes+0x6): multiple definition of `libvirt_event_poll_dispatch_handle_semaphore' ../src/libvirt_probes.o:(.probes+0x6): first defined here And so on. Signed-off-by: Michal Privoznik --- I'd push this as build breaker, but there's another approach to consider, create qemu_BARE_LDADDS and fill it with the qemu_impl, etc. Then, let: qemu_LDADDS = $(qemu_BARE_LDADDS) $(LDADDS) What's your preference? tests/Makefile.am | 29 +++-- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index a262c7b..b9a6345 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -509,12 +509,11 @@ endif WITH_STORAGE if WITH_DTRACE_PROBES qemu_LDADDS += ../src/libvirt_qemu_probes.lo endif WITH_DTRACE_PROBES -qemu_LDADDS += $(LDADDS) qemuxml2argvtest_SOURCES = \ qemuxml2argvtest.c testutilsqemu.c testutilsqemu.h \ testutils.c testutils.h -qemuxml2argvtest_LDADD = $(qemu_LDADDS) $(LIBXML_LIBS) +qemuxml2argvtest_LDADD = $(qemu_LDADDS) $(LDADDS) $(LIBXML_LIBS) qemuxml2argvmock_la_SOURCES = \ qemuxml2argvmock.c @@ -525,62 +524,64 @@ qemuxml2argvmock_la_LDFLAGS = -module -avoid-version \ qemuxml2xmltest_SOURCES = \ qemuxml2xmltest.c testutilsqemu.c testutilsqemu.h \ testutils.c testutils.h -qemuxml2xmltest_LDADD = $(qemu_LDADDS) +qemuxml2xmltest_LDADD = $(qemu_LDADDS) $(LDADDS) qemuxmlnstest_SOURCES = \ qemuxmlnstest.c testutilsqemu.c testutilsqemu.h \ testutils.c testutils.h -qemuxmlnstest_LDADD = $(qemu_LDADDS) +qemuxmlnstest_LDADD = $(qemu_LDADDS) $(LDADDS) qemuargv2xmltest_SOURCES = \ qemuargv2xmltest.c testutilsqemu.c testutilsqemu.h \ testutils.c testutils.h -qemuargv2xmltest_LDADD = $(qemu_LDADDS) +qemuargv2xmltest_LDADD = $(qemu_LDADDS) $(LDADDS) qemuhelptest_SOURCES = qemuhelptest.c testutils.c testutils.h -qemuhelptest_LDADD = $(qemu_LDADDS) +qemuhelptest_LDADD = $(qemu_LDADDS) $(LDADDS) qemumonitortest_SOURCES = qemumonitortest.c testutils.c testutils.h -qemumonitortest_LDADD = $(qemu_LDADDS) +qemumonitortest_LDADD = $(qemu_LDADDS) $(LDADDS) qemumonitorjsontest_SOURCES = \ qemumonitorjsontest.c \ testutils.c testutils.h \ testutilsqemu.c testutilsqemu.h \ $(NULL) -qemumonitorjsontest_LDADD = libqemumonitortestutils.la $(qemu_LDADDS) +qemumonitorjsontest_LDADD = libqemumonitortestutils.la \ + $(qemu_LDADDS) $(LDADDS) qemucapabilitiestest_SOURCES = \ qemucapabilitiestest.c \ testutils.c testutils.h \ testutilsqemu.c testutilsqemu.h \ $(NULL) -qemucapabilitiestest_LDADD = libqemumonitortestutils.la $(qemu_LDADDS) +qemucapabilitiestest_LDADD = libqemumonitortestutils.la \ + $(qemu_LDADDS) $(LDADDS) qemucaps2xmltest_SOURCES = \ qemucaps2xmltest.c \ testutils.c testutils.h \ $(NULL) -qemucaps2xmltest_LDADD = $(qemu_LDADDS) +qemucaps2xmltest_LDADD = $(qemu_LDADDS) $(LDADDS) qemuagenttest_SOURCES = \ qemuagenttest.c \ testutils.c testutils.h \ testutilsqemu.c testutilsqemu.h \ $(NULL) -qemuagenttest_LDADD = libqemumonitortestutils.la $(qemu_LDADDS) +qemuagenttest_LDADD = libqemumonitortestutils.la $(qemu_LDADDS) $(LDADDS) qemuhotplugtest_SOURCES = \ qemuhotplugtest.c \ testutils.c testutils.h \ testutilsqemu.c testutilsqemu.h \ $(NULL) -qemuhotplugtest_LDADD = libqemumonitortestutils.la $(qemu_LDADDS) +qemuhotplugtest_LDADD = libqemumonitortestutils.la $(qemu_LDADDS) $(LDADDS) domainsnapshotxml2xmltest_SOURCES = \ domainsnapshotxml2xmltest.c testutilsqemu.c testutilsqemu.h \ testutils.c testutils.h -domainsnapshotxml2xmltest_LDADD = $(qemu_LDADDS) +domainsnapshotxml2xmltest_LDADD = $(qemu_LDADDS) $(LDADDS) else ! WITH_QEMU EXTRA_DIST += qemuxml2argvtest.c qemuxml2xmltest.c qemuargv2xmltest.c \ qemuxmlnstest.c qemuhelptest.c domainsnapshotxml2xmltest.c \ @@ -837,7 +838,7 @@ domaincapstest_LDADD = $(LDADDS) if WITH_QEMU domaincapstest_SOURCES += testutilsqemu.c testutilsqemu.h -domaincapstest_LDADD += $(qemu_LDADDS) +domaincapstest_LDADD += $(qemu_LDADDS) $(GNULIB_LIBS)
Re: [libvirt] [PATCH]util:ovs:Delete port if it is exist when add port
On 03.07.2014 13:57, Lichunhe wrote: If the ovs service stop abnormal, or host cold reboot, vm is destroyed after ovs service stop. The ovs port which connect to interface of vm will not be clear. When the ovs service restart, recover configuration from db, but the interface is no exist, port recovery failed, and then vm restart on the same host, libvirt add port again, but the port configuration is same as before, ovs will not connect the interface, only store the configuration in db. Below will trigger this problem, virsh start vm service openvswitch-switch stop virsh destroy vm service openvswitch-switch start virsh start vm Signed-off-by: Chunhe Li --- src/util/virnetdevopenvswitch.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index 9bcbfb1..dd601c0 100644 --- a/src/util/virnetdevopenvswitch.c +++ b/src/util/virnetdevopenvswitch.c @@ -84,7 +84,8 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname, cmd = virCommandNew(OVSVSCTL); -virCommandAddArgList(cmd, "--timeout=5", "--", "--may-exist", "add-port", +virCommandAddArgList(cmd, "--timeout=5", "--", "--if-exists", "del-port", +ifname, "--", "--may-exist", "add-port", brname, ifname, NULL); So what's the meaning of '--may-exist' then? Does it do anything useful after all? Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tests: Avoid double linking some libraries
On 07/03/2014 08:34 AM, Michal Privoznik wrote: > The problem is, since 614581f32b domaincapstest is linked with > $(LDADDS) by default. Then, since 94e3f23e8a7 the test may be > conditionally linked with $(qemu_LDADDS) which already contains > $(LDADDS). And some linkers doesn't cope with this nicely: > > CCLD domaincapstest > ../src/libvirt_probes.o:(.probes+0x0): multiple definition of > `libvirt_event_poll_add_handle_semaphore' > ../src/libvirt_probes.o:(.probes+0x0): first defined here > ../src/libvirt_probes.o:(.probes+0x2): multiple definition of > `libvirt_event_poll_update_handle_semaphore' > ../src/libvirt_probes.o:(.probes+0x2): first defined here > ../src/libvirt_probes.o:(.probes+0x4): multiple definition of > `libvirt_event_poll_remove_handle_semaphore' > ../src/libvirt_probes.o:(.probes+0x4): first defined here > ../src/libvirt_probes.o:(.probes+0x6): multiple definition of > `libvirt_event_poll_dispatch_handle_semaphore' > ../src/libvirt_probes.o:(.probes+0x6): first defined here > > And so on. > > Signed-off-by: Michal Privoznik > --- > > I'd push this as build breaker, but there's another approach to consider, > create qemu_BARE_LDADDS and fill it with the qemu_impl, etc. Then, let: > qemu_LDADDS = $(qemu_BARE_LDADDS) $(LDADDS) > > What's your preference? > What is below works for me - so to that degree I'd ACK. However, if someone had a preference for whatever it is you're describing as an option, then I suppose that would be fine too. Makefile's are mostly black magic to me :-) John > tests/Makefile.am | 29 +++-- > 1 file changed, 15 insertions(+), 14 deletions(-) > > diff --git a/tests/Makefile.am b/tests/Makefile.am > index a262c7b..b9a6345 100644 > --- a/tests/Makefile.am > +++ b/tests/Makefile.am > @@ -509,12 +509,11 @@ endif WITH_STORAGE > if WITH_DTRACE_PROBES > qemu_LDADDS += ../src/libvirt_qemu_probes.lo > endif WITH_DTRACE_PROBES > -qemu_LDADDS += $(LDADDS) > > qemuxml2argvtest_SOURCES = \ > qemuxml2argvtest.c testutilsqemu.c testutilsqemu.h \ > testutils.c testutils.h > -qemuxml2argvtest_LDADD = $(qemu_LDADDS) $(LIBXML_LIBS) > +qemuxml2argvtest_LDADD = $(qemu_LDADDS) $(LDADDS) $(LIBXML_LIBS) > > qemuxml2argvmock_la_SOURCES = \ > qemuxml2argvmock.c > @@ -525,62 +524,64 @@ qemuxml2argvmock_la_LDFLAGS = -module -avoid-version \ > qemuxml2xmltest_SOURCES = \ > qemuxml2xmltest.c testutilsqemu.c testutilsqemu.h \ > testutils.c testutils.h > -qemuxml2xmltest_LDADD = $(qemu_LDADDS) > +qemuxml2xmltest_LDADD = $(qemu_LDADDS) $(LDADDS) > > qemuxmlnstest_SOURCES = \ > qemuxmlnstest.c testutilsqemu.c testutilsqemu.h \ > testutils.c testutils.h > -qemuxmlnstest_LDADD = $(qemu_LDADDS) > +qemuxmlnstest_LDADD = $(qemu_LDADDS) $(LDADDS) > > qemuargv2xmltest_SOURCES = \ > qemuargv2xmltest.c testutilsqemu.c testutilsqemu.h \ > testutils.c testutils.h > -qemuargv2xmltest_LDADD = $(qemu_LDADDS) > +qemuargv2xmltest_LDADD = $(qemu_LDADDS) $(LDADDS) > > qemuhelptest_SOURCES = qemuhelptest.c testutils.c testutils.h > -qemuhelptest_LDADD = $(qemu_LDADDS) > +qemuhelptest_LDADD = $(qemu_LDADDS) $(LDADDS) > > qemumonitortest_SOURCES = qemumonitortest.c testutils.c testutils.h > -qemumonitortest_LDADD = $(qemu_LDADDS) > +qemumonitortest_LDADD = $(qemu_LDADDS) $(LDADDS) > > qemumonitorjsontest_SOURCES = \ > qemumonitorjsontest.c \ > testutils.c testutils.h \ > testutilsqemu.c testutilsqemu.h \ > $(NULL) > -qemumonitorjsontest_LDADD = libqemumonitortestutils.la $(qemu_LDADDS) > +qemumonitorjsontest_LDADD = libqemumonitortestutils.la \ > + $(qemu_LDADDS) $(LDADDS) > > qemucapabilitiestest_SOURCES = \ > qemucapabilitiestest.c \ > testutils.c testutils.h \ > testutilsqemu.c testutilsqemu.h \ > $(NULL) > -qemucapabilitiestest_LDADD = libqemumonitortestutils.la $(qemu_LDADDS) > +qemucapabilitiestest_LDADD = libqemumonitortestutils.la \ > + $(qemu_LDADDS) $(LDADDS) > > qemucaps2xmltest_SOURCES = \ > qemucaps2xmltest.c \ > testutils.c testutils.h \ > $(NULL) > -qemucaps2xmltest_LDADD = $(qemu_LDADDS) > +qemucaps2xmltest_LDADD = $(qemu_LDADDS) $(LDADDS) > > qemuagenttest_SOURCES = \ > qemuagenttest.c \ > testutils.c testutils.h \ > testutilsqemu.c testutilsqemu.h \ > $(NULL) > -qemuagenttest_LDADD = libqemumonitortestutils.la $(qemu_LDADDS) > +qemuagenttest_LDADD = libqemumonitortestutils.la $(qemu_LDADDS) $(LDADDS) > > qemuhotplugtest_SOURCES = \ > qemuhotplugtest.c \ > testutils.c testutils.h \ > testutilsqemu.c testutilsqemu.h \ > $(NULL) > -qemuhotplugtest_LDADD = libqemumonitortestutils.la $(qemu_LDADDS) > +qemuhotplugtest_LDADD = libqemumonitortestutils.la $(qemu_LDADDS) $(LDADDS) > > domainsnapshotxml2xmltest_SOURCES = \ > domainsnapshotxml2xmltest.c testutilsqemu.c testutilsqemu.h \ > testutils.c testutils.h > -domainsnapshotxml2xmltest
Re: [libvirt] [PATCH]util:ovs:Delete port if it is exist when add port
Yeah, the '--may-exist' could be removed, it is not useful. >-Original Message- >From: Michal Privoznik [mailto:mpriv...@redhat.com] >Sent: Thursday, July 03, 2014 8:39 PM >To: Lichunhe; libvir-list@redhat.com >Subject: Re: [libvirt] [PATCH]util:ovs:Delete port if it is exist when add port > >On 03.07.2014 13:57, Lichunhe wrote: >> If the ovs service stop abnormal, or host cold reboot, vm is destroyed >> after ovs service stop. The ovs port which connect to interface of vm >> will not be clear. When the ovs service restart, recover configuration >> from db, but the interface is no exist, port recovery failed, and then >> vm restart on the same host, libvirt add port again, but the port >> configuration is same as before, ovs will not connect the interface, >> only store the configuration in db. Below will trigger this problem, >> >> virsh start vm >> service openvswitch-switch stop >> virsh destroy vm >> service openvswitch-switch start >> virsh start vm >> >> >> Signed-off-by: Chunhe Li >> --- >> >> src/util/virnetdevopenvswitch.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/src/util/virnetdevopenvswitch.c >> b/src/util/virnetdevopenvswitch.c index 9bcbfb1..dd601c0 100644 >> --- a/src/util/virnetdevopenvswitch.c >> +++ b/src/util/virnetdevopenvswitch.c >> @@ -84,7 +84,8 @@ int virNetDevOpenvswitchAddPort(const char *brname, >> const char *ifname, >> >> cmd = virCommandNew(OVSVSCTL); >> >> -virCommandAddArgList(cmd, "--timeout=5", "--", "--may-exist", >"add-port", >> +virCommandAddArgList(cmd, "--timeout=5", "--", "--if-exists", >"del-port", >> +ifname, "--", "--may-exist", "add-port", >> brname, ifname, NULL); > >So what's the meaning of '--may-exist' then? Does it do anything useful after >all? > >Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/6] util: storage: Add helper to determine whether storage is local
On 07/03/2014 12:04 PM, Peter Krempa wrote: > There's a lot of places where we skip doing actions based on the > locality of given storage type. The usual pattern is to skip it if: > > virStorageSourceGetActualType(src) == VIR_STORAGE_TYPE_NETWORK > > Add a simple helper to simplify the pattern to > virStorageSourceIsLocalStorage(src) Are you planning to use it anywhere out of those "lot of places"? :) > --- > src/libvirt_private.syms | 1 + > src/util/virstoragefile.c | 7 +++ > src/util/virstoragefile.h | 1 + > 3 files changed, 9 insertions(+) ACK Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/6] audit: Add auditing for serial/parallel/channel/console characted devs
On 07/03/2014 12:05 PM, Peter Krempa wrote: > Add startup auditing and also hotplug auditing for said devices > --- > src/conf/domain_audit.c | 35 +++ > src/conf/domain_audit.h | 7 +++ > src/libvirt_private.syms | 1 + > src/qemu/qemu_hotplug.c | 17 +++-- > 4 files changed, 54 insertions(+), 6 deletions(-) Missing changes in docs/auditlog.html.in > diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c > index c4dcfa5..b7f8123 100644 > --- a/src/conf/domain_audit.c > +++ b/src/conf/domain_audit.c > virDomainAuditDisk(virDomainObjPtr vm, > virStorageSourcePtr oldDef, > virStorageSourcePtr newDef, > @@ -772,6 +795,18 @@ virDomainAuditStart(virDomainObjPtr vm, const char > *reason, bool success) > virDomainAuditRedirdev(vm, redirdev, "start", true); > } > > +for (i = 0; i < vm->def->nserials; i++) > +virDomainAuditChardev(vm, NULL, vm->def->serials[i], "start", true); > + > +for (i = 0; i < vm->def->nparallels; i++) > +virDomainAuditChardev(vm, NULL, vm->def->parallels[i], "start", > true); > + > +for (i = 0; i < vm->def->nchannels; i++) > +virDomainAuditChardev(vm, NULL, vm->def->channels[i], "start", true); > + > +for (i = 0; i < vm->def->nconsoles; i++) > +virDomainAuditChardev(vm, NULL, vm->def->consoles[i], "start", true); > + I wonder if working around the first console aliased to the first serial port (or was it the other way around?) is worth it to prevent logging the same device twice. ACK with the docs added. Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/6] conf: audit: Split out common steps to audit domain devices
On 07/03/2014 12:04 PM, Peter Krempa wrote: > Extract common operations done when creating an audit message to a > separate generic function that can be reused and convert RNG, disk, FS > and net audit to use it. > --- > src/conf/domain_audit.c | 175 > > 1 file changed, 57 insertions(+), 118 deletions(-) > > diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c > index 91095b1..4c4290c 100644 > --- a/src/conf/domain_audit.c > +++ b/src/conf/domain_audit.c > @@ -93,46 +93,73 @@ virDomainAuditChardevPath(virDomainChrSourceDefPtr chr) > } > > > -void > -virDomainAuditDisk(virDomainObjPtr vm, > - const char *oldDef, const char *newDef, > - const char *reason, bool success) > +static void > +virDomainAuditGenericDev(virDomainObjPtr vm, > + const char *type, > + const char *oldsrcpath, > + const char *newsrcpath, > + const char *reason, > + bool success) > { > +char *newdev = NULL; > +char *olddev = NULL; > char uuidstr[VIR_UUID_STRING_BUFLEN]; > char *vmname; vmname can be used unitialized in the cleanup section on OOM > char *oldsrc = NULL; > char *newsrc = NULL; > const char *virt; > > -virUUIDFormat(vm->def->uuid, uuidstr); > -if (!(vmname = virAuditEncode("vm", vm->def->name))) { > -VIR_WARN("OOM while encoding audit message"); > +/* if both new and old source aren't provided don't log anything */ > +if (!newsrcpath && !oldsrcpath) Please move this to the next commit and let this one be just code movement. > return; > -} > + > +if (virAsprintfQuiet(&newdev, "new-%s", type) < 0) > +goto no_memory; > + > +if (virAsprintfQuiet(&olddev, "old-%s", type) < 0) > +goto no_memory; > + > +virUUIDFormat(vm->def->uuid, uuidstr); > + > +if (!(vmname = virAuditEncode("vm", vm->def->name))) > +goto no_memory; > > if (!(virt = virDomainVirtTypeToString(vm->def->virtType))) { > -VIR_WARN("Unexpected virt type %d while encoding audit message", > vm->def->virtType); > +VIR_WARN("Unexpected virt type %d while encoding audit message", > + vm->def->virtType); > virt = "?"; > } > > @@ -274,34 +228,19 @@ virDomainAuditNet(virDomainObjPtr vm, >virDomainNetDefPtr oldDef, virDomainNetDefPtr newDef, >const char *reason, bool success) > { > - > -VIR_AUDIT(VIR_AUDIT_RECORD_RESOURCE, success, > - "virt=%s resrc=net reason=%s %s uuid=%s old-net=%s new-net=%s", > - virt, reason, vmname, uuidstr, > - oldDef ? oldMacstr : "?", > - newDef ? newMacstr : "?"); > - > -VIR_FREE(vmname); > +virDomainAuditGenericDev(vm, "fs", The resrc should be "net" here, not "fs". > + oldDef ? oldMacstr : NULL, > + newDef ? newMacstr : NULL, > + reason, success); > } > > /** > ACK with the nits fixed. Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/6] audit: disk: Refactor disk auditing to avoid auditing remote storage
On 07/03/2014 12:05 PM, Peter Krempa wrote: > Pass the virStorageSource struct to the auditing function and check if > storage is local before auditting. *auditing This would be nice to document in auditlog.html > --- > src/conf/domain_audit.c | 25 - > src/conf/domain_audit.h | 4 ++-- > src/lxc/lxc_driver.c| 6 +++--- > src/qemu/qemu_driver.c | 4 ++-- > src/qemu/qemu_hotplug.c | 21 - > 5 files changed, 31 insertions(+), 29 deletions(-) > ACK Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/6] audit: Audit smartcard devices
On 07/03/2014 12:05 PM, Peter Krempa wrote: > --- > src/conf/domain_audit.c | 48 > 1 file changed, 48 insertions(+) > ACK if you add docs. Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/6] conf: audit: rng: Reorder new and old RNG device definitions
On 07/03/2014 12:04 PM, Peter Krempa wrote: > The audit functions usually take the old definition before the new one > in the argument list. Unify RNG device to use the same order. > --- > src/conf/domain_audit.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > ACK signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tests: Avoid double linking some libraries
On 07/03/2014 06:34 AM, Michal Privoznik wrote: > The problem is, since 614581f32b domaincapstest is linked with > $(LDADDS) by default. Then, since 94e3f23e8a7 the test may be > conditionally linked with $(qemu_LDADDS) which already contains > $(LDADDS). And some linkers doesn't cope with this nicely: I haven't reviewed this yet, but I'd like a shot at it. Give me 24 hours to respond (I'm first trying to push some of my acked patches as well as maint branch backports) -- 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
Re: [libvirt] [PATCH] tests: Avoid double linking some libraries
On 07/03/2014 08:44 AM, John Ferlan wrote: > > > On 07/03/2014 08:34 AM, Michal Privoznik wrote: >> The problem is, since 614581f32b domaincapstest is linked with >> $(LDADDS) by default. Then, since 94e3f23e8a7 the test may be >> conditionally linked with $(qemu_LDADDS) which already contains >> $(LDADDS). And some linkers doesn't cope with this nicely: >> >> CCLD domaincapstest >> ../src/libvirt_probes.o:(.probes+0x0): multiple definition of >> `libvirt_event_poll_add_handle_semaphore' >> ../src/libvirt_probes.o:(.probes+0x0): first defined here >> ../src/libvirt_probes.o:(.probes+0x2): multiple definition of >> `libvirt_event_poll_update_handle_semaphore' >> ../src/libvirt_probes.o:(.probes+0x2): first defined here >> ../src/libvirt_probes.o:(.probes+0x4): multiple definition of >> `libvirt_event_poll_remove_handle_semaphore' >> ../src/libvirt_probes.o:(.probes+0x4): first defined here >> ../src/libvirt_probes.o:(.probes+0x6): multiple definition of >> `libvirt_event_poll_dispatch_handle_semaphore' >> ../src/libvirt_probes.o:(.probes+0x6): first defined here >> >> And so on. >> >> Signed-off-by: Michal Privoznik >> --- >> >> I'd push this as build breaker, but there's another approach to consider, >> create qemu_BARE_LDADDS and fill it with the qemu_impl, etc. Then, let: >> qemu_LDADDS = $(qemu_BARE_LDADDS) $(LDADDS) >> >> What's your preference? >> > > What is below works for me - so to that degree I'd ACK. However, if > someone had a preference for whatever it is you're describing as an > option, then I suppose that would be fine too. Makefile's are mostly > black magic to me :-) > > John Let me amend my last statement - something is not quite working... I tried to apply this patch to other work I'm doing and things fell apart with missing symbols. If I try to reset my environment back to the top of the tree, then the build seems to work fine... If I apply this patch, then things fall apart in the same way. It's kind of difficult to describe, but there seems to be some dependency that isn't quite right or doesn't get reset if this patch is removed from the environment. I think the following steps make things reproducible. 1. Start with a "clean" top... 2. Build, will get the multiple definition failure... 3. Apply this patch (git am) 4. Build - get lots of failures 5. git reset HEAD^ 6. git co tests/Makefile.am 7. Build cleanly 8. git am the first change from my series 9. Build, results from step2 10. Repeat steps 3->7 for each/all of the patches from my series. I'm still wondering why steps 3 & 4 worked for me before - perhaps because I was testing something intermediary. Like I said before these Makefiles are black magic to me! John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tests: Avoid double linking some libraries
On 07/03/2014 12:47 PM, John Ferlan wrote: > > > On 07/03/2014 08:44 AM, John Ferlan wrote: >> >> >> On 07/03/2014 08:34 AM, Michal Privoznik wrote: >>> The problem is, since 614581f32b domaincapstest is linked with >>> $(LDADDS) by default. Then, since 94e3f23e8a7 the test may be >>> conditionally linked with $(qemu_LDADDS) which already contains >>> $(LDADDS). And some linkers doesn't cope with this nicely: >>> >>> CCLD domaincapstest >>> ../src/libvirt_probes.o:(.probes+0x0): multiple definition of >>> `libvirt_event_poll_add_handle_semaphore' >>> ../src/libvirt_probes.o:(.probes+0x0): first defined here >>> ../src/libvirt_probes.o:(.probes+0x2): multiple definition of >>> `libvirt_event_poll_update_handle_semaphore' >>> ../src/libvirt_probes.o:(.probes+0x2): first defined here >>> ../src/libvirt_probes.o:(.probes+0x4): multiple definition of >>> `libvirt_event_poll_remove_handle_semaphore' >>> ../src/libvirt_probes.o:(.probes+0x4): first defined here >>> ../src/libvirt_probes.o:(.probes+0x6): multiple definition of >>> `libvirt_event_poll_dispatch_handle_semaphore' >>> ../src/libvirt_probes.o:(.probes+0x6): first defined here >>> >>> And so on. >>> >>> Signed-off-by: Michal Privoznik >>> --- >>> >>> I'd push this as build breaker, but there's another approach to consider, >>> create qemu_BARE_LDADDS and fill it with the qemu_impl, etc. Then, let: >>> qemu_LDADDS = $(qemu_BARE_LDADDS) $(LDADDS) >>> >>> What's your preference? >>> >> >> What is below works for me - so to that degree I'd ACK. However, if >> someone had a preference for whatever it is you're describing as an >> option, then I suppose that would be fine too. Makefile's are mostly >> black magic to me :-) >> >> John > > Let me amend my last statement - something is not quite working... > > I tried to apply this patch to other work I'm doing and things fell > apart with missing symbols. If I try to reset my environment back to > the top of the tree, then the build seems to work fine... If I apply > this patch, then things fall apart in the same way. > > It's kind of difficult to describe, but there seems to be some > dependency that isn't quite right or doesn't get reset if this patch is > removed from the environment. > > I think the following steps make things reproducible. > > 1. Start with a "clean" top... > 2. Build, will get the multiple definition failure... > 3. Apply this patch (git am) > 4. Build - get lots of failures > 5. git reset HEAD^ > 6. git co tests/Makefile.am > 7. Build cleanly > 8. git am the first change from my series > 9. Build, results from step2 > 10. Repeat steps 3->7 for each/all of the patches from my series. > > I'm still wondering why steps 3 & 4 worked for me before - perhaps > because I was testing something intermediary. > > Like I said before these Makefiles are black magic to me! > > John > The following works for me: $ git diff tests/Makefile.am diff --git a/tests/Makefile.am b/tests/Makefile.am index a262c7b..2441742 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -833,12 +833,12 @@ vircaps2xmltest_LDADD = $(LDADDS) domaincapstest_SOURCES = \ domaincapstest.c testutils.h testutils.c -domaincapstest_LDADD = $(LDADDS) - if WITH_QEMU domaincapstest_SOURCES += testutilsqemu.c testutilsqemu.h -domaincapstest_LDADD += $(qemu_LDADDS) -endif WITH_QEMU +domaincapstest_LDADD = $(qemu_LDADDS) +else ! WITH_QEMU +domaincapstest_LDADD = $(LDADDS) +endif ! WITH_QEMU if WITH_LIBVIRTD libvirtdconftest_SOURCES = \ $ Not sure if it's the "right" solution, but I'm at least able to avoid the cycle of apply, build, remove, build. John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC: 'old' event for leaseshelper.c when lease renews
> Bummer. When 'init' is sent to the leases helper program, the > interface name is not known :'( so the helper program doesn't know > which *.status file it has to read and print to stdout. > > Simon came up with the following hack: > "" > The most obvious nasty hack to make this work would be to have a set of > filesystem links to the real lease-change script, each with a different > name, and configure each dnsmasq to call a unique link. The script then > checks argv[0] to find the name it was called by and then transforms > that into the name of the corresponding database file. > > > So we have something like > > scripts/interface1 is a link to /lib/libvirt/lease-change- > script > scripts/interface2 is a link to /lib/libvirt/lease-change-script > > and start dnsmasq with > > dnsmasq --interface=interface1 --dhcp-script=scripts/interface1 > > and the script finds the basename of argv[0[: > > scripts/interface1 -> interface1 > > and prepends the directory where the lease files are > > interface1 ->leasefiles/interface1 > > That works in the absence on the DNSMASQ_INTERFACE variable. > "" > > Is this hack acceptable? Ping! -- Nehal J Wani -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tests: Avoid double linking some libraries
On 03.07.2014 20:34, John Ferlan wrote: On 07/03/2014 12:47 PM, John Ferlan wrote: On 07/03/2014 08:44 AM, John Ferlan wrote: On 07/03/2014 08:34 AM, Michal Privoznik wrote: The problem is, since 614581f32b domaincapstest is linked with $(LDADDS) by default. Then, since 94e3f23e8a7 the test may be conditionally linked with $(qemu_LDADDS) which already contains $(LDADDS). And some linkers doesn't cope with this nicely: CCLD domaincapstest ../src/libvirt_probes.o:(.probes+0x0): multiple definition of `libvirt_event_poll_add_handle_semaphore' ../src/libvirt_probes.o:(.probes+0x0): first defined here ../src/libvirt_probes.o:(.probes+0x2): multiple definition of `libvirt_event_poll_update_handle_semaphore' ../src/libvirt_probes.o:(.probes+0x2): first defined here ../src/libvirt_probes.o:(.probes+0x4): multiple definition of `libvirt_event_poll_remove_handle_semaphore' ../src/libvirt_probes.o:(.probes+0x4): first defined here ../src/libvirt_probes.o:(.probes+0x6): multiple definition of `libvirt_event_poll_dispatch_handle_semaphore' ../src/libvirt_probes.o:(.probes+0x6): first defined here And so on. Signed-off-by: Michal Privoznik --- I'd push this as build breaker, but there's another approach to consider, create qemu_BARE_LDADDS and fill it with the qemu_impl, etc. Then, let: qemu_LDADDS = $(qemu_BARE_LDADDS) $(LDADDS) What's your preference? What is below works for me - so to that degree I'd ACK. However, if someone had a preference for whatever it is you're describing as an option, then I suppose that would be fine too. Makefile's are mostly black magic to me :-) John Let me amend my last statement - something is not quite working... I tried to apply this patch to other work I'm doing and things fell apart with missing symbols. If I try to reset my environment back to the top of the tree, then the build seems to work fine... If I apply this patch, then things fall apart in the same way. It's kind of difficult to describe, but there seems to be some dependency that isn't quite right or doesn't get reset if this patch is removed from the environment. I think the following steps make things reproducible. 1. Start with a "clean" top... 2. Build, will get the multiple definition failure... 3. Apply this patch (git am) 4. Build - get lots of failures 5. git reset HEAD^ 6. git co tests/Makefile.am 7. Build cleanly 8. git am the first change from my series 9. Build, results from step2 10. Repeat steps 3->7 for each/all of the patches from my series. I'm still wondering why steps 3 & 4 worked for me before - perhaps because I was testing something intermediary. Like I said before these Makefiles are black magic to me! John The following works for me: $ git diff tests/Makefile.am diff --git a/tests/Makefile.am b/tests/Makefile.am index a262c7b..2441742 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -833,12 +833,12 @@ vircaps2xmltest_LDADD = $(LDADDS) domaincapstest_SOURCES = \ domaincapstest.c testutils.h testutils.c -domaincapstest_LDADD = $(LDADDS) - if WITH_QEMU domaincapstest_SOURCES += testutilsqemu.c testutilsqemu.h -domaincapstest_LDADD += $(qemu_LDADDS) -endif WITH_QEMU +domaincapstest_LDADD = $(qemu_LDADDS) +else ! WITH_QEMU +domaincapstest_LDADD = $(LDADDS) +endif ! WITH_QEMU if WITH_LIBVIRTD libvirtdconftest_SOURCES = \ $ Not sure if it's the "right" solution, but I'm at least able to avoid the cycle of apply, build, remove, build. John The problem is, I've unintentionally omitted one qemu_LDADDS change: @@ -1055,7 +1056,7 @@ if WITH_QEMU securityselinuxlabeltest_SOURCES = \ securityselinuxlabeltest.c testutils.h testutils.c \ testutilsqemu.h testutilsqemu.c -securityselinuxlabeltest_LDADD = $(qemu_LDADDS) $(SELINUX_LIBS) +securityselinuxlabeltest_LDADD = $(qemu_LDADDS) $(LDADDS) $(SELINUX_LIBS) securityselinuxlabeltest_DEPENDENCIES = libsecurityselinuxhelper.la \ ../src/libvirt.la endif WITH_QEMU With this everything should be working as expected. I'll rather post it as v2. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] tests: Avoid double linking some libraries
The problem is, since 614581f32b domaincapstest is linked with $(LDADDS) by default. Then, since 94e3f23e8a7 the test may be conditionally linked with $(qemu_LDADDS) which already contains $(LDADDS). And some linkers doesn't cope with this nicely: CCLD domaincapstest ../src/libvirt_probes.o:(.probes+0x0): multiple definition of `libvirt_event_poll_add_handle_semaphore' ../src/libvirt_probes.o:(.probes+0x0): first defined here ../src/libvirt_probes.o:(.probes+0x2): multiple definition of `libvirt_event_poll_update_handle_semaphore' ../src/libvirt_probes.o:(.probes+0x2): first defined here ../src/libvirt_probes.o:(.probes+0x4): multiple definition of `libvirt_event_poll_remove_handle_semaphore' ../src/libvirt_probes.o:(.probes+0x4): first defined here ../src/libvirt_probes.o:(.probes+0x6): multiple definition of `libvirt_event_poll_dispatch_handle_semaphore' ../src/libvirt_probes.o:(.probes+0x6): first defined here And so on. Signed-off-by: Michal Privoznik --- diff to v1: -fix securityselinuxlabeltest too tests/Makefile.am | 31 --- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index a262c7b..bc1040a 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -509,12 +509,11 @@ endif WITH_STORAGE if WITH_DTRACE_PROBES qemu_LDADDS += ../src/libvirt_qemu_probes.lo endif WITH_DTRACE_PROBES -qemu_LDADDS += $(LDADDS) qemuxml2argvtest_SOURCES = \ qemuxml2argvtest.c testutilsqemu.c testutilsqemu.h \ testutils.c testutils.h -qemuxml2argvtest_LDADD = $(qemu_LDADDS) $(LIBXML_LIBS) +qemuxml2argvtest_LDADD = $(qemu_LDADDS) $(LDADDS) $(LIBXML_LIBS) qemuxml2argvmock_la_SOURCES = \ qemuxml2argvmock.c @@ -525,62 +524,64 @@ qemuxml2argvmock_la_LDFLAGS = -module -avoid-version \ qemuxml2xmltest_SOURCES = \ qemuxml2xmltest.c testutilsqemu.c testutilsqemu.h \ testutils.c testutils.h -qemuxml2xmltest_LDADD = $(qemu_LDADDS) +qemuxml2xmltest_LDADD = $(qemu_LDADDS) $(LDADDS) qemuxmlnstest_SOURCES = \ qemuxmlnstest.c testutilsqemu.c testutilsqemu.h \ testutils.c testutils.h -qemuxmlnstest_LDADD = $(qemu_LDADDS) +qemuxmlnstest_LDADD = $(qemu_LDADDS) $(LDADDS) qemuargv2xmltest_SOURCES = \ qemuargv2xmltest.c testutilsqemu.c testutilsqemu.h \ testutils.c testutils.h -qemuargv2xmltest_LDADD = $(qemu_LDADDS) +qemuargv2xmltest_LDADD = $(qemu_LDADDS) $(LDADDS) qemuhelptest_SOURCES = qemuhelptest.c testutils.c testutils.h -qemuhelptest_LDADD = $(qemu_LDADDS) +qemuhelptest_LDADD = $(qemu_LDADDS) $(LDADDS) qemumonitortest_SOURCES = qemumonitortest.c testutils.c testutils.h -qemumonitortest_LDADD = $(qemu_LDADDS) +qemumonitortest_LDADD = $(qemu_LDADDS) $(LDADDS) qemumonitorjsontest_SOURCES = \ qemumonitorjsontest.c \ testutils.c testutils.h \ testutilsqemu.c testutilsqemu.h \ $(NULL) -qemumonitorjsontest_LDADD = libqemumonitortestutils.la $(qemu_LDADDS) +qemumonitorjsontest_LDADD = libqemumonitortestutils.la \ + $(qemu_LDADDS) $(LDADDS) qemucapabilitiestest_SOURCES = \ qemucapabilitiestest.c \ testutils.c testutils.h \ testutilsqemu.c testutilsqemu.h \ $(NULL) -qemucapabilitiestest_LDADD = libqemumonitortestutils.la $(qemu_LDADDS) +qemucapabilitiestest_LDADD = libqemumonitortestutils.la \ + $(qemu_LDADDS) $(LDADDS) qemucaps2xmltest_SOURCES = \ qemucaps2xmltest.c \ testutils.c testutils.h \ $(NULL) -qemucaps2xmltest_LDADD = $(qemu_LDADDS) +qemucaps2xmltest_LDADD = $(qemu_LDADDS) $(LDADDS) qemuagenttest_SOURCES = \ qemuagenttest.c \ testutils.c testutils.h \ testutilsqemu.c testutilsqemu.h \ $(NULL) -qemuagenttest_LDADD = libqemumonitortestutils.la $(qemu_LDADDS) +qemuagenttest_LDADD = libqemumonitortestutils.la $(qemu_LDADDS) $(LDADDS) qemuhotplugtest_SOURCES = \ qemuhotplugtest.c \ testutils.c testutils.h \ testutilsqemu.c testutilsqemu.h \ $(NULL) -qemuhotplugtest_LDADD = libqemumonitortestutils.la $(qemu_LDADDS) +qemuhotplugtest_LDADD = libqemumonitortestutils.la $(qemu_LDADDS) $(LDADDS) domainsnapshotxml2xmltest_SOURCES = \ domainsnapshotxml2xmltest.c testutilsqemu.c testutilsqemu.h \ testutils.c testutils.h -domainsnapshotxml2xmltest_LDADD = $(qemu_LDADDS) +domainsnapshotxml2xmltest_LDADD = $(qemu_LDADDS) $(LDADDS) else ! WITH_QEMU EXTRA_DIST += qemuxml2argvtest.c qemuxml2xmltest.c qemuargv2xmltest.c \ qemuxmlnstest.c qemuhelptest.c domainsnapshotxml2xmltest.c \ @@ -837,7 +838,7 @@ domaincapstest_LDADD = $(LDADDS) if WITH_QEMU domaincapstest_SOURCES += testutilsqemu.c testutilsqemu.h -domaincapstest_LDADD += $(qemu_LDADDS) +domaincapstest_LDADD += $(qemu_LDADDS) $(GNULIB_LIBS) endif WITH_QEMU if WITH_LIBVIRTD @@ -1055,7 +1056,7 @@ if WITH_QEMU securityselinuxlabeltest_SOURCES = \ securityselinuxlabeltest.c testutils.h testutils.c
Re: [libvirt] new openvz driver (bossonvz)
On 07/03/2014 03:49 PM, Bosson VZ wrote: > Fair enough. We have no problem to adapt to the new process. I don't know how > libcontainer is designed > internally and how low-level it is but having a thin user-space wrapper > around the vzkernel would be wonderful. Here's the preliminary version of it will look like: https://github.com/xemul/libct/ The API would get slightly shuffled to fit more the https://github.com/docker/libcontainer/pull/28 > I wanted to use libvzctl in bossonvz to abstract the driver away from the > low-level stuff but the library > is just too much interconnected with the veconf parser. Well, the libcontainer API is not going to mess with configs at all. Everything will be run-time. > The current vz kernel API is not the nicest thing to work with to be honest > and a shared user-space library > would help both vzctl and bossonvz. If you are interested in my POV on the > kernel API, I would say you > should leave more work for the user-space. At the moment, when the user-space > asks for a new container, > the vzkernel magically creates a new environment and sets everything (esp. > cgroups) by itself without any > chance for the user-space to set anything. Because libvirt has its own cgroup > hierarchy model and any > libvirt driver should honor it, bossonvz should, idealy, create cgroups for > the container under the libvirt > hierarchy but it cannot since the kernel won't let it. Yes, this is our 15-years-old API that was kept compatible with tools. Soon after we started merging containers upstream we started the process of its deprecation, and in the upcoming Rhel7-based kernel we're very close to the goal. > As I see it, the driver has to do two separate things to successfully use the > kernel API. There are the > vz specific syscalls/ioctls (e.g. VZCTL_ENV_CREATE_DATA) and there is a > preparatory work done in the > user-space (preparing forked sub-processes, pipes, etc.). Is the new SDK > going to cover only the low-level > code (ioctls) or will I be able to use some sort of a high-level function > like spawn_environment() and get > PID of the newly created container's init? The SDK is going to be quite high-level, and more sophisticated than existing libvzctl. The libcontainer is going to be low-level but still more abstract than the kernel API. > The amount of vz specific code in the bossonvz driver is not that big. The > ioctls is just a single > bossonvz_environment.c file. The harder part is the preparatory part which is > located in bossonvz_container.c > and is quite complex since it closely mimics the code path of vzctl > (otherwise the driver would not work). It's interesting. Can you point out where you code is? And, btw, I think you would be interested in taking part in libcontainer design, discussions and development. The link above is the place where we try to settle down the new API and you're welcome to join :) Thanks, Pavel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] tests: Avoid double linking some libraries
On 07/03/2014 01:17 PM, Michal Privoznik wrote: > The problem is, since 614581f32b domaincapstest is linked with > $(LDADDS) by default. Then, since 94e3f23e8a7 the test may be > conditionally linked with $(qemu_LDADDS) which already contains > $(LDADDS). And some linkers doesn't cope with this nicely: > > CCLD domaincapstest > ../src/libvirt_probes.o:(.probes+0x0): multiple definition of > `libvirt_event_poll_add_handle_semaphore' > ../src/libvirt_probes.o:(.probes+0x0): first defined here > ../src/libvirt_probes.o:(.probes+0x2): multiple definition of > `libvirt_event_poll_update_handle_semaphore' > ../src/libvirt_probes.o:(.probes+0x2): first defined here > ../src/libvirt_probes.o:(.probes+0x4): multiple definition of > `libvirt_event_poll_remove_handle_semaphore' > ../src/libvirt_probes.o:(.probes+0x4): first defined here > ../src/libvirt_probes.o:(.probes+0x6): multiple definition of > `libvirt_event_poll_dispatch_handle_semaphore' > ../src/libvirt_probes.o:(.probes+0x6): first defined here > > And so on. > > Signed-off-by: Michal Privoznik > --- > > diff to v1: > -fix securityselinuxlabeltest too > > tests/Makefile.am | 31 --- > 1 file changed, 16 insertions(+), 15 deletions(-) This version makes sense to me. Rather than hiding LDADDS inside qemu_LDADDS, it makes qemu_LDADDS be only the additional libraries to add, and thus usable in conditions. 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
Re: [libvirt] [RFC][scale] new API for querying domains stats
On 07/01/2014 02:35 AM, Daniel P. Berrange wrote: > 1. A custom callback to run per API > > typedef (void)(*virDomainBlockInfoCallback)(virDomainPtr dom, > bool isError, > virDomainBlockInfoPtr info, > void *opaque); > It might be nice to require the callback to return an int; 0 to keep going, non-zero to stop immediately. > int virDomainGetBlockInfoAsync(virDomainPtr dom, >const char *disk, >virDomainBlockInfoCallback cb, >void *opaque, >unsigned int flags); What should this function return on success, 0 or the number of times the callback was reached? However, even if we add a callback return value (non-zero to quit immediately), I don't think feeding it directly to the return value is nice; we still want to reserve negative values for errors (couldn't even invoke callbacks, perhaps because dom was a bad pointer). Besides, a user can always use opaque to collect counts of how many times the callback was invoked, and/or a specific return value on early exit. -- 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
Re: [libvirt] [RFC][scale] new API for querying domains stats
On 07/01/2014 03:33 AM, Daniel P. Berrange wrote: > 1. Time to write() the RPC call to the socket > 2. Time for libvirtd to process the RPC call > 3. Time to recv() the RPC reply from the socket > ...and so on.. > > If the time for item 2 dominates over the time for items 1 & 2 (which > it should really) then the client thread is going to be sleeping in a > poll() for the bulk of the duration of the libvirt API call. If we had > an async API mechanism, then the VDSM time would essentially be consumed > with > > 1. Time to write() the RPC call to the socket > 2. Time to write() the RPC call to the socket > 3. Time to write() the RPC call to the socket > 4. Time to write() the RPC call to the socket > 5. Time to write() the RPC call to the socket > 6. Time to write() the RPC call to the socket > 7. wait for replies to start arriving > 8. Time to recv() the RPC reply from the socket > 9. Time to recv() the RPC reply from the socket > 10. Time to recv() the RPC reply from the socket > 11. Time to recv() the RPC reply from the socket > 12. Time to recv() the RPC reply from the socket > 13. Time to recv() the RPC reply from the socket > 14. Time to recv() the RPC reply from the socket This assumes you are still calling one async call per domain query. With regards to a bulk API, are you thinking synchronous? 1. Time to write() the RPC call - one bulk request 2. wait for reply - oh, and we'd better increase our on-wire size limits 3. Time to recv() the RPC reply - one bulk response or asynchronous? 1. Time to write() the RPC call - one bulk request 2. wait for replies to start arriving 3. Time to recv() an RPC async reply - first domain 4. Time to recv() an RPC async reply - second domain ... n. Time to recv() final RPC async reply The asynchronous works nicely in that we don't have to size up our max RPC on-wire limits, but implies that you still need a callback invoked once per reply received, instead of getting all data back in one giant memory blob. -- 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
Re: [libvirt] [PATCH v2] tests: Avoid double linking some libraries
On 07/03/2014 03:17 PM, Michal Privoznik wrote: > The problem is, since 614581f32b domaincapstest is linked with > $(LDADDS) by default. Then, since 94e3f23e8a7 the test may be > conditionally linked with $(qemu_LDADDS) which already contains > $(LDADDS). And some linkers doesn't cope with this nicely: > > CCLD domaincapstest > ../src/libvirt_probes.o:(.probes+0x0): multiple definition of > `libvirt_event_poll_add_handle_semaphore' > ../src/libvirt_probes.o:(.probes+0x0): first defined here > ../src/libvirt_probes.o:(.probes+0x2): multiple definition of > `libvirt_event_poll_update_handle_semaphore' > ../src/libvirt_probes.o:(.probes+0x2): first defined here > ../src/libvirt_probes.o:(.probes+0x4): multiple definition of > `libvirt_event_poll_remove_handle_semaphore' > ../src/libvirt_probes.o:(.probes+0x4): first defined here > ../src/libvirt_probes.o:(.probes+0x6): multiple definition of > `libvirt_event_poll_dispatch_handle_semaphore' > ../src/libvirt_probes.o:(.probes+0x6): first defined here > > And so on. > > Signed-off-by: Michal Privoznik > --- > > diff to v1: > -fix securityselinuxlabeltest too > > tests/Makefile.am | 31 --- > 1 file changed, 16 insertions(+), 15 deletions(-) ACK This one works for me. thanks! John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] tests: Avoid double linking some libraries
On 03.07.2014 21:29, Eric Blake wrote: On 07/03/2014 01:17 PM, Michal Privoznik wrote: The problem is, since 614581f32b domaincapstest is linked with $(LDADDS) by default. Then, since 94e3f23e8a7 the test may be conditionally linked with $(qemu_LDADDS) which already contains $(LDADDS). And some linkers doesn't cope with this nicely: CCLD domaincapstest ../src/libvirt_probes.o:(.probes+0x0): multiple definition of `libvirt_event_poll_add_handle_semaphore' ../src/libvirt_probes.o:(.probes+0x0): first defined here ../src/libvirt_probes.o:(.probes+0x2): multiple definition of `libvirt_event_poll_update_handle_semaphore' ../src/libvirt_probes.o:(.probes+0x2): first defined here ../src/libvirt_probes.o:(.probes+0x4): multiple definition of `libvirt_event_poll_remove_handle_semaphore' ../src/libvirt_probes.o:(.probes+0x4): first defined here ../src/libvirt_probes.o:(.probes+0x6): multiple definition of `libvirt_event_poll_dispatch_handle_semaphore' ../src/libvirt_probes.o:(.probes+0x6): first defined here And so on. Signed-off-by: Michal Privoznik --- diff to v1: -fix securityselinuxlabeltest too tests/Makefile.am | 31 --- 1 file changed, 16 insertions(+), 15 deletions(-) This version makes sense to me. Rather than hiding LDADDS inside qemu_LDADDS, it makes qemu_LDADDS be only the additional libraries to add, and thus usable in conditions. ACK. Thanks, pushed now. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 1/5] blockjob: allow omitted arguments to QMP block-commit
On 06/24/2014 01:10 AM, Peter Krempa wrote: > On 06/24/14 01:30, Eric Blake wrote: >> We are about to turn on support for active block commit. Although >> qemu 2.0 was the first version to mostly support it, that version >> mis-handles 0-length files, and doesn't have anything available for >> easy probing. But qemu 2.1 fixed bugs, and made life simpler by >> letting the 'top' argument be optional. Unless someone begs for >> active commit with qemu 2.0, for now we are just going to enable >> it only by probing for qemu 2.1 behavior (anyone backporting active >> commit can also backport the optional argument behavior). >> >> * src/qemu/qemu_monitor.h (qemuMonitorSupportsActiveCommit): New >> prototype. >> * src/qemu/qemu_monitor.c (qemuMonitorSupportsActiveCommit): >> Implement it. >> * src/qemu/qemu_monitor_json.h (qemuMonitorJSONBlockCommit): >> Allow NULL for top and base, for probing purposes. >> * src/qemu/qemu_monitor_json.c (qemuMonitorJSONBlockCommit): >> Likewise, implementing the probe. >> * tests/qemumonitorjsontest.c (mymain): Enable... >> (testQemuMonitorJSONqemuMonitorSupportsActiveCommit): ...a new test. >> > > ACK, Now pushed, after amending the commit message to point to the qemu.git commit this was gated on. -- 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
Re: [libvirt] [PATCH v4 2/5] blockjob: turn on qemu capability bit for active commit
On 06/24/2014 01:15 AM, Peter Krempa wrote: > On 06/24/14 01:30, Eric Blake wrote: >> Use the probing functionality added in the last patch to turn on >> a capability bit when active commit is present, and gate active >> commit on that capability. >> >> For my own reference: the difference between BLOCKJOB_SYNC and >> BLOCKJOB_ASYNC is whether qemu generated an event at the >> conclusion of blockpull; basically, RHEL 6.2 was the only release >> of qemu that has the sync semantics and lacks the event. RHEL >> 6.3 added blockcopy, but also picked up on the upstream style >> of qemu generating events. As no one is likely to backport >> active commit to RHEL 6.2, it's safe for blockcommit to always >> require async blockjob support. >> >> Modifying qemucapabilitiestest is painful; the .replies files would >> be so much easier if they had comments correlating which command >> generated the given reply. Maybe I'll fix that up later... >> > > ACK, Pushed. -- 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
Re: [libvirt] [PATCH v4 3/5] blockjob: expose active commit capability
On 06/24/2014 02:06 AM, Peter Krempa wrote: > On 06/24/14 01:30, Eric Blake wrote: >> Add an element to QEMU's capability XML, to show if the underlying >> qemu binary supports active commit. This allows the client to know >> ahead of time if they can rely on this method, or must fall back >> to older techniques such as blockpull. Without this information, >> the only way to check for active commit is to attempt one and check >> for errors. >> >> This attribute can be a simple binary (active commit is supported >> only if is in the capabilities) rather than a >> full-blown toggle definition. (In contrast, the >> capability had to be a toggle because we forgot to add it at the >> same time as turning on the feature of external snapshots; and >> therefore, the absence of the attribute is not sufficient to >> conclude whether disk snapshots are supported.) >> >> Our documentation for features was rather sparse; this fleshes out >> more of the details for other existing capabilities (and cost me >> some time trawling git history). I'm going to split this patch. The documentation for the pre-existing bits, >> +if (!virCapabilitiesAddGuestFeature(guest, "activecommit", >> +virQEMUCapsGet(qemubinCaps, >> + >> QEMU_CAPS_ACTIVE_COMMIT), >> +0)) > > Yuck, integers for boolean values. Pre-existing though. and a cleanup of int-to-bool, is worth having, but now that we have virConnectGetDomainCapabilities, I'd rather expose this new capability through that API than by overloading another feature into this code. -- 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
Re: [libvirt] [PATCH 0/5] Reorganize storage auth (both disk and pool)
On 06/27/2014 11:11 AM, John Ferlan wrote: > While understandably not really a 1.2.6 candidate, I figured I'd post this > now in hopes that it gets the ball rolling. The changes will help with > a related bz to support libiscsi for SCSI passthrough devices where a > entry for an iscsi adapter would be able to have data. > > The bulk of changes are code motion/merge. The details for each are: > > The first patch will create new API's which will then be used by the > parse domain disk and the parse storage pool code if the " is found. The logic merges the parsing found in the domain disk and > storage pool code with one slight caveat - the domain disk code expects > to find " use the Type{To/From}String API's in virstoragefile.c resulted in a > link time failure (even though secret_conf.h was included). Never quite > figured it out - I will take suggestions though. Although since the > domain disk code is all that really cares about it and the domain_conf > code can use the Type{To/From}String API's - I just left it as is. The > new copy API is used when the pool code needs to copy it's auth data into > each domain disk's auth entry. > > The second patch will fix a not-run test that tests the auth. It seems > it was not added to the active list of tests because the output generated > did not include the "usage" information (eg " usage='mycluster_myname") that > is part of the element. Additionally because it did not include > it the output was "". So rather than ignore > the test - add to the filters to avoid looking for " to the closing ">". The -auth.xml and -auth.args files needed adjustments > to follow the non "-auth" version of the files that have changed over time, > but not also changed int the "-auth" files. > > The third patch will cause the domain disk algorithms to utilize the > new generic parse and format APIs as well as using the 'authdef' instead > of the inline structure fields. This patch also restores the checking for > in the recently restored auth test. Still avoiding the whole > line as I saw no way to filter just the usage that's in > the stored XML file. > > The fourth patch is a mostly innocuous html change - it could go early, > but it just felt better in this place. > > The fifth patch will cause the storage pool algorithms to utilize the > new generic parse/format API's as well as new authdef structure for > accessing the data. The removal of the duplicated cephx/chap structures > seems (in my mind at least) to simplify things. They were essentially > copies of each other with no seemingly real value in keeping separate > other than the visual in the code of ".chap." or ".cephx.". > > John Ferlan (5): > virstorage: Introduce virStorageAuthDef > qemuargv2xmltest: Resurrect RBD and iSCSI auth > Utilize virDomainDiskAuth for domain disk > formatdomain: Fix issues found describing auth > Utilize virDomainDiskAuth for storage pools > > docs/formatdomain.html.in | 24 +-- > src/conf/domain_conf.c | 106 ++ > src/conf/storage_conf.c| 152 ++ > src/conf/storage_conf.h| 38 +--- > src/libvirt_private.syms | 5 +- > src/qemu/qemu_command.c| 72 --- > src/qemu/qemu_conf.c | 46 + > src/storage/storage_backend_iscsi.c| 41 ++-- > src/storage/storage_backend_rbd.c | 65 +++--- > src/util/virstoragefile.c | 219 > +++-- > src/util/virstoragefile.h | 37 +++- > tests/qemuargv2xmltest.c | 3 + > ...qemuxml2argv-disk-drive-network-iscsi-auth.args | 4 +- > .../qemuxml2argv-disk-drive-network-iscsi-auth.xml | 12 +- > .../qemuxml2argv-disk-drive-network-rbd-auth.xml | 4 +- > 15 files changed, 415 insertions(+), 413 deletions(-) > Made adjustments from reviews, made sure my build worked, retested, and pushed. Thanks for the reviews - John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH]util:ovs:Delete port if it is exist when add port
Resubmit the patch, remove '--may-exist' and I have tested, it work. diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index 9bcbfb1..2c414ad 100644 --- a/src/util/virnetdevopenvswitch.c +++ b/src/util/virnetdevopenvswitch.c @@ -84,8 +84,8 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname, cmd = virCommandNew(OVSVSCTL); -virCommandAddArgList(cmd, "--timeout=5", "--", "--may-exist", "add-port", -brname, ifname, NULL); +virCommandAddArgList(cmd, "--timeout=5", "--", "--if-exists", "del-port", +ifname, "--", "add-port", brname, ifname, NULL); if (virtVlan && virtVlan->nTags > 0) { >-Original Message- >From: libvir-list-boun...@redhat.com [mailto:libvir-list-boun...@redhat.com] >On Behalf Of Lichunhe >Sent: Thursday, July 03, 2014 9:21 PM >To: Michal Privoznik; libvir-list@redhat.com >Subject: Re: [libvirt] [PATCH]util:ovs:Delete port if it is exist when add port > >Yeah, the '--may-exist' could be removed, it is not useful. > >>-Original Message- >>From: Michal Privoznik [mailto:mpriv...@redhat.com] >>Sent: Thursday, July 03, 2014 8:39 PM >>To: Lichunhe; libvir-list@redhat.com >>Subject: Re: [libvirt] [PATCH]util:ovs:Delete port if it is exist when >>add port >> >>On 03.07.2014 13:57, Lichunhe wrote: >>> If the ovs service stop abnormal, or host cold reboot, vm is >>> destroyed after ovs service stop. The ovs port which connect to >>> interface of vm will not be clear. When the ovs service restart, >>> recover configuration from db, but the interface is no exist, port >>> recovery failed, and then vm restart on the same host, libvirt add >>> port again, but the port configuration is same as before, ovs will >>> not connect the interface, only store the configuration in db. Below >>> will trigger this problem, >>> >>> virsh start vm >>> service openvswitch-switch stop >>> virsh destroy vm >>> service openvswitch-switch start >>> virsh start vm >>> >>> >>> Signed-off-by: Chunhe Li >>> --- >>> >>> src/util/virnetdevopenvswitch.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/src/util/virnetdevopenvswitch.c >>> b/src/util/virnetdevopenvswitch.c index 9bcbfb1..dd601c0 100644 >>> --- a/src/util/virnetdevopenvswitch.c >>> +++ b/src/util/virnetdevopenvswitch.c >>> @@ -84,7 +84,8 @@ int virNetDevOpenvswitchAddPort(const char >*brname, >>> const char *ifname, >>> >>> cmd = virCommandNew(OVSVSCTL); >>> >>> -virCommandAddArgList(cmd, "--timeout=5", "--", "--may-exist", >>"add-port", >>> +virCommandAddArgList(cmd, "--timeout=5", "--", "--if-exists", >>"del-port", >>> +ifname, "--", "--may-exist", "add-port", >>> brname, ifname, NULL); >> >>So what's the meaning of '--may-exist' then? Does it do anything useful after >all? >> >>Michal > >-- >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] [PATCH] LXC: fix an improper comments for lxcDomainDestroyFlags
Currently @flag is not used yet. Signed-off-by: Chen Hanxiao --- src/lxc/lxc_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 79c3b4a..5c4ab58 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1420,7 +1420,7 @@ lxcConnectDomainEventDeregisterAny(virConnectPtr conn, /** * lxcDomainDestroyFlags: * @dom: pointer to domain to destroy - * @flags: an OR'ed set of virDomainDestroyFlags + * @flags: extra flags; not used yet. * * Sends SIGKILL to container root process to terminate the container * -- 1.9.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list