[libvirt] [PATCH 0/3] Fix handling of data returned by XML hooks
Peter Krempa (3): util: string: Add helper to check whether string is empty qemu: restore: Fix restoring of VM when the restore hook returns empty XML qemu: migration: Make check for empty hook XML robust src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_migration.c | 2 +- src/util/virstring.c | 16 ++++++++++++++++ src/util/virstring.h | 2 ++ 5 files changed, 21 insertions(+), 2 deletions(-) -- 2.1.0
The helper checks whether a string contains only whitespace or is NULL. This will be helpful to skip cases where a user string is optional, but may be provided empty with the same meaning. --- src/libvirt_private.syms | 1 + src/util/virstring.c | 16 ++++++++++++++++ src/util/virstring.h | 2 ++ 3 files changed, 19 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 9f749b7..d6b752e 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1999,6 +1999,7 @@ virStrdup; virStringArrayHasString; virStringFreeList; virStringFreeListCount; +virStringIsEmpty; virStringJoin; virStringListLength; virStringReplace; diff --git a/src/util/virstring.c b/src/util/virstring.c index df6464a..3dad9dd 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -633,6 +633,22 @@ virSkipSpacesBackwards(const char *str, char **endp) *endp = NULL; } +/** + * virStringIsEmpty: + * @str: string to check + * + * Returns true if string is empty (may contain only whitespace) or NULL. + */ +bool +virStringIsEmpty(const char *str) +{ + if (!str) + return true; + + virSkipSpaces(&str); + return str[0] == '\0'; +} + char * virArgvToString(const char *const *argv) { diff --git a/src/util/virstring.h b/src/util/virstring.h index 40ebaeb..2ec60fa 100644 --- a/src/util/virstring.h +++ b/src/util/virstring.h @@ -107,6 +107,8 @@ void virTrimSpaces(char *str, char **endp) ATTRIBUTE_NONNULL(1); void virSkipSpacesBackwards(const char *str, char **endp) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +bool virStringIsEmpty(const char *str); + char *virStrncpy(char *dest, const char *src, size_t n, size_t destbytes) ATTRIBUTE_RETURN_CHECK; char *virStrcpy(char *dest, const char *src, size_t destbytes) -- 2.1.0
The documentation for the restore hook states that returning an empty XML is equivalent with copying the input. There was a bug in the code checking the returned string by checking the string instead of the contents. Use the new helper to check if the string is empty. --- src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 009abc6..a062c06 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5831,7 +5831,7 @@ qemuDomainRestoreFlags(virConnectPtr conn, &xmlout)) < 0) goto cleanup; - if (hookret == 0 && xmlout) { + if (hookret == 0 && !virStringIsEmpty(xmlout)) { VIR_DEBUG("Using hook-filtered domain XML: %s", xmlout); hook_taint = true; newxml = xmlout; -- 2.1.0
Also consider whitespace only strings returned from the hook as empty result. --- src/qemu/qemu_migration.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index e135249..ca70e35 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2570,7 +2570,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, if (hookret < 0) { goto cleanup; } else if (hookret == 0) { - if (!*xmlout) { + if (virStringIsEmpty(xmlout)) { VIR_DEBUG("Migrate hook filter returned nothing; using the" " original XML"); } else { -- 2.1.0
On Wed, Oct 22, 2014 at 11:48:03AM +0200, Peter Krempa wrote:
Peter Krempa (3): util: string: Add helper to check whether string is empty qemu: restore: Fix restoring of VM when the restore hook returns empty XML qemu: migration: Make check for empty hook XML robust
src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_migration.c | 2 +- src/util/virstring.c | 16 ++++++++++++++++ src/util/virstring.h | 2 ++ 5 files changed, 21 insertions(+), 2 deletions(-)
-- 2.1.0
You're still missing one check in qemuDomainObjRestore(), but it's the last one. Fixing this in virHookCall() seems to make it unnecessarily complicated. ACK if you fix qemuDomainObjRestore() as well. Martin
On 10/22/14 15:01, Martin Kletzander wrote:
On Wed, Oct 22, 2014 at 11:48:03AM +0200, Peter Krempa wrote:
Peter Krempa (3): util: string: Add helper to check whether string is empty qemu: restore: Fix restoring of VM when the restore hook returns empty XML qemu: migration: Make check for empty hook XML robust
src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_migration.c | 2 +- src/util/virstring.c | 16 ++++++++++++++++ src/util/virstring.h | 2 ++ 5 files changed, 21 insertions(+), 2 deletions(-)
-- 2.1.0
You're still missing one check in qemuDomainObjRestore(), but it's the last one. Fixing this in virHookCall() seems to make it unnecessarily complicated. ACK if you fix qemuDomainObjRestore() as well.
Aaah, thanks for pointing out the missing piece.
Martin
Pushed; Thanks. Peter
participants (2)
-
Martin Kletzander -
Peter Krempa