June 2017 - Devel - libvirt List Archives

[libvirt] [PATCH] nodedev: udev: Drop udev monitor fd check in udevEventHandleCallback
by Erik Skultety 26 Jun '17

26 Jun '17

We do perform that magical check every time an event is received, but unless we explicitly unref'd or disconnected the monitor, we should not get a spurious event from a different source. Signed-off-by: Erik Skultety <eskultet(a)redhat.com> --- This is just a poke patch, since I also checked libudev's source and still fail to understand why we need that check. Unless entered by *_monitor_unref with the last reference or with *monitor_disconnect, libudev doesn't change the fd arbitrarily. So the only reason for us to check for that would be if we disconnected from our side, but didn't unregister the callback in which case it was our responsibility to clean up properly and we shouldn't get to the callback in the first place. Thanks for comments in advance. src/node_device/node_device_udev.c | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 174124a1b..7820c49c4 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1602,24 +1602,15 @@ nodeStateCleanup(void) static void udevEventHandleCallback(int watch ATTRIBUTE_UNUSED, - int fd, + int fd ATTRIBUTE_UNUSED, int events ATTRIBUTE_UNUSED, void *data ATTRIBUTE_UNUSED) { struct udev_device *device = NULL; struct udev_monitor *udev_monitor = DRV_STATE_UDEV_MONITOR(driver); const char *action = NULL; - int udev_fd = -1; nodeDeviceLock(); - udev_fd = udev_monitor_get_fd(udev_monitor); - if (fd != udev_fd) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("File descriptor returned by udev %d does not " - "match node device file descriptor %d"), - fd, udev_fd); - goto cleanup; - } device = udev_monitor_receive_device(udev_monitor); if (device == NULL) { -- 2.13.2

2 2

[libvirt] [PATCH go-xml] support virtualport for interface and add test code
by ZhenweiPi 26 Jun '17

26 Jun '17

--- domain.go | 29 +++++++++++++++++------------ domain_test.go | 4 ++++ 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/domain.go b/domain.go index eb7ff9e..ecb8550 100644 --- a/domain.go +++ b/domain.go @@ -185,19 +185,24 @@ type DomainInterfaceDriver struct { Queues uint `xml:"queues,attr,omitempty"` } +type DomainInterfaceVirtualport struct { + Type string `xml:"type,attr"` +} + type DomainInterface struct { - XMLName xml.Name `xml:"interface"` - Type string `xml:"type,attr"` - MAC *DomainInterfaceMAC `xml:"mac"` - Model *DomainInterfaceModel `xml:"model"` - Source *DomainInterfaceSource `xml:"source"` - Target *DomainInterfaceTarget `xml:"target"` - Alias *DomainInterfaceAlias `xml:"alias"` - Link *DomainInterfaceLink `xml:"link"` - Boot *DomainDeviceBoot `xml:"boot"` - Script *DomainInterfaceScript `xml:"script"` - Driver *DomainInterfaceDriver `xml:"driver"` - Address *DomainAddress `xml:"address"` + XMLName xml.Name `xml:"interface"` + Type string `xml:"type,attr"` + MAC *DomainInterfaceMAC `xml:"mac"` + Model *DomainInterfaceModel `xml:"model"` + Source *DomainInterfaceSource `xml:"source"` + Target *DomainInterfaceTarget `xml:"target"` + Alias *DomainInterfaceAlias `xml:"alias"` + Link *DomainInterfaceLink `xml:"link"` + Boot *DomainDeviceBoot `xml:"boot"` + Script *DomainInterfaceScript `xml:"script"` + Driver *DomainInterfaceDriver `xml:"driver"` + Virtualport *DomainInterfaceVirtualport `xml:"virtualport"` + Address *DomainAddress `xml:"address"` } type DomainChardevSource struct { diff --git a/domain_test.go b/domain_test.go index 6b37719..9de725c 100644 --- a/domain_test.go +++ b/domain_test.go @@ -565,6 +565,9 @@ var domainTestData = []struct { Model: &DomainInterfaceModel{ Type: "virtio", }, + Virtualport: &DomainInterfaceVirtualport{ + Type: "openvswitch", + }, }, }, }, @@ -577,6 +580,7 @@ var domainTestData = []struct { ` <interface type="network">`, ` <mac address="00:11:22:33:44:55"></mac>`, ` <model type="virtio"></model>`, + ` <virtualport type="openvswitch"></virtualport>`, ` </interface>`, ` </devices>`, `</domain>`, -- 2.7.4

2 1

[libvirt] [PATCH go-xml] add append attr for chardev
by zhenwei.pi 26 Jun '17

26 Jun '17

--- domain.go | 5 +++-- domain_test.go | 14 ++++++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/domain.go b/domain.go index 82bf728..00231b2 100644 --- a/domain.go +++ b/domain.go @@ -227,8 +227,9 @@ type DomainInterface struct { } type DomainChardevSource struct { - Mode string `xml:"mode,attr"` - Path string `xml:"path,attr"` + Mode string `xml:"mode,attr,omitempty"` + Path string `xml:"path,attr"` + Append string `xml:"append,attr,omitempty"` } type DomainChardevTarget struct { diff --git a/domain_test.go b/domain_test.go index 122aada..f50f1fd 100644 --- a/domain_test.go +++ b/domain_test.go @@ -297,6 +297,16 @@ var domainTestData = []struct { Port: &serialPort, }, }, + DomainSerial{ + Type: "file", + Source: &DomainChardevSource{ + Path: "/tmp/serial.log", + Append: "off", + }, + Target: &DomainSerialTarget{ + Port: &serialPort, + }, + }, }, Channels: []DomainChannel{ DomainChannel{ @@ -332,6 +342,10 @@ var domainTestData = []struct { ` <serial type="pty">`, ` <target type="isa" port="0"></target>`, ` </serial>`, + ` <serial type="file">`, + ` <source path="/tmp/serial.log" append="off"></source>`, + ` <target port="0"></target>`, + ` </serial>`, ` <console type="pty">`, ` <target type="virtio" port="0"></target>`, ` </console>`, -- 2.7.4

2 1

[libvirt] [PATCH] qemu: Avoid fd leak on incoming tunneled migration
by Jiri Denemark 26 Jun '17

26 Jun '17

While qemuProcessIncomingDefNew takes an fd argument and stores it in qemuProcessIncomingDef structure, the caller is still responsible for closing the file descriptor. Introduced by commit v1.2.21-140-ge7c6f4575. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com> --- src/qemu/qemu_migration.c | 1 - src/qemu/qemu_process.c | 3 +++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 8129dcd40..c23fffef2 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2690,7 +2690,6 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, listenAddress, port, dataFD[0]))) goto stopjob; - dataFD[0] = -1; /* the FD is now owned by incoming */ if (qemuProcessPrepareDomain(dconn, driver, vm, VIR_QEMU_PROCESS_START_AUTODESTROY) < 0) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index fa9990e5d..443ec88fe 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4169,6 +4169,9 @@ qemuProcessIncomingDefFree(qemuProcessIncomingDefPtr inc) * This function does not copy @path, the caller is responsible for keeping * the @path pointer valid during the lifetime of the allocated * qemuProcessIncomingDef structure. + * + * The caller is responsible for closing @fd, calling + * qemuProcessIncomingDefFree will NOT close it. */ qemuProcessIncomingDefPtr qemuProcessIncomingDefNew(virQEMUCapsPtr qemuCaps, -- 2.13.1

2 2

[libvirt] [PATCH] README: Remove unnecessary empty line
by Andrea Bolognani 26 Jun '17

26 Jun '17

Signed-off-by: Andrea Bolognani <abologna(a)redhat.com> --- Pushed as trivial. README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/README.md b/README.md index 1035bcf..d0ce149 100644 --- a/README.md +++ b/README.md @@ -54,7 +54,6 @@ $ make $ make install ``` - The libvirt code relies on a large number of 3rd party libraries. These will be detected during execution of the configure script and a summary printed which lists any missing (optional) dependencies. -- 2.7.5

1 0

[libvirt] [PATCH v2] util: Extract locale-related fixes into separate functions
by Peter Krempa 26 Jun '17

26 Jun '17

From: Martin Kletzander <mkletzan(a)redhat.com> Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- src/util/virstring.c | 114 +++++++++++++++++++++++++++++++++------------------ 1 file changed, 75 insertions(+), 39 deletions(-) v2: - fixed Martin's laziness ... oh wait. diff --git a/src/util/virstring.c b/src/util/virstring.c index feea5be05..9b54bd6fb 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -521,19 +521,81 @@ virStrToLong_ullp(char const *s, char **end_ptr, int base, /* In case thread-safe locales are available */ #if HAVE_NEWLOCALE -static locale_t virLocale; +typedef locale_t virLocale; +static virLocale virLocaleRaw; static int virLocaleOnceInit(void) { - virLocale = newlocale(LC_ALL_MASK, "C", (locale_t)0); - if (!virLocale) + virLocaleRaw = newlocale(LC_ALL_MASK, "C", (locale_t)0); + if (!virLocaleRaw) return -1; return 0; } VIR_ONCE_GLOBAL_INIT(virLocale); -#endif + +/** + * virLocaleSetRaw: + * + * @oldlocale: set to old locale pointer + * + * Sets the locale to 'C' to allow operating on non-localized objects. + * Returns 0 on success -1 on error. + */ +static int +virLocaleSetRaw(virLocale *oldlocale) +{ + if (virLocaleInitialize() < 0) + return -1; + *oldlocale = uselocale(virLocaleRaw); + return 0; +} + +static void +virLocaleRevert(virLocale *oldlocale) +{ + uselocale(*oldlocale); +} + +static void +virLocaleFixupRadix(char **strp ATTRIBUTE_UNUSED) +{ +} + +#else /* !HAVE_NEWLOCALE */ + +typedef int virLocale; + +static int +virLocaleSetRaw(virLocale *oldlocale ATTRIBUTE_UNUSED) +{ + return 0; +} + +static void +virLocaleRevert(virLocale *oldlocale ATTRIBUTE_UNUSED) +{ +} + +static void +virLocaleFixupRadix(char **strp) +{ + char *radix, *tmp; + struct lconv *lc; + + lc = localeconv(); + radix = lc->decimal_point; + tmp = strstr(*strp, radix); + if (tmp) { + *tmp = '.'; + if (strlen(radix) > 1) + memmove(tmp + 1, tmp + strlen(radix), strlen(*strp) - (tmp - *strp)); + } +} + +#endif /* !HAVE_NEWLOCALE */ + /** * virStrToDouble @@ -547,22 +609,17 @@ virStrToDouble(char const *s, char **end_ptr, double *result) { + virLocale oldlocale; double val; char *p; int err; errno = 0; -#if HAVE_NEWLOCALE - locale_t old_loc; - if (virLocaleInitialize() < 0) + if (virLocaleSetRaw(&oldlocale) < 0) return -1; - - old_loc = uselocale(virLocale); -#endif val = strtod(s, &p); /* exempt from syntax-check */ -#if HAVE_NEWLOCALE - uselocale(old_loc); -#endif + virLocaleRevert(&oldlocale); + err = (errno || (!end_ptr && *p) || p == s); if (end_ptr) *end_ptr = p; @@ -582,38 +639,17 @@ virStrToDouble(char const *s, int virDoubleToStr(char **strp, double number) { + virLocale oldlocale; int ret = -1; -#if HAVE_NEWLOCALE - - locale_t old_loc; - - if (virLocaleInitialize() < 0) - goto error; + if (virLocaleSetRaw(&oldlocale) < 0) + return -1; - old_loc = uselocale(virLocale); ret = virAsprintf(strp, "%lf", number); - uselocale(old_loc); - -#else - char *radix, *tmp; - struct lconv *lc; + virLocaleRevert(&oldlocale); + virLocaleFixupRadix(strp); - if ((ret = virAsprintf(strp, "%lf", number) < 0)) - goto error; - - lc = localeconv(); - radix = lc->decimal_point; - tmp = strstr(*strp, radix); - if (tmp) { - *tmp = '.'; - if (strlen(radix) > 1) - memmove(tmp + 1, tmp + strlen(radix), strlen(*strp) - (tmp - *strp)); - } - -#endif /* HAVE_NEWLOCALE */ - error: return ret; } -- 2.12.2

3 2

[libvirt] [PATCH] HACKING: Refresh after changes to source file
by Andrea Bolognani 26 Jun '17

26 Jun '17

Commit 79c1900fc1eb changed docs/hacking.html.in, but *of course* I forgot once again to update the text-only version of the file at the same time. Signed-off-by: Andrea Bolognani <abologna(a)redhat.com> --- Pushed as trivial. HACKING | 54 ++++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 38 insertions(+), 16 deletions(-) diff --git a/HACKING b/HACKING index b78a9ae..1d9f3f1 100644 --- a/HACKING +++ b/HACKING @@ -43,27 +43,49 @@ post your patches: git pull --rebase (fix any conflicts) git send-email --cover-letter --no-chain-reply-to --annotate \ - --to=libvir-list(a)redhat.com master + --confirm=always --to=libvir-list(a)redhat.com master -(Note that the "git send-email" subcommand may not be in the main git package +For a single patch you can omit "--cover-letter", but a series of two or more +patches needs a cover letter. + +Note that the "git send-email" subcommand may not be in the main git package and using it may require installation of a separate package, for example the -"git-email" package in Fedora.) For a single patch you can omit -"--cover-letter", but a series of two or more patches needs a cover letter. If -you get tired of typing "--to=libvir-list(a)redhat.com" designation you can set -it in git config: +"git-email" package in Fedora and Debian. If this is your first time using +"git send-email", you might need to configure it to point it to your SMTP +server with something like: + + git config --global sendemail.smtpServer stmp.youremailprovider.net + +If you get tired of typing "--to=libvir-list(a)redhat.com" all the time, you can +configure that to be automatically handled as well: git config sendemail.to libvir-list(a)redhat.com -Please follow this as close as you can, especially the rebase and git -send-email part, as it makes life easier for other developers to review your -patch set. One should avoid sending patches as attachments, but rather send -them in email body along with commit message. If a developer is sending -another version of the patch (e.g. to address review comments), they are -advised to note differences to previous versions after the "---" line in the -patch so that it helps reviewers but doesn't become part of git history. -Moreover, such patch needs to be prefixed correctly with -"--subject-prefix=PATCHv2" appended to "git send-email" (substitute "v2" with -the correct version if needed though). +As a rule, patches should be sent to the mailing list only: all developers are +subscribed to libvir-list and read it regularly, so please don't CC individual +developers unless they've explicitly asked you to. + +Avoid using mail clients for sending patches, as most of them will mangle the +messages in some way, making them unusable for our purposes. Gmail and other +Web-based mail clients are particularly bad at this. + +If everything went well, your patch should show up on the libvir-list archives +<https://www.redhat.com/archives/libvir-list/> in a matter of minutes; if you +still can't find it on there after an hour or so, you should double-check your +setup. Note that your very first post to the mailing list will be subject to +moderation, and it's not uncommon for that to take around a day. + +Please follow this as close as you can, especially the rebase and "git +send-email" part, as it makes life easier for other developers to review your +patch set. + +One should avoid sending patches as attachments, but rather send them in email +body along with commit message. If a developer is sending another version of +the patch (e.g. to address review comments), they are advised to note +differences to previous versions after the "---" line in the patch so that it +helps reviewers but doesn't become part of git history. Moreover, such patch +needs to be prefixed correctly with "--subject-prefix=PATCHv2" appended to +"git send-email" (substitute "v2" with the correct version if needed though). (5) In your commit message, make the summary line reasonably short (60 characters is typical), followed by a blank line, followed by any longer description of -- 2.7.5

1 0

[libvirt] [PATCH] qemuMonitorTextAddDrive: Fail on unrecognized disk format
by Michal Privoznik 25 Jun '17

25 Jun '17

Since qemu commit 3ef6c40ad0b it can fail if trying to hotplug a disk that is not qcow2 despite us saying it is. We need to error out in that case. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/qemu/qemu_monitor_text.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 66c94fbcd..737e8389b 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -1963,6 +1963,12 @@ int qemuMonitorTextAddDrive(qemuMonitorPtr mon, goto cleanup; } + if (strstr(reply, "Image is not in")) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("Incorrect disk format")); + goto cleanup; + } + ret = 0; cleanup: -- 2.13.0

2 1

[libvirt] [PATCH] docs: Add callback-related info to virStream{Abort, Finish}
by Martin Kletzander 25 Jun '17

25 Jun '17

When one has a non-blocking stream and aborts or finishes it without removing the callback, any event loop invocation will trigger that callback, but it cannot be removed any more. We cannot remove the callback automatically from virStream{Abort,Finish} functions due to forward-compatibility. So let's at least document this behaviour, because it is not easy to find out the reason for. Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- Notes: The discussion about the reasons: https://www.redhat.com/archives/libvir-list/2017-June/msg00038.html src/libvirt-stream.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/libvirt-stream.c b/src/libvirt-stream.c index d7a8f581608f..c49f20264fd7 100644 --- a/src/libvirt-stream.c +++ b/src/libvirt-stream.c @@ -1131,6 +1131,9 @@ virStreamEventRemoveCallback(virStreamPtr stream) * errors, so if this returns a success code the application can * be sure that all data has been successfully processed. * + * If the stream is non-blocking, any callback must be removed + * beforehand. + * * Returns 0 on success, -1 upon error */ int @@ -1170,6 +1173,9 @@ virStreamFinish(virStreamPtr stream) * streams this can be used to inform the driver that it * should stop sending data. * + * If the stream is non-blocking, any callback must be removed + * beforehand. + * * Returns 0 on success, -1 upon error */ int -- 2.13.1

2 1

[libvirt] [PATCH v2] hacking: Improve 'git send-email' documentation
by Andrea Bolognani 25 Jun '17

25 Jun '17

For the benefit of first time contributors, we point out that 'git send-email' might have to be installed separately; however, we omit the fact that some configuration will likely be needed before it can successfully deliver patches to the mailing list. Some minor tweaks to the existing contents are included as well. Signed-off-by: Andrea Bolognani <abologna(a)redhat.com> --- docs/hacking.html.in | 46 ++++++++++++++++++++++++++++++++++------------ 1 file changed, 34 insertions(+), 12 deletions(-) diff --git a/docs/hacking.html.in b/docs/hacking.html.in index d6a574c..975ee69 100644 --- a/docs/hacking.html.in +++ b/docs/hacking.html.in @@ -29,8 +29,8 @@ file from zanata. </li> - <li>Post patches using "git send-email", with git rename - detection enabled. You need a one-time setup of: + <li>Post patches using <code>git send-email</code>, with git + rename detection enabled. You need a one-time setup of: <pre> git config diff.renames true </pre> @@ -50,22 +50,44 @@ git pull --rebase (fix any conflicts) git send-email --cover-letter --no-chain-reply-to --annotate \ - --to=libvir-list(a)redhat.com master + --confirm=always --to=libvir-list(a)redhat.com master </pre> - (Note that the "git send-email" subcommand may not be in - the main git package and using it may require installation of a - separate package, for example the "git-email" package in - Fedora.) For a single patch you can omit + For a single patch you can omit <code>--cover-letter</code>, but a series of two or more - patches needs a cover letter. If you get tired of typing - <code>--to=libvir-list(a)redhat.com</code> designation you can - set it in git config: + patches needs a cover letter. + Note that the <code>git send-email</code> subcommand may not + be in the main git package and using it may require installation + of a separate package, for example the "git-email" package in + Fedora and Debian. If this is your first time using + <code>git send-email</code>, you might need to configure it to + point it to your SMTP server with something like: +<pre> + git config --global sendemail.smtpServer stmp.youremailprovider.net +</pre> + If you get tired of typing + <code>--to=libvir-list(a)redhat.com</code> all the time, you can + configure that to be automatically handled as well: <pre> git config sendemail.to libvir-list(a)redhat.com </pre> + As a rule, patches should be sent to the mailing list only: all + developers are subscribed to libvir-list and read it regularly, so + please don't CC individual developers unless they've explicitly + asked you to. + Avoid using mail clients for sending patches, as most of them + will mangle the messages in some way, making them unusable for our + purposes. Gmail and other Web-based mail clients are particularly + bad at this. + If everything went well, your patch should show up on the + <a href="https://www.redhat.com/archives/libvir-list/">libvir-list + archives</a> in a matter of minutes; if you still can't find it on + there after an hour or so, you should double-check your setup. Note + that your very first post to the mailing list will be subject to + moderation, and it's not uncommon for that to take around a day. Please follow this as close as you can, especially the rebase and - git send-email part, as it makes life easier for other developers to - review your patch set. One should avoid sending patches as attachments, + <code>git send-email</code> part, as it makes life easier for other + developers to review your patch set. + One should avoid sending patches as attachments, but rather send them in email body along with commit message. If a developer is sending another version of the patch (e.g. to address review comments), they are advised to note differences to previous -- 2.7.5

2 1