Re: [libvirt] [Users] Migration issues with ovirt 3.3
On Wed, Oct 09, 2013 at 16:18:25 +0100, Dan Kenigsberg wrote: On Wed, Oct 09, 2013 at 04:52:20PM +0200, Gianluca Cecchi wrote: On Wed, Oct 9, 2013 at 3:43 PM, Dan Kenigsberg wrote: On Wed, Oct 09, 2013 at 02:42:22PM +0200, Gianluca Cecchi wrote: On Tue, Oct 8, 2013 at 10:40 AM, Dan Kenigsberg wrote: But migration still fails It seems like an unrelated failure. I do not know what's blocking migration traffic. Could you see if libvirtd.log and qemu logs at source and destinaiton have clues? It seems that on VM.log under qemu of desdt host I have: ... -incoming tcp:[::]:49153: Failed to bind socket: Address already in use Is that port really taken (`ss -ntp` should tell by whom)? yeah ! It seems gluster uses it on both sides Since libvirt has been using this port range first, would you open a bug on gluster to avoid it? Dan. (prays that Vdsm is not expected to edit libvirt's migration port range on installation) If it makes you feel better, you can't even do that :-) Unfortunately, the port range used for migration is hard-coded in libvirt. And since we don't use port allocator yet (patches are in progress), we don't automatically avoid ports that are already taken. All this is going to change, though. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu_migration: Check ABI stability against MIGRATABLE xml
On 09.10.2013 05:09, Eric Blake wrote: On 10/08/2013 03:43 AM, Michal Privoznik wrote: https://bugzilla.redhat.com/show_bug.cgi?id=994364 If user provides domain XML in migration we check if it doesn't break ABI via virDomainDefCheckABIStability(). However, if the provided XML was generated via virDomainGetXMLDesc(..., VIR_DOMAIN_XML_MIGRATABLE) it is missing some devices, e.g. 'pci-root' controller. Hence, the ABI stability check fails even though it is stable. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/qemu/qemu_migration.c | 41 + 1 file changed, 33 insertions(+), 8 deletions(-) Oh my. How many different ways is this going to bite us? Is it worth fixing virDomainDefCheckABIStability to do the sanitization itself, instead of making every single caller across multiple patches repeat the work? What you have seems to work, but I'm really starting to wonder if it is the best patch. Weak ACK. Unfortunately, we can't do that simply. virDomainDefCheckABIStability() is defined in domain_conf.c and thus meant to be driver-unaware. I mean, in this patch I'm correctly using qemuDomainDefFormatLive() which on MIGRATABLE flag enabled removes the default usb controller. However, this constraint applies just for the qemu driver. So even if I'd call virDomainDefCopy() within virDomainDefCheckABIStability() to get the MIGRATABLE xml, it is not sufficient for qemu driver. But what about introducing qemuDomainDefCheckABIStability: (writing from the top of my head, doesn't have to necessary compile) qemuDomainDefCheckABIStability(virQEMUDriverPtr driver, virCapsPtr caps, virDomainDefPtr src, virDomainDefPtr dst) { migratableDefStr = qemuDomainDefFormatLive(driver, src, true, false); migratableDef = virDomainDefParseString(migratableDefstr, caps, ...) return virDomainDefCheckABIStability(migratableDef, dst); } And replacing (nearly?) all virDomainDefCheckABIStability() calls under src/qemu/* with the new wrapper? Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv4 2/3] qemu: hostdev: Add checks if PCI passthrough is availabe in the host
On 10/09/13 15:02, Christophe Fergeau wrote: If it's not pushed already, there's a typo in the subject: 'availabe' Thanks for noticing. Pushed with fixed subject. Christophe On Tue, Oct 08, 2013 at 05:46:57PM +0200, Peter Krempa wrote: Add code to check availability of PCI passhthrough using VFIO and the legacy KVM passthrough and use it when starting VMs and hotplugging devices to live machine. --- Notes: Version 4: - moved this function so that it can be called from qemuPrepareHostdevPCIDevices() - now caching the support for passthrough types right away src/qemu/qemu_hostdev.c | 125 1 file changed, 125 insertions(+) Peter 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 1/4] qemu: Fix coding style in qemuDomainSaveFlags()
On 10/09/13 18:38, Eric Blake wrote: On 10/09/2013 10:31 AM, Peter Krempa wrote: Avoid mixed brace style in an if statement and fix formatting of error messages. --- src/qemu/qemu_driver.c | 18 -- 1 file changed, 8 insertions(+), 10 deletions(-) ACK. Pushed; Thanks. Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv4 1/3] qemu: hostdev: Fix function spacing and header formatting
On 10/09/13 14:41, Laine Stump wrote: On 10/08/2013 06:46 PM, Peter Krempa wrote: --- Notes: New in this version. src/qemu/qemu_hostdev.c | 60 ++--- 1 file changed, 42 insertions(+), 18 deletions(-) ... ACK Pushed; Thanks. Peter 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/4] qemu: refactor qemuCompressProgramAvailable()
On 10/09/13 22:37, Eric Blake wrote: On 10/09/2013 10:31 AM, Peter Krempa wrote: --- src/qemu/qemu_driver.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) ACK. Pushed; Thanks. Peter 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/4] qemu: snapshot: Add support for compressing external snapshot memory
On 10/09/13 18:38, Daniel P. Berrange wrote: On Wed, Oct 09, 2013 at 06:31:32PM +0200, Peter Krempa wrote: The regular save image code has the support to compress images using a specified algorithm. This was not implemented for external checkpoints although it shares most of the backend code. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1017227 --- src/qemu/qemu.conf | 6 ++ src/qemu/qemu_conf.c | 1 + src/qemu/qemu_conf.h | 1 + src/qemu/qemu_driver.c | 23 +-- 4 files changed, 29 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 7cf67df..aff5e6d 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -283,9 +283,15 @@ # specified format isn't valid, or the requested compression program can't be # found. # +# snapshot_image_format specifies the compression algorithm of the memory save +# image when an external snapshot of a domain is taken. This does not apply +# on disk image format. It is an error if the specified format isn't valid, +# or the requested compression program can't be found. +# #save_image_format = raw #dump_image_format = raw #managedsave_image_format = raw +#snapshot_image_format = raw Same question here as previous patch. A snapshot is really just another form of saved image. Here I have a bit stronger opinion compared to the managedsave case. I really think that users might prefer different settings for snapshots especially that snapshots can be created with a live guest, where a compression algorithm may inhibit successful finish of creating a snapshot as the migration procedure will be too slow to accomodate changing a lot of pages in the guest memory. Thus it might be desired to store snapshots uncompressed or just with a quick algorithm. The second option would be to disable compression for live snapshots, but that might be unfortunate too. Daniel Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Introduce qemuDomainDefCheckABIStability
https://bugzilla.redhat.com/show_bug.cgi?id=994364 Whenever we check for ABI stability, we have new xml (e.g. provided by user, or obtained from snapshot, whatever) which we compare to old xml and see if ABI won't break. However, if the new xml was produced via virDomainGetXMLDesc(..., VIR_DOMAIN_XML_MIGRATABLE) it lacks some devices, e.g. 'pci-root' controller. Hence, the ABI stability check fails even though it is stable. Moreover, we can't simply fix virDomainDefCheckABIStability because removing the correct devices is task for the driver. For instance, qemu driver wants to remove the usb controller too, while LXC driver doesn't. That's why we need special qemu wrapper over virDomainDefCheckABIStability which removes the correct devices from domain XML, produces MIGRATABLE xml and calls the check ABI stability function. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/qemu/qemu_domain.c| 22 ++ src/qemu/qemu_domain.h| 3 +++ src/qemu/qemu_driver.c| 10 ++ src/qemu/qemu_migration.c | 4 ++-- 4 files changed, 29 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 968e323..f3e2ba1 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2342,3 +2342,25 @@ qemuDomainUpdateDeviceList(virQEMUDriverPtr driver, priv-qemuDevices = aliases; return 0; } + +bool +qemuDomainDefCheckABIStability(virQEMUDriverPtr driver, + virDomainDefPtr src, + virDomainDefPtr dst) +{ +virDomainDefPtr migratableDefSrc = NULL; +virDomainDefPtr migratableDefDst = NULL; +const int flags = VIR_DOMAIN_XML_SECURE | VIR_DOMAIN_XML_UPDATE_CPU | VIR_DOMAIN_XML_MIGRATABLE; +bool ret = false; + +if (!(migratableDefSrc = qemuDomainDefCopy(driver, src, flags)) || +!(migratableDefDst = qemuDomainDefCopy(driver, dst, flags))) +goto cleanup; + +ret = virDomainDefCheckABIStability(migratableDefSrc, migratableDefDst); + +cleanup: +virDomainDefFree(migratableDefSrc); +virDomainDefFree(migratableDefDst); +return ret; +} diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 21f116c..5d9fab9 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -368,4 +368,7 @@ extern virDomainDefParserConfig virQEMUDriverDomainDefParserConfig; int qemuDomainUpdateDeviceList(virQEMUDriverPtr driver, virDomainObjPtr vm); +bool qemuDomainDefCheckABIStability(virQEMUDriverPtr driver, +virDomainDefPtr src, +virDomainDefPtr dst); #endif /* __QEMU_DOMAIN_H__ */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c71aecc..8481591 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3077,7 +3077,7 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, virDomainPtr dom, VIR_DOMAIN_XML_INACTIVE))) { goto endjob; } -if (!virDomainDefCheckABIStability(vm-def, def)) { +if (!qemuDomainDefCheckABIStability(driver, vm-def, def)) { virDomainDefFree(def); goto endjob; } @@ -12889,7 +12889,6 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, qemuDomainObjPrivatePtr priv; int rc; virDomainDefPtr config = NULL; -virDomainDefPtr migratableDef = NULL; virQEMUDriverConfigPtr cfg = NULL; virCapsPtr caps = NULL; @@ -13006,11 +13005,7 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, /* Transitions 5, 6, 8, 9 */ /* Check for ABI compatibility. We need to do this check against * the migratable XML or it will always fail otherwise */ -if (!(migratableDef = qemuDomainDefCopy(driver, vm-def, - VIR_DOMAIN_XML_MIGRATABLE))) -goto cleanup; - -if (config !virDomainDefCheckABIStability(migratableDef, config)) { +if (config !qemuDomainDefCheckABIStability(driver, vm-def, config)) { virErrorPtr err = virGetLastError(); if (!(flags VIR_DOMAIN_SNAPSHOT_REVERT_FORCE)) { @@ -13215,7 +13210,6 @@ cleanup: } if (vm) virObjectUnlock(vm); -virDomainDefFree(migratableDef); virObjectUnref(caps); virObjectUnref(cfg); diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 3a1aab7..59fd263 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2039,7 +2039,7 @@ static char VIR_DOMAIN_XML_INACTIVE))) goto cleanup; -if (!virDomainDefCheckABIStability(vm-def, def)) +if (!qemuDomainDefCheckABIStability(driver, vm-def, def)) goto cleanup; rv = qemuDomainDefFormatLive(driver, def, false,
Re: [libvirt] [PATCH] maint: make syntax-check default to a no-op on maint branches
On 10/09/2013 04:10 PM, Daniel P. Berrange wrote: We should absolutely be running syntax-check as normal to identify that on -maint branches just as on master. IMHO all we want todo is add: local-checks-to-skip += sc_copyright_check When on -maint branches. I agree. make syntax-check is just as useful on a -maint branch as on master; I like to run it on those branches not as a matter of habit, but in order to catch potential problems. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv4 3/3] qemu: Prefer VFIO for PCI device passthrough
On 10/09/13 16:01, Laine Stump wrote: On 10/08/2013 06:46 PM, Peter Krempa wrote: Prefer using VFIO (if available) to the legacy KVM device passthrough. With this patch a PCI passthrough device without the driver configured will be started with VFIO if it's available on the host. If not legacy KVM passthrough is checked and error is reported if it's not available. --- Notes: Version 4: - Adapted to call tree change docs/formatdomain.html.in | 9 - src/conf/domain_conf.h| 2 +- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_hostdev.c | 30 +- src/qemu/qemu_hostdev.h | 4 +++- src/qemu/qemu_hotplug.c | 2 +- src/qemu/qemu_process.c | 15 --- tests/qemuxml2argvtest.c | 11 +++ 8 files changed, 54 insertions(+), 21 deletions(-) ACk, with a less misleading error message. I've changed the error message to _(invalid PCI passthrough type '%s') and pushed. Thanks for the review. Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Init @pcidevs in qemuPrepareHostdevPCIDevices
At the beginning of the function qemuPrepareHostdevPCICheckSupport() is called. After that @pcidevs is initialized. However, if the very first command fails, we go to 'cleanup' label where virObjectUnref(pcidevs) is called. Obviously, it is called before @pcidevs was able to get initialized. Compiler warns about it: CC qemu/libvirt_driver_qemu_impl_la-qemu_hostdev.lo qemu/qemu_hostdev.c: In function 'qemuPrepareHostdevPCIDevices': qemu/qemu_hostdev.c:824:19: error: 'pcidevs' may be used uninitialized in this function [-Werror=maybe-uninitialized] virObjectUnref(pcidevs); ^ cc1: all warnings being treated as errors Signed-off-by: Michal Privoznik mpriv...@redhat.com --- Pushed under buildbreaker and trivial rules. src/qemu/qemu_hostdev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c index 81e0e88..43c03cc 100644 --- a/src/qemu/qemu_hostdev.c +++ b/src/qemu/qemu_hostdev.c @@ -633,7 +633,7 @@ qemuPrepareHostdevPCIDevices(virQEMUDriverPtr driver, int nhostdevs, virQEMUCapsPtr qemuCaps) { -virPCIDeviceListPtr pcidevs; +virPCIDeviceListPtr pcidevs = NULL; int last_processed_hostdev_vf = -1; size_t i; int ret = -1; -- 1.8.1.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/6] API documentation improvements
ping? At Wed, 25 Sep 2013 08:49:03 +0200, Claudio Bley wrote: Hi. It's been a while since I tackled this, but here it goes... This is version 4 of https://www.redhat.com/archives/libvir-list/2013-January/msg02094.html exclusive of the already applied patches, of course. Changes since v3: * skipped the ECMAScript code highlighting patch[1] in this series (postponed for now) * added link generation patch (#6) which I had proposed seperately back in Jan 2013, too. * added a reference to an affected API in patch #1 and #4 as per Eric's comments * changed the code block XSL processing to avoid cutting off characters at the beginning of a line [1] https://www.redhat.com/archives/libvir-list/2013-January/msg02104.html [2] https://www.redhat.com/archives/libvir-list/2013-January/msg00884.html Claudio Bley (6): docs: process code blocks similar to Markdown docs: add class description to div's containing descriptions docs: define style of code blocks inside descriptions libvirt.c: add 2 spaces of indentation to example code of virStreamSend libvirt.c: indent code of virDomainGetMemoryParameters's documentation docs: generate links from plain text documentation docs/libvirt.css |7 +++ docs/newapi.xsl | 121 +++--- src/libvirt.c| 130 +++--- 3 files changed, 167 insertions(+), 91 deletions(-) -- 1.7.9.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- AV-Test GmbH, Henricistraße 20, 04155 Leipzig, Germany Phone: +49 341 265 310 19 Web:http://www.av-test.org Eingetragen am / Registered at: Amtsgericht Stendal (HRB 114076) Geschaeftsfuehrer (CEO): Andreas Marx, Guido Habicht, Maik Morgenstern -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] AArch64: Porting of armv7l conditons to run qemu for aarch64.
AArch64 qemu has similar behavior as armv7l, like use of mmio etc. This patch adds similar bypass checks what we have for armv7l to aarch64. E.g. we are enabling mmio transport for Nicdev. Making addDefaultUSB and addDefaultMemballoon to false etc. Signed-off-by: Anup Patel anup.pa...@linaro.org Signed-off-by: Pranavkumar Sawargaonkar pranavku...@linaro.org --- src/qemu/qemu_capabilities.c |2 +- src/qemu/qemu_command.c |8 +--- src/qemu/qemu_domain.c |5 - 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 1eae4ba..74a1690 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2872,7 +2872,7 @@ virQEMUCapsSupportsChardev(virDomainDefPtr def, !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) return false; -if (def-os.arch != VIR_ARCH_ARMV7L) +if ((def-os.arch != VIR_ARCH_ARMV7L) (def-os.arch != VIR_ARCH_AARCH64)) return true; /* This may not be true for all ARM machine types, but at least diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 22cc5f2..1c4de84 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -427,7 +427,8 @@ qemuDomainSupportsNicdev(virDomainDefPtr def, return false; /* non-virtio ARM nics require legacy -net nic */ -if (def-os.arch == VIR_ARCH_ARMV7L +if (((def-os.arch == VIR_ARCH_ARMV7L) || +(def-os.arch == VIR_ARCH_AARCH64)) net-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO) return false; @@ -1339,7 +1340,8 @@ static int qemuDomainAssignARMVirtioMMIOAddresses(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) { -if (def-os.arch == VIR_ARCH_ARMV7L +if (((def-os.arch == VIR_ARCH_ARMV7L) || +(def-os.arch == VIR_ARCH_AARCH64)) STRPREFIX(def-os.machine, vexpress-) virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_MMIO)) { qemuDomainPrimeVirtioDeviceAddresses( @@ -1870,7 +1872,7 @@ cleanup: static bool qemuDomainSupportsPCI(virDomainDefPtr def) { -if (def-os.arch != VIR_ARCH_ARMV7L) +if ((def-os.arch != VIR_ARCH_ARMV7L) (def-os.arch != VIR_ARCH_AARCH64)) return true; if (STREQ(def-os.machine, versatilepb)) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 968e323..f4986d7 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -727,7 +727,10 @@ qemuDomainDefPostParse(virDomainDefPtr def, addDefaultUSB = false; addDefaultMemballoon = false; break; - +case VIR_ARCH_AARCH64: + addDefaultUSB = false; + addDefaultMemballoon = false; + break; case VIR_ARCH_ALPHA: case VIR_ARCH_PPC: case VIR_ARCH_PPC64: -- 1.7.9.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 0/5] qemumonitorjsontest: Introduce some tests
On 03.10.2013 12:49, Michal Privoznik wrote: v2 to some patches that weren't ACKed yet. Michal Privoznik (5): qemuMonitorTestFree: Join worker thread qemumonitorjsontest: Extend the test for yet another monitor commands qemumonitorjsontest: Test qemuMonitorJSONGetCPUInfo qemumonitorjsontest: Test qemuMonitorJSONGetVirtType qemumonitorjsontest: Test qemuMonitorJSONSendKey tests/qemumonitorjsontest.c | 253 +++ tests/qemumonitortestutils.c | 5 +- 2 files changed, 256 insertions(+), 2 deletions(-) Ping? 1/5 is ACked and pushed, but what about the others? Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/6] AArch64: Add AArch64 architecture to list of valid arches.
Hi Daniel, On 9 October 2013 20:14, Daniel P. Berrange berra...@redhat.com wrote: On Tue, Oct 08, 2013 at 07:19:06PM +0530, Pranavkumar Sawargaonkar wrote: Adding AArch64(ARMv8 64bit) to the current list of valid architectures. For now, AArch64 name would imply AArch64 LE mode only. In future, we might have separate names for AArch64 LE and BE. Presumably the kernel has fixed 'aarch64' as always referring to LE and would add a variant like 'aarch64b' for BE mode if that was ever supported ? Firstly thanks for Acking my patches. Yes linux currently uses aarch64 for LE . There is a BE port going on and should add a variant similar to what you have mentioned to distinguish. Signed-off-by: Anup Patel anup.pa...@linaro.org Signed-off-by: Pranavkumar Sawargaonkar pranavku...@linaro.org --- src/util/virarch.c |1 + src/util/virarch.h |1 + 2 files changed, 2 insertions(+) diff --git a/src/util/virarch.c b/src/util/virarch.c index 694eba1..9e88c68 100644 --- a/src/util/virarch.c +++ b/src/util/virarch.c @@ -38,6 +38,7 @@ static const struct virArchData { { armv6l, 32, VIR_ARCH_LITTLE_ENDIAN }, { armv7l, 32, VIR_ARCH_LITTLE_ENDIAN }, { armv7b, 32, VIR_ARCH_BIG_ENDIAN }, +{ aarch64, 64, VIR_ARCH_LITTLE_ENDIAN }, { cris, 32, VIR_ARCH_LITTLE_ENDIAN }, { i686, 32, VIR_ARCH_LITTLE_ENDIAN }, diff --git a/src/util/virarch.h b/src/util/virarch.h index 3530f7c..d0bf9d9 100644 --- a/src/util/virarch.h +++ b/src/util/virarch.h @@ -30,6 +30,7 @@ typedef enum { VIR_ARCH_ARMV6L, /* ARMv6 32 LE http://en.wikipedia.org/wiki/ARM_architecture */ VIR_ARCH_ARMV7L, /* ARMv7 32 LE http://en.wikipedia.org/wiki/ARM_architecture */ VIR_ARCH_ARMV7B, /* ARMv7 32 BE http://en.wikipedia.org/wiki/ARM_architecture */ +VIR_ARCH_AARCH64, /* ARMv8 64 LE http://en.wikipedia.org/wiki/ARM_architecture */ VIR_ARCH_CRIS, /* ETRAX 32 LE http://en.wikipedia.org/wiki/ETRAX_CRIS */ VIR_ARCH_I686, /* x86 32 LE http://en.wikipedia.org/wiki/X86 */ ACK 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 Thanks, Pranav -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] AArch64: Porting of armv7l conditons to run qemu for aarch64.
On Thu, Oct 10, 2013 at 05:10:41PM +0530, Pranavkumar Sawargaonkar wrote: AArch64 qemu has similar behavior as armv7l, like use of mmio etc. This patch adds similar bypass checks what we have for armv7l to aarch64. E.g. we are enabling mmio transport for Nicdev. Making addDefaultUSB and addDefaultMemballoon to false etc. Signed-off-by: Anup Patel anup.pa...@linaro.org Signed-off-by: Pranavkumar Sawargaonkar pranavku...@linaro.org --- src/qemu/qemu_capabilities.c |2 +- src/qemu/qemu_command.c |8 +--- src/qemu/qemu_domain.c |5 - 3 files changed, 10 insertions(+), 5 deletions(-) Our general rule is that changes to qemu_command.c should be accompanied by additions to tests/qemuxml2argvtest.c to validate their correctness, otherwise it is all to easy for us to have regressions in behaviour later. 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] AArch64: Porting of armv7l conditons to run qemu for aarch64.
Hi, On 10 October 2013 17:20, Daniel P. Berrange berra...@redhat.com wrote: On Thu, Oct 10, 2013 at 05:10:41PM +0530, Pranavkumar Sawargaonkar wrote: AArch64 qemu has similar behavior as armv7l, like use of mmio etc. This patch adds similar bypass checks what we have for armv7l to aarch64. E.g. we are enabling mmio transport for Nicdev. Making addDefaultUSB and addDefaultMemballoon to false etc. Signed-off-by: Anup Patel anup.pa...@linaro.org Signed-off-by: Pranavkumar Sawargaonkar pranavku...@linaro.org --- src/qemu/qemu_capabilities.c |2 +- src/qemu/qemu_command.c |8 +--- src/qemu/qemu_domain.c |5 - 3 files changed, 10 insertions(+), 5 deletions(-) Our general rule is that changes to qemu_command.c should be accompanied by additions to tests/qemuxml2argvtest.c to validate their correctness, otherwise it is all to easy for us to have regressions in behaviour later. Ok i will revise this patch. 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 :| Thanks, Pranav -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 0/2] Enable compression of external snapshots and managed save images
Version 2 treats save images and managed save images as same, using the same config option. Peter Krempa (2): qemu: managedsave: Add support for compressing managed save images qemu: snapshot: Add support for compressing external snapshot memory src/qemu/qemu.conf | 12 +--- src/qemu/qemu_conf.c | 2 ++ src/qemu/qemu_conf.h | 1 + src/qemu/qemu_driver.c | 46 ++ 4 files changed, 54 insertions(+), 7 deletions(-) -- 1.8.3.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 1/2] qemu: managedsave: Add support for compressing managed save images
The regular save image code has the support to compress images using a specified algorithm. This was not implemented for managed save although it shares most of the backend code. --- src/qemu/qemu.conf | 6 +++--- src/qemu/qemu_driver.c | 23 +-- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 5fd6263..cf82ffe 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -269,9 +269,9 @@ # saving a domain in order to save disk space; the list above is in descending # order by performance and ascending order by compression ratio. # -# save_image_format is used when you use 'virsh save' at scheduled -# saving, and it is an error if the specified save_image_format is -# not valid, or the requested compression program can't be found. +# save_image_format is used when you use 'virsh save' or 'virsh managedsave' +# at scheduled saving, and it is an error if the specified save_image_format +# is not valid, or the requested compression program can't be found. # # dump_image_format is used when you use 'virsh dump' at emergency # crashdump, and if the specified dump_image_format is not valid, or diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index cfdbb9a..6f15240 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3232,6 +3232,8 @@ static int qemuDomainManagedSave(virDomainPtr dom, unsigned int flags) { virQEMUDriverPtr driver = dom-conn-privateData; +virQEMUDriverConfigPtr cfg = NULL; +int compressed = QEMU_SAVE_FORMAT_RAW; virDomainObjPtr vm; char *name = NULL; int ret = -1; @@ -3257,13 +3259,29 @@ qemuDomainManagedSave(virDomainPtr dom, unsigned int flags) goto cleanup; } +cfg = virQEMUDriverGetConfig(driver); +if (cfg-saveImageFormat) { +compressed = qemuSaveCompressionTypeFromString(cfg-saveImageFormat); +if (compressed 0) { +virReportError(VIR_ERR_OPERATION_FAILED, %s, + _(Invalid save image format specified + in configuration file)); +goto cleanup; +} +if (!qemuCompressProgramAvailable(compressed)) { +virReportError(VIR_ERR_OPERATION_FAILED, %s, + _(Compression program for image format + in configuration file isn't available)); +goto cleanup; +} +} + if (!(name = qemuDomainManagedSavePath(driver, vm))) goto cleanup; VIR_INFO(Saving state of domain '%s' to '%s', vm-def-name, name); -if ((ret = qemuDomainSaveInternal(driver, dom, vm, name, - QEMU_SAVE_FORMAT_RAW, +if ((ret = qemuDomainSaveInternal(driver, dom, vm, name, compressed, NULL, flags)) == 0) vm-hasManagedSave = true; @@ -3273,6 +3291,7 @@ cleanup: if (vm) virObjectUnlock(vm); VIR_FREE(name); +virObjectUnref(cfg); return ret; } -- 1.8.3.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 2/2] qemu: snapshot: Add support for compressing external snapshot memory
The regular save image code has the support to compress images using a specified algorithm. This was not implemented for external checkpoints although it shares most of the backend code. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1017227 --- src/qemu/qemu.conf | 6 ++ src/qemu/qemu_conf.c | 2 ++ src/qemu/qemu_conf.h | 1 + src/qemu/qemu_driver.c | 23 +-- 4 files changed, 30 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index cf82ffe..7ca1761 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -278,8 +278,14 @@ # the requested compression program can't be found, this falls # back to raw compression. # +# snapshot_image_format specifies the compression algorithm of the memory save +# image when an external snapshot of a domain is taken. This does not apply +# on disk image format. It is an error if the specified format isn't valid, +# or the requested compression program can't be found. +# #save_image_format = raw #dump_image_format = raw +#snapshot_image_format = raw # When a domain is configured to be auto-dumped when libvirtd receives a # watchdog event from qemu guest, libvirtd will save dump files in directory diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 1a41caf..d6e8519 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -521,6 +521,8 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, GET_VALUE_STR(save_image_format, cfg-saveImageFormat); GET_VALUE_STR(dump_image_format, cfg-dumpImageFormat); +GET_VALUE_STR(snapshot_image_format, cfg-snapshotImageFormat); + GET_VALUE_STR(auto_dump_path, cfg-autoDumpPath); GET_VALUE_BOOL(auto_dump_bypass_cache, cfg-autoDumpBypassCache); GET_VALUE_BOOL(auto_start_bypass_cache, cfg-autoStartBypassCache); diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index da29a2a..09dca51 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -144,6 +144,7 @@ struct _virQEMUDriverConfig { char *saveImageFormat; char *dumpImageFormat; +char *snapshotImageFormat; char *autoDumpPath; bool autoDumpBypassCache; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6f15240..a1c4ed9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12116,6 +12116,8 @@ qemuDomainSnapshotCreateActiveExternal(virConnectPtr conn, bool transaction = virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_TRANSACTION); int thaw = 0; /* 1 if freeze succeeded, -1 if freeze failed */ bool pmsuspended = false; +virQEMUDriverConfigPtr cfg = NULL; +int compressed = QEMU_SAVE_FORMAT_RAW; if (qemuDomainObjBeginAsyncJob(driver, vm, QEMU_ASYNC_JOB_SNAPSHOT) 0) goto cleanup; @@ -12177,12 +12179,28 @@ qemuDomainSnapshotCreateActiveExternal(virConnectPtr conn, JOB_MASK(QEMU_JOB_SUSPEND) | JOB_MASK(QEMU_JOB_MIGRATION_OP)); +cfg = virQEMUDriverGetConfig(driver); +if (cfg-snapshotImageFormat) { +compressed = qemuSaveCompressionTypeFromString(cfg-snapshotImageFormat); +if (compressed 0) { +virReportError(VIR_ERR_OPERATION_FAILED, %s, + _(Invalid snapshot image format specified + in configuration file)); +goto cleanup; +} +if (!qemuCompressProgramAvailable(compressed)) { +virReportError(VIR_ERR_OPERATION_FAILED, %s, + _(Compression program for image format + in configuration file isn't available)); +goto cleanup; +} +} + if (!(xml = qemuDomainDefFormatLive(driver, vm-def, true, true))) goto endjob; if ((ret = qemuDomainSaveMemory(driver, vm, snap-def-file, -xml, QEMU_SAVE_FORMAT_RAW, -resume, 0, +xml, compressed, resume, 0, QEMU_ASYNC_JOB_SNAPSHOT)) 0) goto endjob; @@ -12271,6 +12289,7 @@ endjob: cleanup: VIR_FREE(xml); +virObjectUnref(cfg); if (memory_unlink ret 0) unlink(snap-def-file); -- 1.8.3.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 3/4] qemu: Implement support for VIR_MIGRATE_PARAM_LISTEN_ADDRESS
On 10/09/2013 03:32 PM, Michal Privoznik wrote: Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/qemu/qemu_driver.c| 26 +++ src/qemu/qemu_migration.c | 113 +++--- src/qemu/qemu_migration.h | 13 -- 3 files changed, 103 insertions(+), 49 deletions(-) @@ -2260,31 +2254,66 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, if (VIR_STRDUP(migrateFrom, stdio) 0) goto cleanup; } else { +virSocketAddr listenAddressSocket; +bool encloseAddress = false; +bool hostIPv6Capable = false; +bool qemuIPv6Capable = false; virQEMUCapsPtr qemuCaps = NULL; struct addrinfo *info = NULL; struct addrinfo hints = { .ai_flags = AI_ADDRCONFIG, .ai_socktype = SOCK_STREAM }; +if (getaddrinfo(::, NULL, hints, info) == 0) { +freeaddrinfo(info); +hostIPv6Capable = true; +} if (!(qemuCaps = virQEMUCapsCacheLookupCopy(driver-qemuCapsCache, (*def)-emulator))) goto cleanup; -/* Listen on :: instead of 0.0.0.0 if QEMU understands it - * and there is at least one IPv6 address configured - */ -if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_IPV6_MIGRATION) -getaddrinfo(::, NULL, hints, info) == 0) { -freeaddrinfo(info); -listenAddr = [::]; +qemuIPv6Capable = virQEMUCapsGet(qemuCaps, QEMU_CAPS_IPV6_MIGRATION); +virObjectUnref(qemuCaps); + +if (listenAddress) { +if (virSocketAddrIsNumeric(listenAddress)) { +/* listenAddress is numeric IPv4 or IPv6 */ +if (virSocketAddrParse(listenAddressSocket, listenAddress, AF_UNSPEC) 0) +goto cleanup; + +/* address parsed successfully */ +if (VIR_SOCKET_ADDR_IS_FAMILY(listenAddressSocket, AF_INET6)) { +if (!qemuIPv6Capable) { +virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s, + _(qemu isn't capable of IPv6)); +goto cleanup; +} +if (!hostIPv6Capable) { +virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s, + _(host isn't capable of IPv6)); +goto cleanup; +} +} + +/* IPv6 address must be enclosed in angle brackets on the cmd line */ +encloseAddress = true; This sets it for IPv4 as well. Also s/ angle// (and maybe 'escape' would be better than 'enclose') +} else { +/* listenAddress is a hostname */ +} } else { -listenAddr = 0.0.0.0; +/* Listen on :: instead of 0.0.0.0 if QEMU understands it + * and there is at least one IPv6 address configured + */ +listenAddress = qemuIPv6Capable hostIPv6Capable ? +encloseAddress = true, :: : 0.0.0.0; } -virObjectUnref(qemuCaps); -/* QEMU will be started with -incoming [::]:port - * or -incoming 0.0.0.0:port +/* QEMU will be started with -incoming [IPv6 addr]:port, + * -incoming IPv4 addr:port or -incoming hostname:port */ -if (virAsprintf(migrateFrom, tcp:%s:%d, listenAddr, port) 0) +if ((encloseAddress + virAsprintf(migrateFrom, tcp:[%s]:%d, listenAddress, port) 0) || +(!encloseAddress + virAsprintf(migrateFrom, tcp:%s:%d, listenAddress, port) 0)) goto cleanup; } ACK with that fixed -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 1/4] Migration: Introduce VIR_MIGRATE_PARAM_LISTEN_ADDRESS
On 10/09/2013 03:32 PM, Michal Privoznik wrote: --- a/tools/virsh.pod +++ b/tools/virsh.pod +Optional Ilisten-address sets the listen address that hypervisor on the +destination side should bind to for incoming migration. Both, IPv4 and IPv6 Redundant comma. --^ +addresses are accepted as well as hostnames (the resolving is done on +destination). Some hypervisors do not support this feature an will return an +error if this parameter is used. + =item Bmigrate-setmaxdowntime Idomain Idowntime Set maximum tolerable downtime for a domain which is being live-migrated to -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 4/4] qemu_conf: Introduce migration_address
On 10/09/2013 03:32 PM, Michal Privoznik wrote: This configuration knob is there to override default listen address for -incoming for all qemu domains. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/qemu/libvirtd_qemu.aug | 3 +++ src/qemu/qemu.conf | 6 ++ src/qemu/qemu_conf.c | 2 ++ src/qemu/qemu_conf.h | 3 +++ src/qemu/qemu_driver.c | 7 +++ src/qemu/test_libvirtd_qemu.aug.in | 1 + 6 files changed, 22 insertions(+) diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index cd13d53..591d78d 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -78,6 +78,8 @@ module Libvirtd_qemu = | int_entry keepalive_interval | int_entry keepalive_count + let network_entry = str_entry migration_address + (* Each entry in the config is one of the following ... *) let entry = vnc_entry | spice_entry @@ -88,6 +90,7 @@ module Libvirtd_qemu = | process_entry | device_entry | rpc_entry + | network_entry let comment = [ label #comment . del /#[ \t]*/ # . store /([^ \t\n][^\n]*)?/ . del /\n/ \n ] let empty = [ label #empty . eol ] diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 5fd6263..4403aaf 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -424,3 +424,9 @@ # Defaults to -1. # #seccomp_sandbox = 1 + + + +# Override the listen address for all incoming migrations. Defaults to +# 0.0.0.0 or :: in case if both host and qemu are capable of IPv6. +#migration_address = 127.0.0.1 diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 1a41caf..16a0b92 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -546,6 +546,8 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, GET_VALUE_LONG(seccomp_sandbox, cfg-seccompSandbox); +GET_VALUE_STR(migration_address, cfg-migrationAddress); + ret = 0; cleanup: diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index da29a2a..40adfce 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -155,6 +155,9 @@ struct _virQEMUDriverConfig { unsigned int keepAliveCount; int seccompSandbox; + +/* The default for -incoming */ +char *migrationAddress; }; /* Main driver state */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e12a1de..7d87b2d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10264,6 +10264,7 @@ qemuDomainMigratePrepare3Params(virConnectPtr dconn, unsigned int flags) { virQEMUDriverPtr driver = dconn-privateData; +virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); You'll leak a reference to cfg if @flags or @params are invalid. virDomainDefPtr def = NULL; const char *dom_xml = NULL; const char *dname = NULL; @@ -10306,6 +10307,11 @@ qemuDomainMigratePrepare3Params(virConnectPtr dconn, if (virDomainMigratePrepare3ParamsEnsureACL(dconn, def) 0) goto cleanup; +/* If no listenAddress has been passed via @params, + * use the one from qemu.conf */ +if (!listenAddress) +listenAddress = cfg-migrationAddress; + You can initialize listenAddress to this instead of NULL. ACK with the leak fixed. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 2/4] virsocket: Introduce virSocketAddrIsWildcard
On 10/09/2013 03:32 PM, Michal Privoznik wrote: This function takes exactly one argument: an address to check. It returns true, iff the address is an IPv4 or IPv6 address in numeric Why use iff if you still need to say what happens otherwise? :) format, false if otherwise (e.g. for examplehost). s/ if// @@ -824,3 +839,27 @@ virSocketAddrGetIpPrefix(const virSocketAddrPtr address, */ return 0; } Missing space. +/** + * virSocketAddrIsNumeric: + * @address: address to check + * + * Check if passed address is an IP address in numeric format. For + * instance, for 0.0.0.0 true is returned, for 'examplehost + * false is returned. + * + * Returns: true iff @address is an IP address, See my comment for the commit message. + * false otherwise + */ +bool +virSocketAddrIsNumeric(const char *address) +{ +struct addrinfo *res; +unsigned short family; + +if (virSocketAddrParseInternal(res, address, AF_UNSPEC, false) 0) +return false; + +family = res-ai_addr-sa_family; Is this any different than using int res-ai_family? +freeaddrinfo(res); +return family == AF_INET || family == AF_INET6; +} ACK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 1/4] Migration: Introduce VIR_MIGRATE_PARAM_LISTEN_ADDRESS
On 10/10/2013 07:52 AM, Ján Tomko wrote: On 10/09/2013 03:32 PM, Michal Privoznik wrote: --- a/tools/virsh.pod +++ b/tools/virsh.pod +Optional Ilisten-address sets the listen address that hypervisor on the +destination side should bind to for incoming migration. Both, IPv4 and IPv6 Redundant comma. --^ +addresses are accepted as well as hostnames (the resolving is done on +destination). Some hypervisors do not support this feature an will return an s/an will/and will/ +error if this parameter is used. + =item Bmigrate-setmaxdowntime Idomain Idowntime Set maximum tolerable downtime for a domain which is being live-migrated to -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 2/2] storage: btrfs subvolumes for directory pools
This commit adds support for btrfs subvolumes as directory volumes. The directory storage pool can be used to manage btrfs subvolumes on an existing btrfs filesystem. The driver can create new blank subvolumes and snapshots of existing subvolumes as well as delete existing subvolumes. The subvolumes created are automatically made visible on the host side and can be attached to domains using the filesystem tags as defined in 'format domain' documentation. Subvolumes do not implement quotas at the moment because the current (btrfs-progs-0.20.rc1.20130501git7854c8b-4.fc20.x86_64) support for quota management in btrfs-progs is lacking the necessary features, for example it's not possible to see the quota assigned to a certain subvolume and usage information is only updated on syncfs(2). Quota support will be implemented once the tools gain the necessary features. Signed-off-by: Oskari Saarenmaa o...@ohmu.fi --- I did not add a new virDirIsExecutable function as it wasn't really needed by this patch set, it's sufficient to just verify that the backing volume is a directory before running btrfs subvolume snapshot. Part 1/2 still applies cleanly on today's git master so not resending it. configure.ac | 25 ++- docs/schemas/storagevol.rng | 1 + docs/storage.html.in | 30 ++- libvirt.spec.in | 4 + src/conf/storage_conf.c | 3 + src/conf/storage_conf.h | 1 + src/storage/storage_backend.c | 15 +- src/storage/storage_backend_fs.c | 219 -- src/util/virstoragefile.c | 4 +- src/util/virstoragefile.h | 1 + tests/storagevolxml2xmlin/vol-btrfs-snapshot.xml | 13 ++ tests/storagevolxml2xmlin/vol-btrfs.xml | 9 + tests/storagevolxml2xmlout/vol-btrfs-snapshot.xml | 26 +++ tests/storagevolxml2xmlout/vol-btrfs.xml | 17 ++ tests/storagevolxml2xmltest.c | 2 + 15 files changed, 340 insertions(+), 30 deletions(-) create mode 100644 tests/storagevolxml2xmlin/vol-btrfs-snapshot.xml create mode 100644 tests/storagevolxml2xmlin/vol-btrfs.xml create mode 100644 tests/storagevolxml2xmlout/vol-btrfs-snapshot.xml create mode 100644 tests/storagevolxml2xmlout/vol-btrfs.xml diff --git a/configure.ac b/configure.ac index 1993fab..a82640c 100644 --- a/configure.ac +++ b/configure.ac @@ -1646,6 +1646,10 @@ AC_ARG_WITH([storage-sheepdog], [AS_HELP_STRING([--with-storage-sheepdog], [with Sheepdog backend for the storage driver @:@default=check@:@])], [],[with_storage_sheepdog=check]) +AC_ARG_WITH([storage-btrfs], + [AS_HELP_STRING([--with-storage-btrfs], +[with Btrfs backend for the storage driver @:@default=check@:@])], + [],[with_storage_btrfs=check]) if test $with_libvirtd = no; then with_storage_dir=no @@ -1858,6 +1862,24 @@ fi AM_CONDITIONAL([WITH_STORAGE_SHEEPDOG], [test $with_storage_sheepdog = yes]) +if test $with_storage_btrfs = yes || test $with_storage_btrfs = check; then + AC_PATH_PROG([BTRFS], [btrfs], [], [$PATH:/sbin:/usr/sbin]) + + if test $with_storage_btrfs = yes ; then +if test -z $BTRFS ; then AC_MSG_ERROR([We need btrfs -command for btrfs storage driver]) ; fi + else +if test -z $BTRFS ; then with_storage_btrfs=no ; fi +if test $with_storage_btrfs = check ; then with_storage_btrfs=yes ; fi + fi + + if test $with_storage_btrfs = yes ; then +AC_DEFINE_UNQUOTED([WITH_STORAGE_BTRFS], 1, [whether Btrfs backend for storage driver is enabled]) +AC_DEFINE_UNQUOTED([BTRFS],[$BTRFS],[Location of btrfs program]) + fi +fi +AM_CONDITIONAL([WITH_STORAGE_BTRFS], [test $with_storage_btrfs = yes]) + + LIBPARTED_CFLAGS= LIBPARTED_LIBS= @@ -1945,7 +1967,7 @@ AC_SUBST([DEVMAPPER_CFLAGS]) AC_SUBST([DEVMAPPER_LIBS]) with_storage=no -for backend in dir fs lvm iscsi scsi mpath rbd disk; do +for backend in dir fs lvm iscsi scsi mpath rbd disk btrfs; do if eval test \$with_storage_$backend = yes; then with_storage=yes break @@ -2670,6 +2692,7 @@ AC_MSG_NOTICE([ mpath: $with_storage_mpath]) AC_MSG_NOTICE([Disk: $with_storage_disk]) AC_MSG_NOTICE([ RBD: $with_storage_rbd]) AC_MSG_NOTICE([Sheepdog: $with_storage_sheepdog]) +AC_MSG_NOTICE([ Btrfs: $with_storage_btrfs]) AC_MSG_NOTICE([]) AC_MSG_NOTICE([Security Drivers]) AC_MSG_NOTICE([]) diff --git a/docs/schemas/storagevol.rng b/docs/schemas/storagevol.rng index 8bc5907..8814973 100644 --- a/docs/schemas/storagevol.rng +++ b/docs/schemas/storagevol.rng @@ -201,6 +201,7 @@ valueqed/value valuevmdk/value valuevpc/value + valuevolume/value /choice /define diff --git a/docs/storage.html.in b/docs/storage.html.in index 1181444..03018ab 100644 --- a/docs/storage.html.in +++ b/docs/storage.html.in @@ -119,12
Re: [libvirt] [PATCH v2 0/5] qemumonitorjsontest: Introduce some tests
On 10/10/2013 01:43 PM, Michal Privoznik wrote: On 03.10.2013 12:49, Michal Privoznik wrote: v2 to some patches that weren't ACKed yet. Michal Privoznik (5): qemuMonitorTestFree: Join worker thread qemumonitorjsontest: Extend the test for yet another monitor commands qemumonitorjsontest: Test qemuMonitorJSONGetCPUInfo qemumonitorjsontest: Test qemuMonitorJSONGetVirtType qemumonitorjsontest: Test qemuMonitorJSONSendKey tests/qemumonitorjsontest.c | 253 +++ tests/qemumonitortestutils.c | 5 +- 2 files changed, 256 insertions(+), 2 deletions(-) Ping? 1/5 is ACked and pushed, but what about the others? ACK series -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 2/2] storage: btrfs subvolumes for directory pools
On Thu, Oct 10, 2013 at 05:52:22PM +0300, Oskari Saarenmaa wrote: This commit adds support for btrfs subvolumes as directory volumes. The directory storage pool can be used to manage btrfs subvolumes on an existing btrfs filesystem. The driver can create new blank subvolumes and snapshots of existing subvolumes as well as delete existing subvolumes. The subvolumes created are automatically made visible on the host side and can be attached to domains using the filesystem tags as defined in 'format domain' documentation. Subvolumes do not implement quotas at the moment because the current (btrfs-progs-0.20.rc1.20130501git7854c8b-4.fc20.x86_64) support for quota management in btrfs-progs is lacking the necessary features, for example it's not possible to see the quota assigned to a certain subvolume and usage information is only updated on syncfs(2). Quota support will be implemented once the tools gain the necessary features. Signed-off-by: Oskari Saarenmaa o...@ohmu.fi --- I did not add a new virDirIsExecutable function as it wasn't really needed by this patch set, it's sufficient to just verify that the backing volume is a directory before running btrfs subvolume snapshot. Part 1/2 still applies cleanly on today's git master so not resending it. configure.ac | 25 ++- docs/schemas/storagevol.rng | 1 + docs/storage.html.in | 30 ++- libvirt.spec.in | 4 + src/conf/storage_conf.c | 3 + src/conf/storage_conf.h | 1 + src/storage/storage_backend.c | 15 +- src/storage/storage_backend_fs.c | 219 -- src/util/virstoragefile.c | 4 +- src/util/virstoragefile.h | 1 + tests/storagevolxml2xmlin/vol-btrfs-snapshot.xml | 13 ++ tests/storagevolxml2xmlin/vol-btrfs.xml | 9 + tests/storagevolxml2xmlout/vol-btrfs-snapshot.xml | 26 +++ tests/storagevolxml2xmlout/vol-btrfs.xml | 17 ++ tests/storagevolxml2xmltest.c | 2 + 15 files changed, 340 insertions(+), 30 deletions(-) create mode 100644 tests/storagevolxml2xmlin/vol-btrfs-snapshot.xml create mode 100644 tests/storagevolxml2xmlin/vol-btrfs.xml create mode 100644 tests/storagevolxml2xmlout/vol-btrfs-snapshot.xml create mode 100644 tests/storagevolxml2xmlout/vol-btrfs.xml diff --git a/configure.ac b/configure.ac index 1993fab..a82640c 100644 --- a/configure.ac +++ b/configure.ac @@ -1646,6 +1646,10 @@ AC_ARG_WITH([storage-sheepdog], [AS_HELP_STRING([--with-storage-sheepdog], [with Sheepdog backend for the storage driver @:@default=check@:@])], [],[with_storage_sheepdog=check]) +AC_ARG_WITH([storage-btrfs], + [AS_HELP_STRING([--with-storage-btrfs], +[with Btrfs backend for the storage driver @:@default=check@:@])], + [],[with_storage_btrfs=check]) We no longer have a backend for btrfs - it is just a part of the fs driver. if test $with_libvirtd = no; then with_storage_dir=no @@ -1858,6 +1862,24 @@ fi AM_CONDITIONAL([WITH_STORAGE_SHEEPDOG], [test $with_storage_sheepdog = yes]) +if test $with_storage_btrfs = yes || test $with_storage_btrfs = check; then + AC_PATH_PROG([BTRFS], [btrfs], [], [$PATH:/sbin:/usr/sbin]) + + if test $with_storage_btrfs = yes ; then +if test -z $BTRFS ; then AC_MSG_ERROR([We need btrfs -command for btrfs storage driver]) ; fi + else +if test -z $BTRFS ; then with_storage_btrfs=no ; fi +if test $with_storage_btrfs = check ; then with_storage_btrfs=yes ; fi + fi + + if test $with_storage_btrfs = yes ; then +AC_DEFINE_UNQUOTED([WITH_STORAGE_BTRFS], 1, [whether Btrfs backend for storage driver is enabled]) +AC_DEFINE_UNQUOTED([BTRFS],[$BTRFS],[Location of btrfs program]) + fi +fi +AM_CONDITIONAL([WITH_STORAGE_BTRFS], [test $with_storage_btrfs = yes]) Just use 'WITH_BTRFS' here - the WITH_STORAGE_ is for actual storage backends. + + LIBPARTED_CFLAGS= LIBPARTED_LIBS= @@ -1945,7 +1967,7 @@ AC_SUBST([DEVMAPPER_CFLAGS]) AC_SUBST([DEVMAPPER_LIBS]) with_storage=no -for backend in dir fs lvm iscsi scsi mpath rbd disk; do +for backend in dir fs lvm iscsi scsi mpath rbd disk btrfs; do if eval test \$with_storage_$backend = yes; then with_storage=yes break @@ -2670,6 +2692,7 @@ AC_MSG_NOTICE([ mpath: $with_storage_mpath]) AC_MSG_NOTICE([Disk: $with_storage_disk]) AC_MSG_NOTICE([ RBD: $with_storage_rbd]) AC_MSG_NOTICE([Sheepdog: $with_storage_sheepdog]) +AC_MSG_NOTICE([ Btrfs: $with_storage_btrfs]) Again not needed here. AC_MSG_NOTICE([]) AC_MSG_NOTICE([Security Drivers]) AC_MSG_NOTICE([]) diff --git a/docs/schemas/storagevol.rng b/docs/schemas/storagevol.rng index
[libvirt] [PATCH] Move virNetDevVPort enum impl into virnetdevvportprofile.c
From: Daniel P. Berrange berra...@redhat.com The enum for virNetDevVPort is declared in the header file virnetdevvportprofile.h, but for some reason the impl is in netdev_vport_profile_conf.c. This causes a dep from src/util onto src/conf which is not allowed. Move the enum impl into virnetdevvportprofile.c to break the circle. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/conf/netdev_vport_profile_conf.c | 7 --- src/util/virnetdevvportprofile.c | 6 ++ 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/conf/netdev_vport_profile_conf.c b/src/conf/netdev_vport_profile_conf.c index e8199e2..a16a04a 100644 --- a/src/conf/netdev_vport_profile_conf.c +++ b/src/conf/netdev_vport_profile_conf.c @@ -30,13 +30,6 @@ #define VIR_FROM_THIS VIR_FROM_NONE -VIR_ENUM_IMPL(virNetDevVPort, VIR_NETDEV_VPORT_PROFILE_LAST, - none, - 802.1Qbg, - 802.1Qbh, - openvswitch) - - virNetDevVPortProfilePtr virNetDevVPortProfileParse(xmlNodePtr node, unsigned int flags) { diff --git a/src/util/virnetdevvportprofile.c b/src/util/virnetdevvportprofile.c index 07155b9..1cae20a 100644 --- a/src/util/virnetdevvportprofile.c +++ b/src/util/virnetdevvportprofile.c @@ -29,6 +29,12 @@ #define VIR_FROM_THIS VIR_FROM_NET +VIR_ENUM_IMPL(virNetDevVPort, VIR_NETDEV_VPORT_PROFILE_LAST, + none, + 802.1Qbg, + 802.1Qbh, + openvswitch) + VIR_ENUM_IMPL(virNetDevVPortProfileOp, VIR_NETDEV_VPORT_PROFILE_OP_LAST, create, save, -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 2/2] storage: btrfs subvolumes for directory pools
Thanks for the review, 10.10.2013 18:57, Daniel P. Berrange kirjoitti: On Thu, Oct 10, 2013 at 05:52:22PM +0300, Oskari Saarenmaa wrote: +if test $with_storage_btrfs = yes || test $with_storage_btrfs = check; then + AC_PATH_PROG([BTRFS], [btrfs], [], [$PATH:/sbin:/usr/sbin]) [...] +AM_CONDITIONAL([WITH_STORAGE_BTRFS], [test $with_storage_btrfs = yes]) Just use 'WITH_BTRFS' here - the WITH_STORAGE_ is for actual storage backends. Do we actually need a flag for enabling / disabling btrfs usage as it's not a separate storage pool? I'm thinking about just calling AC_PATH_PROG(btrfs) if FS pool is enabled, and using #ifdef BTRFS in the fs code to use btrfs if available. --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -90,6 +90,7 @@ struct _virStorageVolTarget { virStorageEncryptionPtr encryption; virBitmapPtr features; char *compat; +char *uuid; }; This struct represents the public XML data format - adding random fields to it for individual driver use is not allowed. Ok, I'll put this thing somewhere else. +if (volv-backingStore.path == NULL) { +/* backing store not in the pool, ignore it */ Backing stores for volumes are not required to be in the same pool as the source. For example it is valid to have a qcow2 file backed by a lvm volume. In btrfs's case the note about backing store in an existing subvolume is informational only; all subvolumes, snapshots or not, are independent and in case we can't find a symbolic name for the parent volume I think it's safe to just ignore it. I'll add this note to the comment. I'm thinking it would be nice to have a dedicate file with APIs for btrfs eg src/util/virbtrfs.{h,c} and have it contain things like virBtrFSCreateVolume() virBtrFSCreateVolume() virBtrFS...() and so on, so we don't have all these virCommand calls littering this file. I'm not sure it makes sense to do this until there's another user for btrfs commands in libvirt, but I can do it if you like. I think it'd be more useful to have a module which implements copy-on-write operations for different filesystems (btrfs, zfs, something else?) with a common api. diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index d5effa4..4dc6c81 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -46,6 +46,7 @@ enum virStorageFileFormat { VIR_STORAGE_FILE_FAT, VIR_STORAGE_FILE_VHD, VIR_STORAGE_FILE_VDI, +VIR_STORAGE_FILE_VOLUME, This isn't right - volumes already have a type={file,block,dir,volume} and we shouldn't duplicate this in the format attribute too. Hmm.. I don't see such a field in virStorageVolType - am I looking at a wrong enum? diff --git a/tests/storagevolxml2xmlin/vol-btrfs-snapshot.xml b/tests/storagevolxml2xmlin/vol-btrfs-snapshot.xml new file mode 100644 index 000..5a58b56 --- /dev/null +++ b/tests/storagevolxml2xmlin/vol-btrfs-snapshot.xml @@ -0,0 +1,13 @@ +volume + nameclone/name + capacity unit=bytes0/capacity + allocation unit=bytes0/allocation + target +path/lxc/clone/path +format type='volume'/ + /target + backingStore +path/lxc/vanilla/path +format type='volume'/ + /backingStore +/volume Using 'format' in this way is wrong. What we should be doing is exposing the volume 'type' as an attribute eg volume type='volume' ... /volume I suppose that makes sense, but it would make this incompatible with existing volume creation. Right now (with the current patch) I can use existing virsh tooling to run, for example 'virsh vol-create-as default guest-1 0 --format volume --backing-store guest-base' diff --git a/tests/storagevolxml2xmlout/vol-btrfs-snapshot.xml b/tests/storagevolxml2xmlout/vol-btrfs-snapshot.xml new file mode 100644 index 000..75830d3 --- /dev/null +++ b/tests/storagevolxml2xmlout/vol-btrfs-snapshot.xml @@ -0,0 +1,26 @@ +volume + nameclone/name + key(null)/key If you're getting (null) here, then something has gone wrong I'll look into this. Thanks, Oskari -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 2/2] storage: btrfs subvolumes for directory pools
On 10/10/2013 09:57 AM, Daniel P. Berrange wrote: +} +if (volv-backingStore.path == NULL) { +/* backing store not in the pool, ignore it */ Backing stores for volumes are not required to be in the same pool as the source. For example it is valid to have a qcow2 file backed by a lvm volume. What's more, qemu allows one to have qcow2 data atop a network device; but right now libvirt forcefully assumes that all network devices (nbd, ceph, sheepdog, gluster) contain only raw data rather than other formats. I'm trying to investigate what implications this has, but among other things, I think we need to use more polymorphic callbacks where for a given backing store string, we determine which storage driver to use, then that storage driver tells us how to manage data from that storage (including having qcow2 data atop network storage). -- 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] Move virNetDevVPort enum impl into virnetdevvportprofile.c
On 10/10/2013 10:30 AM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com The enum for virNetDevVPort is declared in the header file virnetdevvportprofile.h, but for some reason the impl is in netdev_vport_profile_conf.c. This causes a dep from src/util onto src/conf which is not allowed. Move the enum impl into virnetdevvportprofile.c to break the circle. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/conf/netdev_vport_profile_conf.c | 7 --- src/util/virnetdevvportprofile.c | 6 ++ 2 files changed, 6 insertions(+), 7 deletions(-) ACK. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] VMware: Simplify array walk for driver type
Rather than walking the possible driver backends by handle, use a helper function. Additionally I've done a bit of refactoring in the code over the past few commits so add myself to the copyright line. --- src/vmware/vmware_driver.c | 10 +++--- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c index 25adb15..79954e0 100644 --- a/src/vmware/vmware_driver.c +++ b/src/vmware/vmware_driver.c @@ -2,6 +2,7 @@ /* * Copyright (C) 2011-2012 Red Hat, Inc. * Copyright 2010, diateam (www.diateam.net) + * Copyright (C) 2013. Doug Goldstein car...@cardoe.com * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -156,13 +157,8 @@ vmwareConnectOpen(virConnectPtr conn, goto cleanup; } -driver-type = -1; -for (i = 0; i VMWARE_DRIVER_LAST; i++) { -if (STREQ(tmp, vmwareDriverTypeToString(i))) { -driver-type = i; -break; -} -} +/* Match the non-'vmware' part of the scheme as the driver backend */ +driver-type = vmwareDriverTypeFromString(tmp); if (driver-type == -1) { virReportError(VIR_ERR_INTERNAL_ERROR, _(unable to find valid -- 1.8.1.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] VMware: Do version detection earlier
Do VMware version detection earlier as future patches will need the version information to populate capabilities correctly. --- src/vmware/vmware_driver.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c index 79954e0..0e7a37f 100644 --- a/src/vmware/vmware_driver.c +++ b/src/vmware/vmware_driver.c @@ -166,6 +166,9 @@ vmwareConnectOpen(virConnectPtr conn, goto cleanup; } +if (vmwareExtractVersion(driver) 0) +goto cleanup; + if (!(driver-domains = virDomainObjListNew())) goto cleanup; @@ -178,9 +181,6 @@ vmwareConnectOpen(virConnectPtr conn, if (vmwareLoadDomains(driver) 0) goto cleanup; -if (vmwareExtractVersion(driver) 0) -goto cleanup; - conn-privateData = driver; return VIR_DRV_OPEN_SUCCESS; -- 1.8.1.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC] net-dhcp-leases: virNetworkDHCPLeases struct design
On 10/08/2013 06:39 AM, Nehal J Wani wrote: Continued from https://www.redhat.com/archives/libvir-list/2013-October/msg00325.html Since no one likes the idea of having a union, but we do want to support dhcpv6, following is the new design: struct _virNetworkDHCPLeases { char *interface;/* Network interface name (non-null) */ long long expirytime; /* Seconds since epoch (non-null) */ (non-null) makes no sense for an integer. On the other hand, should we document a sentinel for unknown (or infinite) lease expiration? int type; /* virIPAddrType (non-null) */ (non-null) makes no sense for an integer. unsigned int prefix;/* IP address prefix (non-null) */ (non-null) makes no sense for an integer; should we document that 0 means unknown? char *mac; /* MAC address (mostly non-null, except rare cases) */ I guess it could be NULL for the generic virNetworkDHCPLeases query; but I don't see how it can possibly be NULL for the specific per-MAC virNetworkDHCPLeases ForMAC(). Probably also worth documenting that it is in ASCII form (not raw bytes). If you implement it as remote_string on the RPC side, then you are guaranteeing that it is non-NULL; are we hurting ourselves if we make that guarantee? char *iaid; /* IAID (ipv6) (null, in case of ipv4) */ char *ipaddr; /* IP address (non-null) */ Maybe mention that it is in ASCII form, not raw bytes (maybe mention that once, at the top of the struct, since it now applies to both MAC and IP strings). char *hostname; /* Hostname (can be null) */ char *clientid; /* Client ID(ipv4) or DUID(ipv6) (can be null, in case of ipv4) */ }; Seems like a reasonable struct to me. I'm not an IPv6 expert, if anyone else wants to chime in, but it's certainly looking better for use in a v5 series than what we were debating in v4. -- 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] esx: Fix floppy.fileName handling in the vmx file parser
On 10/09/2013 07:39 PM, Geoff Hickey wrote: The vmx file parsing code was reporting errors when parsing floppy.fileName entries if the filename didn't end in .flp. There is no such restriction in ESX; even using the GUI to configure floppy filenames you can specify any arbitrary file with any extension. Fix by changing the vmx parsing code so that it uses the floppy.fileType value to determine whether floppy.fileName refers to a block device or a regular file. --- src/vmx/vmx.c | 26 +- 1 file changed, 5 insertions(+), 21 deletions(-) diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 38b7cc0..ffe7e7a 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -2250,27 +2250,18 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con goto cleanup; } } else if (device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) { -if (virFileHasSuffix(fileName, .flp)) { -if (fileType != NULL) { -if (STRCASENEQ(fileType, file)) { -virReportError(VIR_ERR_INTERNAL_ERROR, - _(Expecting VMX entry '%s' to be 'file' but Your mailer corrupted your patch. Would you mind resending it via 'git send-email', or at a bare minimum as an attachment rather than inline, so that maintainers can just 'git am' it? Applying: esx: Fix floppy.fileName handling in the vmx file parser fatal: corrupt patch at line 10 Repository lacks necessary blobs to fall back on 3-way merge. Cannot fall back to three-way merge. Patch failed at 0001 esx: Fix floppy.fileName handling in the vmx file parser See also http://libvirt.org/hacking.html for hints. +if (fileType != NULL STRCASEEQ(fileType, device)) { It's simpler to just use a pointer variable in boolean context instead of explicit comparison against NULL (this isn't Java). As in: if (fileType STRCASEEQ(fileType, device)) { At any rate, your change looks reasonable to me; I wouldn't mind applying it if you can resend to touch up those issues. -- 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] qemu: Introduce qemuDomainDefCheckABIStability
On 10/10/2013 03:23 AM, Michal Privoznik wrote: https://bugzilla.redhat.com/show_bug.cgi?id=994364 Whenever we check for ABI stability, we have new xml (e.g. provided by user, or obtained from snapshot, whatever) which we compare to old xml and see if ABI won't break. However, if the new xml was produced via virDomainGetXMLDesc(..., VIR_DOMAIN_XML_MIGRATABLE) it lacks some devices, e.g. 'pci-root' controller. Hence, the ABI stability check fails even though it is stable. Moreover, we can't simply fix virDomainDefCheckABIStability because removing the correct devices is task for the driver. For instance, qemu driver wants to remove the usb controller too, while LXC driver doesn't. That's why we need special qemu wrapper over virDomainDefCheckABIStability which removes the correct devices from domain XML, produces MIGRATABLE xml and calls the check ABI stability function. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/qemu/qemu_domain.c| 22 ++ src/qemu/qemu_domain.h| 3 +++ src/qemu/qemu_driver.c| 10 ++ src/qemu/qemu_migration.c | 4 ++-- 4 files changed, 29 insertions(+), 10 deletions(-) Yes, this will make life simpler for all callers. 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] [PATCH] esx: Fix floppy.fileName handling in the vmx file parser
On Thu, Oct 10, 2013 at 2:21 PM, Eric Blake ebl...@redhat.com wrote: On 10/09/2013 07:39 PM, Geoff Hickey wrote: The vmx file parsing code was reporting errors when parsing floppy.fileName entries if the filename didn't end in .flp. There is no such restriction in ESX; even using the GUI to configure floppy filenames you can specify any arbitrary file with any extension. Fix by changing the vmx parsing code so that it uses the floppy.fileType value to determine whether floppy.fileName refers to a block device or a regular file. --- src/vmx/vmx.c | 26 +- 1 file changed, 5 insertions(+), 21 deletions(-) diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 38b7cc0..ffe7e7a 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -2250,27 +2250,18 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con goto cleanup; } } else if (device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) { -if (virFileHasSuffix(fileName, .flp)) { -if (fileType != NULL) { -if (STRCASENEQ(fileType, file)) { -virReportError(VIR_ERR_INTERNAL_ERROR, - _(Expecting VMX entry '%s' to be 'file' but Your mailer corrupted your patch. Would you mind resending it via 'git send-email', or at a bare minimum as an attachment rather than inline, so that maintainers can just 'git am' it? Applying: esx: Fix floppy.fileName handling in the vmx file parser fatal: corrupt patch at line 10 Repository lacks necessary blobs to fall back on 3-way merge. Cannot fall back to three-way merge. Patch failed at 0001 esx: Fix floppy.fileName handling in the vmx file parser See also http://libvirt.org/hacking.html for hints. +if (fileType != NULL STRCASEEQ(fileType, device)) { It's simpler to just use a pointer variable in boolean context instead of explicit comparison against NULL (this isn't Java). As in: if (fileType STRCASEEQ(fileType, device)) { At any rate, your change looks reasonable to me; I wouldn't mind applying it if you can resend to touch up those issues. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list FWIW, I've been fixing a number of these poor assumptions lately. There's no limitation that there must be a fileName specified either and the way the patch is formulated it will treat it as a fatal error that there is a floppy drive defined with no file name for the floppy image. -- Doug Goldstein -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC] net-dhcp-leases: virNetworkDHCPLeases struct design
On Fri, Oct 11, 2013 at 12:36 AM, Eric Blake ebl...@redhat.com wrote: On 10/08/2013 06:39 AM, Nehal J Wani wrote: Continued from https://www.redhat.com/archives/libvir-list/2013-October/msg00325.html Since no one likes the idea of having a union, but we do want to support dhcpv6, following is the new design: struct _virNetworkDHCPLeases { char *interface;/* Network interface name (non-null) */ long long expirytime; /* Seconds since epoch (non-null) */ (non-null) makes no sense for an integer. On the other hand, should we document a sentinel for unknown (or infinite) lease expiration? dnsmasq will return the value 0 for expirytime in case of infinite expiration. int type; /* virIPAddrType (non-null) */ (non-null) makes no sense for an integer. unsigned int prefix;/* IP address prefix (non-null) */ (non-null) makes no sense for an integer; should we document that 0 means unknown? char *mac; /* MAC address (mostly non-null, except rare cases) */ I guess it could be NULL for the generic virNetworkDHCPLeases query; but I don't see how it can possibly be NULL for the specific per-MAC virNetworkDHCPLeases ForMAC(). Probably also worth documenting that it is in ASCII form (not raw bytes). If you implement it as remote_string on the RPC side, then you are guaranteeing that it is non-NULL; are we hurting ourselves if we make that guarantee? Well, in the discussion with dnsmasq developers, Simon had said, if you're interested in the MAC addresses of clients, the very latest dnsmasq code can determine that in most cases. Since it says 'most cases', and we don't want to take risks, I was being skeptical in keeping it NON-NULL. In the case for virNetworkDHCPLeasesForMAC(), if the user passes NULL for@mac, the API will automatically throw an error: +/* Validate the MAC address */ +if (virMacAddrParse(mac, addr) 0) { +virReportInvalidArg(mac, %s, +_(Given MAC Address doesn't comply + with the standard (IEEE 802) format)); +goto error; +} So, rewriting: /** * _virNetworkDHCPLeases: * For DHCPv4, the information returned: * - Network Interface Name * - Expiry Time * - MAC address (can be NULL, only in rare cases) * - IAID (NULL) * - IPv4 address (with type and prefix) * - Hostname (can be NULL) * - Client ID (can be NULL) * * For DHCPv6, the information returned: * - Network Interface Name * - Expiry Time * - MAC address (can be NULL, only in rare cases) * - IAID (can be NULL, only in rare cases) * - IPv6 address (with type and prefix) * - Hostname (can be NULL) * - Client DUID * * Note: @mac, @iaid, @ipaddr, @clientid are in ASCII form, not raw bytes. * Note: @expirytime can 0, in case the lease is for infinite time. */ struct _virNetworkDHCPLeases { char *interface;/* Network interface name */ long long expirytime; /* Seconds since epoch */ int type; /* virIPAddrType */ unsigned int prefix;/* IP address prefix */ char *mac; /* MAC address */ char *iaid; /* IAID */ char *ipaddr; /* IP address */ char *hostname; /* Hostname */ char *clientid; /* Client ID or DUID */ }; /* Remote struct */ struct remote_network_dhcp_lease { remote_nonnull_string interface; hyper expirytime; int type; unsigned int prefix; remote_string mac; remote_string iaid; remote_nonnull_string ipaddr; remote_string hostname; remote_string clientid; }; -- Nehal J Wani -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] esx: Fix floppy.fileName handling in the vmx file parser
Eric: No problem, I'll resubmit with git (sorry about that, it's been long enough since my last submit that I forgot about that), but I'll fix the issue that Doug pointed out first. As for the pointer comparison to NULL, I'm in complete agreement, but that particular source file uses the != NULL idiom everywhere. I'd be happy to remove it everywhere, but that should be done as a separate submit, I would think? Thanks, 2nd try soon. :) On Thu, Oct 10, 2013 at 4:15 PM, Doug Goldstein car...@gentoo.org wrote: On Thu, Oct 10, 2013 at 2:21 PM, Eric Blake ebl...@redhat.com wrote: On 10/09/2013 07:39 PM, Geoff Hickey wrote: The vmx file parsing code was reporting errors when parsing floppy.fileName entries if the filename didn't end in .flp. There is no such restriction in ESX; even using the GUI to configure floppy filenames you can specify any arbitrary file with any extension. Fix by changing the vmx parsing code so that it uses the floppy.fileType value to determine whether floppy.fileName refers to a block device or a regular file. --- src/vmx/vmx.c | 26 +- 1 file changed, 5 insertions(+), 21 deletions(-) diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 38b7cc0..ffe7e7a 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -2250,27 +2250,18 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con goto cleanup; } } else if (device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) { -if (virFileHasSuffix(fileName, .flp)) { -if (fileType != NULL) { -if (STRCASENEQ(fileType, file)) { -virReportError(VIR_ERR_INTERNAL_ERROR, - _(Expecting VMX entry '%s' to be 'file' but Your mailer corrupted your patch. Would you mind resending it via 'git send-email', or at a bare minimum as an attachment rather than inline, so that maintainers can just 'git am' it? Applying: esx: Fix floppy.fileName handling in the vmx file parser fatal: corrupt patch at line 10 Repository lacks necessary blobs to fall back on 3-way merge. Cannot fall back to three-way merge. Patch failed at 0001 esx: Fix floppy.fileName handling in the vmx file parser See also http://libvirt.org/hacking.html for hints. +if (fileType != NULL STRCASEEQ(fileType, device)) { It's simpler to just use a pointer variable in boolean context instead of explicit comparison against NULL (this isn't Java). As in: if (fileType STRCASEEQ(fileType, device)) { At any rate, your change looks reasonable to me; I wouldn't mind applying it if you can resend to touch up those issues. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list FWIW, I've been fixing a number of these poor assumptions lately. There's no limitation that there must be a fileName specified either and the way the patch is formulated it will treat it as a fatal error that there is a floppy drive defined with no file name for the floppy image. -- Doug Goldstein -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] rpc: Fix getsockopt on Snow Leopard and lower
Since 5a468b38b6 we use SOL_LOCAL for the 2nd argument of getsockopt() however Lion added the define SOL_LOCAL set to 0, which is the value to the 2nd argument of getsockopt() for Unix sockets on Mac OS X. So instead of using the define just pass 0 so we restore compatibility with Snow Leopard and Leopard. Reported at https://github.com/mxcl/homebrew/pull/23141 --- src/rpc/virnetsocket.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index a2823ef..8651a8b 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -1160,7 +1160,13 @@ int virNetSocketGetUNIXIdentity(virNetSocketPtr sock, virObjectLock(sock); # if defined(__APPLE__) -if (getsockopt(sock-fd, SOL_LOCAL, LOCAL_PEERCRED, cr, cr_len) 0) { +/* Lion (10.7) and higher define the value to be passed to getsockopt() + * as SOL_LOCAL for Unix sockets, it is defined to 0. However + * Snow Leopard and lower did not have this define so you had to + * pass 0. To support the most cases we just pass 0, this behavior + * matches PostgreSQL and CUPS source code. + */ +if (getsockopt(sock-fd, 0, LOCAL_PEERCRED, cr, cr_len) 0) { # else if (getsockopt(sock-fd, SOL_SOCKET, LOCAL_PEERCRED, cr, cr_len) 0) { # endif -- 1.8.1.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] esx: Fix floppy.fileName handling in the vmx file parser
On 10/10/2013 02:49 PM, Geoff Hickey wrote: Eric: No problem, I'll resubmit with git (sorry about that, it's been long enough since my last submit that I forgot about that), but I'll fix the issue that Doug pointed out first. As for the pointer comparison to NULL, I'm in complete agreement, but that particular source file uses the != NULL idiom everywhere. I'd be happy to remove it everywhere, but that should be done as a separate submit, I would think? Indeed, if you're going to clean up the entire file, do it as a separate commit. Thanks, 2nd try soon. :) Looking forward to it. Also, we tend to avoid top-posting on this list. -- 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] rpc: Fix getsockopt on Snow Leopard and lower
On 10/10/2013 03:44 PM, Doug Goldstein wrote: Since 5a468b38b6 we use SOL_LOCAL for the 2nd argument of getsockopt() however Lion added the define SOL_LOCAL set to 0, which is the value to the 2nd argument of getsockopt() for Unix sockets on Mac OS X. So instead of using the define just pass 0 so we restore compatibility with Snow Leopard and Leopard. Reported at https://github.com/mxcl/homebrew/pull/23141 --- +++ b/src/rpc/virnetsocket.c @@ -1160,7 +1160,13 @@ int virNetSocketGetUNIXIdentity(virNetSocketPtr sock, virObjectLock(sock); # if defined(__APPLE__) -if (getsockopt(sock-fd, SOL_LOCAL, LOCAL_PEERCRED, cr, cr_len) 0) { +/* Lion (10.7) and higher define the value to be passed to getsockopt() + * as SOL_LOCAL for Unix sockets, it is defined to 0. However + * Snow Leopard and lower did not have this define so you had to + * pass 0. To support the most cases we just pass 0, this behavior + * matches PostgreSQL and CUPS source code. + */ +if (getsockopt(sock-fd, 0, LOCAL_PEERCRED, cr, cr_len) 0) { # else if (getsockopt(sock-fd, SOL_SOCKET, LOCAL_PEERCRED, cr, cr_len) 0) { # endif You know, I think it might be even cleaner-looking if we could hoist the ifdef outside of the function body. Something like: /* VIR_SOL_PEERCRED - the value needed to let getsockopt() work with * LOCAL_PEERCRED */ #ifdef __APPLE__ # ifdef SOL_LOCAL # define VIR_SOL_PEERCRED SOL_LOCAL # else # define VIR_SOL_PEERCRED 0 # endif #else # define VIR_SOL_PEERCRED SOL_SOCKET #endif ... { if (getsockopt(sock-fd, VIR_SOL_PEERCRED, LOCAL_PEERCRED, cr, cr_len) 0) { ... -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu_migrate: Fix assign the same port when migrating concurrently
From f56b290eab36bbb7a9ac53778a55638d473504d1 Mon Sep 17 00:00:00 2001 From: WangYufei james.wangyu...@huawei.com Date: Fri, 11 Oct 2013 11:27:13 +0800 Subject: [PATCH] qemu_migrate: Fix assign the same port when migrating concurrently When we migrate vms concurrently, there's a chance that libvirtd on destination assign the same port for different migrations, which will lead to migration failed during migration prepare phase on destination. So we use virPortAllocator here to solve the problem. Signed-off-by: WangYufei james.wangyu...@huawei.com --- src/qemu/qemu_command.h |3 +++ src/qemu/qemu_conf.h |6 +++--- src/qemu/qemu_domain.h|1 + src/qemu/qemu_driver.c|6 ++ src/qemu/qemu_migration.c | 28 +--- 5 files changed, 34 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 2e2acfb..3277ba4 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -51,6 +51,9 @@ # define QEMU_WEBSOCKET_PORT_MIN 5700 # define QEMU_WEBSOCKET_PORT_MAX 65535 +# define QEMU_MIGRATION_PORT_MIN 49152 +# define QEMU_MIGRATION_PORT_MAX 49215 + typedef struct _qemuBuildCommandLineCallbacks qemuBuildCommandLineCallbacks; typedef qemuBuildCommandLineCallbacks *qemuBuildCommandLineCallbacksPtr; struct _qemuBuildCommandLineCallbacks { diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index da29a2a..3176085 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -221,6 +221,9 @@ struct _virQEMUDriver { /* Immutable pointer, self-locking APIs */ virPortAllocatorPtr webSocketPorts; +/* Immutable pointer, self-locking APIs */ +virPortAllocatorPtr migrationPorts; + /* Immutable pointer, lockless APIs*/ virSysinfoDefPtr hostsysinfo; @@ -242,9 +245,6 @@ struct _qemuDomainCmdlineDef { char **env_value; }; -/* Port numbers used for KVM migration. */ -# define QEMUD_MIGRATION_FIRST_PORT 49152 -# define QEMUD_MIGRATION_NUM_PORTS 64 void qemuDomainCmdlineDefFree(qemuDomainCmdlineDefPtr def); diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 21f116c..16c55a6 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -160,6 +160,7 @@ struct _qemuDomainObjPrivate { unsigned long migMaxBandwidth; char *origname; int nbdPort; /* Port used for migration with NBD */ +int migrationPort; virChrdevsPtr devs; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index cfdbb9a..c08a73c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -687,6 +687,11 @@ qemuStateInitialize(bool privileged, cfg-webSocketPortMax)) == NULL) goto error; +if ((qemu_driver-migrationPorts = + virPortAllocatorNew(QEMU_MIGRATION_PORT_MIN, + QEMU_MIGRATION_PORT_MAX)) == NULL) +goto error; + if (qemuSecurityInit(qemu_driver) 0) goto error; @@ -993,6 +998,7 @@ qemuStateCleanup(void) { virObjectUnref(qemu_driver-domains); virObjectUnref(qemu_driver-remotePorts); virObjectUnref(qemu_driver-webSocketPorts); +virObjectUnref(qemu_driver-migrationPorts); virObjectUnref(qemu_driver-xmlopt); diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 3a1aab7..93ae237 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2147,6 +2147,9 @@ qemuMigrationPrepareCleanup(virQEMUDriverPtr driver, qemuDomainJobTypeToString(priv-job.active), qemuDomainAsyncJobTypeToString(priv-job.asyncJob)); +virPortAllocatorRelease(driver-migrationPorts, priv-migrationPort); +priv-migrationPort = 0; + if (!qemuMigrationJobIsActive(vm, QEMU_ASYNC_JOB_MIGRATION_IN)) return; qemuDomainObjDiscardAsyncJob(driver, vm); @@ -2297,6 +2300,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, *def = NULL; priv = vm-privateData; +priv-migrationPort = port; if (VIR_STRDUP(priv-origname, origname) 0) goto cleanup; @@ -2415,6 +2419,11 @@ cleanup: VIR_FREE(xmlout); VIR_FORCE_CLOSE(dataFD[0]); VIR_FORCE_CLOSE(dataFD[1]); +if (ret 0) { +virPortAllocatorRelease(driver-migrationPorts, port); +if (priv) +priv-migrationPort = 0; +} if (vm) { if (ret = 0 || vm-persistent) virObjectUnlock(vm); @@ -2493,7 +2502,6 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver, const char *origname, unsigned long flags) { -static int port = 0; int this_port; char *hostname = NULL; const char *p; @@ -2521,8 +2529,9 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver, * to be a correct hostname which refers to the target machine). */ if (uri_in == NULL) { -this_port = QEMUD_MIGRATION_FIRST_PORT + port++; -if (port == QEMUD_MIGRATION_NUM_PORTS) port = 0; +