[libvirt] [PATCH 0/7] Adapt machine name generation to new facts

Ján Tomko (7): virbuftest: remove extra G_GNUC_UNUSED markers virbuftest: use g_autofree virbuftest: remove unnecessary labels virbuftest: declare testBufAddStrData earlier virbuftest: use field names when initalizing test info util: add virBufferTrimChars conf: do not generate machine names ending with a dash src/conf/domain_conf.c | 3 + src/libvirt_private.syms | 1 + src/util/virbuffer.c | 26 +++++++++ src/util/virbuffer.h | 1 + tests/virbuftest.c | 123 ++++++++++++++++++++++----------------- tests/virsystemdtest.c | 4 ++ 6 files changed, 103 insertions(+), 55 deletions(-) -- 2.21.0

These functions do use the opaque argument. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/virbuftest.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/virbuftest.c b/tests/virbuftest.c index 56a6ece8f6..f8eadea25a 100644 --- a/tests/virbuftest.c +++ b/tests/virbuftest.c @@ -244,7 +244,7 @@ struct testBufAddStrData { }; static int -testBufAddStr(const void *opaque G_GNUC_UNUSED) +testBufAddStr(const void *opaque) { const struct testBufAddStrData *data = opaque; virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -277,7 +277,7 @@ testBufAddStr(const void *opaque G_GNUC_UNUSED) static int -testBufEscapeStr(const void *opaque G_GNUC_UNUSED) +testBufEscapeStr(const void *opaque) { const struct testBufAddStrData *data = opaque; virBuffer buf = VIR_BUFFER_INITIALIZER; -- 2.21.0

On Tue, Jan 14, 2020 at 08:35:35AM +0100, Ján Tomko wrote:
These functions do use the opaque argument.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- Reviewed-by: Erik Skultety <eskultet@redhat.com>

Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/virbuftest.c | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/tests/virbuftest.c b/tests/virbuftest.c index f8eadea25a..2b241424ad 100644 --- a/tests/virbuftest.c +++ b/tests/virbuftest.c @@ -15,7 +15,7 @@ static int testBufAutoIndent(const void *data G_GNUC_UNUSED) virBufferPtr buf = &bufinit; const char expected[] = " 1\n 2\n 3\n 4\n 5\n 6\n 7\n &\n 8\n 9\n 10\n ' 11'\n"; - char *result = NULL; + g_autofree char *result = NULL; int ret = 0; if (virBufferGetIndent(buf) != 0 || @@ -85,7 +85,6 @@ static int testBufAutoIndent(const void *data G_GNUC_UNUSED) virTestDifference(stderr, expected, result); ret = -1; } - VIR_FREE(result); return ret; } @@ -93,7 +92,7 @@ static int testBufTrim(const void *data G_GNUC_UNUSED) { virBuffer bufinit = VIR_BUFFER_INITIALIZER; virBufferPtr buf = NULL; - char *result = NULL; + g_autofree char *result = NULL; const char *expected = "a,b"; int ret = -1; @@ -123,7 +122,6 @@ static int testBufTrim(const void *data G_GNUC_UNUSED) cleanup: virBufferFreeAndReset(buf); - VIR_FREE(result); return ret; } @@ -133,7 +131,7 @@ static int testBufAddBuffer(const void *data G_GNUC_UNUSED) virBuffer buf2 = VIR_BUFFER_INITIALIZER; virBuffer buf3 = VIR_BUFFER_INITIALIZER; int ret = -1; - char *result = NULL; + g_autofree char *result = NULL; const char *expected = \ " A long time ago, in a galaxy far,\n" \ " far away...\n" \ @@ -234,7 +232,6 @@ static int testBufAddBuffer(const void *data G_GNUC_UNUSED) cleanup: virBufferFreeAndReset(&buf1); virBufferFreeAndReset(&buf2); - VIR_FREE(result); return ret; } @@ -248,7 +245,7 @@ testBufAddStr(const void *opaque) { const struct testBufAddStrData *data = opaque; virBuffer buf = VIR_BUFFER_INITIALIZER; - char *actual; + g_autofree char *actual = NULL; int ret = -1; virBufferAddLit(&buf, "<c>\n"); @@ -271,7 +268,6 @@ testBufAddStr(const void *opaque) ret = 0; cleanup: - VIR_FREE(actual); return ret; } @@ -281,7 +277,7 @@ testBufEscapeStr(const void *opaque) { const struct testBufAddStrData *data = opaque; virBuffer buf = VIR_BUFFER_INITIALIZER; - char *actual; + g_autofree char *actual = NULL; int ret = -1; virBufferAddLit(&buf, "<c>\n"); @@ -304,7 +300,6 @@ testBufEscapeStr(const void *opaque) ret = 0; cleanup: - VIR_FREE(actual); return ret; } @@ -314,7 +309,7 @@ testBufEscapeRegex(const void *opaque) { const struct testBufAddStrData *data = opaque; virBuffer buf = VIR_BUFFER_INITIALIZER; - char *actual; + g_autofree char *actual = NULL; int ret = -1; virBufferEscapeRegex(&buf, "%s", data->data); @@ -333,7 +328,6 @@ testBufEscapeRegex(const void *opaque) ret = 0; cleanup: - VIR_FREE(actual); return ret; } @@ -342,7 +336,7 @@ static int testBufSetIndent(const void *opaque G_GNUC_UNUSED) { virBuffer buf = VIR_BUFFER_INITIALIZER; - char *actual; + g_autofree char *actual = NULL; int ret = -1; virBufferSetIndent(&buf, 11); @@ -361,7 +355,6 @@ testBufSetIndent(const void *opaque G_GNUC_UNUSED) ret = 0; cleanup: - VIR_FREE(actual); return ret; } -- 2.21.0

Remove the ret variables and labels from functions that no longer need them. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/virbuftest.c | 40 ++++++++++++---------------------------- 1 file changed, 12 insertions(+), 28 deletions(-) diff --git a/tests/virbuftest.c b/tests/virbuftest.c index 2b241424ad..bb7fa9e2e9 100644 --- a/tests/virbuftest.c +++ b/tests/virbuftest.c @@ -246,7 +246,6 @@ testBufAddStr(const void *opaque) const struct testBufAddStrData *data = opaque; virBuffer buf = VIR_BUFFER_INITIALIZER; g_autofree char *actual = NULL; - int ret = -1; virBufferAddLit(&buf, "<c>\n"); virBufferAdjustIndent(&buf, 2); @@ -256,19 +255,16 @@ testBufAddStr(const void *opaque) if (!(actual = virBufferContentAndReset(&buf))) { VIR_TEST_DEBUG("buf is empty"); - goto cleanup; + return -1; } if (STRNEQ_NULLABLE(actual, data->expect)) { VIR_TEST_DEBUG("testBufAddStr(): Strings don't match:"); virTestDifference(stderr, data->expect, actual); - goto cleanup; + return -1; } - ret = 0; - - cleanup: - return ret; + return 0; } @@ -278,7 +274,6 @@ testBufEscapeStr(const void *opaque) const struct testBufAddStrData *data = opaque; virBuffer buf = VIR_BUFFER_INITIALIZER; g_autofree char *actual = NULL; - int ret = -1; virBufferAddLit(&buf, "<c>\n"); virBufferAdjustIndent(&buf, 2); @@ -288,19 +283,16 @@ testBufEscapeStr(const void *opaque) if (!(actual = virBufferContentAndReset(&buf))) { VIR_TEST_DEBUG("buf is empty"); - goto cleanup; + return -1; } if (STRNEQ_NULLABLE(actual, data->expect)) { VIR_TEST_DEBUG("testBufEscapeStr(): Strings don't match:"); virTestDifference(stderr, data->expect, actual); - goto cleanup; + return -1; } - ret = 0; - - cleanup: - return ret; + return 0; } @@ -310,25 +302,21 @@ testBufEscapeRegex(const void *opaque) const struct testBufAddStrData *data = opaque; virBuffer buf = VIR_BUFFER_INITIALIZER; g_autofree char *actual = NULL; - int ret = -1; virBufferEscapeRegex(&buf, "%s", data->data); if (!(actual = virBufferContentAndReset(&buf))) { VIR_TEST_DEBUG("testBufEscapeRegex: buf is empty"); - goto cleanup; + return -1; } if (STRNEQ_NULLABLE(actual, data->expect)) { VIR_TEST_DEBUG("testBufEscapeRegex: Strings don't match:"); virTestDifference(stderr, data->expect, actual); - goto cleanup; + return -1; } - ret = 0; - - cleanup: - return ret; + return 0; } @@ -337,7 +325,6 @@ testBufSetIndent(const void *opaque G_GNUC_UNUSED) { virBuffer buf = VIR_BUFFER_INITIALIZER; g_autofree char *actual = NULL; - int ret = -1; virBufferSetIndent(&buf, 11); virBufferAddLit(&buf, "test\n"); @@ -345,17 +332,14 @@ testBufSetIndent(const void *opaque G_GNUC_UNUSED) virBufferAddLit(&buf, "test2\n"); if (!(actual = virBufferContentAndReset(&buf))) - goto cleanup; + return -1; if (STRNEQ(actual, " test\n test2\n")) { VIR_TEST_DEBUG("testBufSetIndent: expected indent not set"); - goto cleanup; + return -1; } - ret = 0; - - cleanup: - return ret; + return 0; } -- 2.21.0

On Tue, Jan 14, 2020 at 08:35:37AM +0100, Ján Tomko wrote:
Remove the ret variables and labels from functions that no longer need them.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- Reviewed-by: Erik Skultety <eskultet@redhat.com>

Move the declaration to the beginning of the file for reuse. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/virbuftest.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/virbuftest.c b/tests/virbuftest.c index bb7fa9e2e9..bb606c1c28 100644 --- a/tests/virbuftest.c +++ b/tests/virbuftest.c @@ -9,6 +9,11 @@ #define VIR_FROM_THIS VIR_FROM_NONE +struct testBufAddStrData { + const char *data; + const char *expect; +}; + static int testBufAutoIndent(const void *data G_GNUC_UNUSED) { virBuffer bufinit = VIR_BUFFER_INITIALIZER; @@ -235,11 +240,6 @@ static int testBufAddBuffer(const void *data G_GNUC_UNUSED) return ret; } -struct testBufAddStrData { - const char *data; - const char *expect; -}; - static int testBufAddStr(const void *opaque) { -- 2.21.0

On Tue, Jan 14, 2020 at 08:35:38AM +0100, Ján Tomko wrote:
Move the declaration to the beginning of the file for reuse.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- Reviewed-by: Erik Skultety <eskultet@redhat.com>

Allow adding new fields without changing all the macros. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/virbuftest.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/virbuftest.c b/tests/virbuftest.c index bb606c1c28..1780b62bf4 100644 --- a/tests/virbuftest.c +++ b/tests/virbuftest.c @@ -372,9 +372,9 @@ mymain(void) DO_TEST("set indent", testBufSetIndent); DO_TEST("autoclean", testBufferAutoclean); -#define DO_TEST_ADD_STR(DATA, EXPECT) \ +#define DO_TEST_ADD_STR(_data, _expect) \ do { \ - struct testBufAddStrData info = { DATA, EXPECT }; \ + struct testBufAddStrData info = { .data = _data, .expect = _expect }; \ if (virTestRun("Buf: AddStr", testBufAddStr, &info) < 0) \ ret = -1; \ } while (0) @@ -384,9 +384,9 @@ mymain(void) DO_TEST_ADD_STR("<a/>\n", "<c>\n <a/>\n</c>"); DO_TEST_ADD_STR("<b>\n <a/>\n</b>\n", "<c>\n <b>\n <a/>\n </b>\n</c>"); -#define DO_TEST_ESCAPE(data, expect) \ +#define DO_TEST_ESCAPE(_data, _expect) \ do { \ - struct testBufAddStrData info = { data, expect }; \ + struct testBufAddStrData info = { .data = _data, .expect = _expect }; \ if (virTestRun("Buf: EscapeStr", testBufEscapeStr, &info) < 0) \ ret = -1; \ } while (0) @@ -400,9 +400,9 @@ mymain(void) DO_TEST_ESCAPE("\x01\x01\x02\x03\x05\x08", "<c>\n <el></el>\n</c>"); -#define DO_TEST_ESCAPE_REGEX(data, expect) \ +#define DO_TEST_ESCAPE_REGEX(_data, _expect) \ do { \ - struct testBufAddStrData info = { data, expect }; \ + struct testBufAddStrData info = { .data = _data, .expect = _expect }; \ if (virTestRun("Buf: EscapeRegex", testBufEscapeRegex, &info) < 0) \ ret = -1; \ } while (0) -- 2.21.0

On Tue, Jan 14, 2020 at 08:35:39AM +0100, Ján Tomko wrote:
Allow adding new fields without changing all the macros.
It would be IMO worth mentioning the error your compiler gave you in the next patch when you hadn't done this adjustment.
Signed-off-by: Ján Tomko <jtomko@redhat.com> ---
Reviewed-by: Erik Skultety <eskultet@redhat.com>

A new helper for trimming combinations of specified characters from the tail of the buffer. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virbuffer.c | 26 ++++++++++++++++++++++++++ src/util/virbuffer.h | 1 + tests/virbuftest.c | 36 ++++++++++++++++++++++++++++++++++++ 4 files changed, 64 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index b97906b852..f46ed29eac 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1642,6 +1642,7 @@ virBufferSetIndent; virBufferStrcat; virBufferStrcatVArgs; virBufferTrim; +virBufferTrimChars; virBufferURIEncodeString; virBufferUse; virBufferVasprintf; diff --git a/src/util/virbuffer.c b/src/util/virbuffer.c index 1b93110919..914c386b18 100644 --- a/src/util/virbuffer.c +++ b/src/util/virbuffer.c @@ -673,6 +673,32 @@ virBufferTrim(virBufferPtr buf, const char *str, int len) g_string_truncate(buf->str, buf->str->len - len); } +/** + * virBufferTrimChars: + * @buf: the buffer to trim + * @trim: the characters to be trimmed + * + * Trim the tail of the buffer. The longest string that can be formed with + * the characters from @trim is trimmed. + */ +void +virBufferTrimChars(virBufferPtr buf, const char *trim) +{ + ssize_t i; + + if (!buf || !buf->str) + return; + + if (!trim) + return; + + for (i = buf->str->len - 1; i > 0; i--) { + if (!strchr(trim, buf->str->str[i])) + break; + } + + g_string_truncate(buf->str, i + 1); +} /** * virBufferAddStr: diff --git a/src/util/virbuffer.h b/src/util/virbuffer.h index 38758a9125..183f78f279 100644 --- a/src/util/virbuffer.h +++ b/src/util/virbuffer.h @@ -92,4 +92,5 @@ size_t virBufferGetIndent(const virBuffer *buf); size_t virBufferGetEffectiveIndent(const virBuffer *buf); void virBufferTrim(virBufferPtr buf, const char *trim, int len); +void virBufferTrimChars(virBufferPtr buf, const char *trim); void virBufferAddStr(virBufferPtr buf, const char *str); diff --git a/tests/virbuftest.c b/tests/virbuftest.c index 1780b62bf4..7919075000 100644 --- a/tests/virbuftest.c +++ b/tests/virbuftest.c @@ -12,6 +12,7 @@ struct testBufAddStrData { const char *data; const char *expect; + const char *arg; }; static int testBufAutoIndent(const void *data G_GNUC_UNUSED) @@ -130,6 +131,30 @@ static int testBufTrim(const void *data G_GNUC_UNUSED) return ret; } +static int +testBufTrimChars(const void *opaque) +{ + const struct testBufAddStrData *data = opaque; + virBuffer buf = VIR_BUFFER_INITIALIZER; + g_autofree char *actual = NULL; + + virBufferAddStr(&buf, data->data); + virBufferTrimChars(&buf, data->arg); + + if (!(actual = virBufferContentAndReset(&buf))) { + VIR_TEST_DEBUG("buf is empty"); + return -1; + } + + if (STRNEQ_NULLABLE(actual, data->expect)) { + VIR_TEST_DEBUG("testBufEscapeStr(): Strings don't match:"); + virTestDifference(stderr, data->expect, actual); + return -1; + } + + return 0; +} + static int testBufAddBuffer(const void *data G_GNUC_UNUSED) { virBuffer buf1 = VIR_BUFFER_INITIALIZER; @@ -411,6 +436,17 @@ mymain(void) DO_TEST_ESCAPE_REGEX("^$.|?*+()[]{}\\", "\\^\\$\\.\\|\\?\\*\\+\\(\\)\\[\\]\\{\\}\\\\"); +#define DO_TEST_TRIM_CHARS(_data, _arg, _expect) \ + do { \ + struct testBufAddStrData info = { .data = _data, .expect = _expect, .arg = _arg }; \ + if (virTestRun("Buf: Trim: " #_data, testBufTrimChars, &info) < 0) \ + ret = -1; \ + } while (0) + + DO_TEST_TRIM_CHARS("Trimmm", "m", "Tri"); + DO_TEST_TRIM_CHARS("-abcd-efgh--", "-", "-abcd-efgh"); + DO_TEST_TRIM_CHARS("-hABC-efgh--", "-h", "-hABC-efg"); + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.21.0

On Tue, Jan 14, 2020 at 08:35:40AM +0100, Ján Tomko wrote:
A new helper for trimming combinations of specified characters from the tail of the buffer.
Signed-off-by: Ján Tomko <jtomko@redhat.com> ---
...
/** * virBufferAddStr: diff --git a/src/util/virbuffer.h b/src/util/virbuffer.h index 38758a9125..183f78f279 100644 --- a/src/util/virbuffer.h +++ b/src/util/virbuffer.h @@ -92,4 +92,5 @@ size_t virBufferGetIndent(const virBuffer *buf); size_t virBufferGetEffectiveIndent(const virBuffer *buf);
void virBufferTrim(virBufferPtr buf, const char *trim, int len); +void virBufferTrimChars(virBufferPtr buf, const char *trim); void virBufferAddStr(virBufferPtr buf, const char *str); diff --git a/tests/virbuftest.c b/tests/virbuftest.c index 1780b62bf4..7919075000 100644 --- a/tests/virbuftest.c +++ b/tests/virbuftest.c @@ -12,6 +12,7 @@ struct testBufAddStrData { const char *data; const char *expect; + const char *arg;
Maybe *opaque? #naming,#bikeshedding... Reviewed-by: Erik Skultety <eskultet@redhat.com>

As of systemd commit: commit d65652f1f21a4b0c59711320f34266c635393c89 Author: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> CommitDate: 2018-12-10 09:56:56 +0100 Partially unify hostname_is_valid() and dns_name_is_valid() Dashes are no longer allowed at the end of machine names. Trim the trailing dashes from the generated name before passing it to machined. https://bugzilla.redhat.com/show_bug.cgi?id=1790409 Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/conf/domain_conf.c | 3 +++ tests/virsystemdtest.c | 4 ++++ 2 files changed, 7 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1290241923..512b7c49d2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -30467,6 +30467,9 @@ virDomainMachineNameAppendValid(virBufferPtr buf, virBufferAddChar(buf, *name); } + + /* trailing dashes are not allowed */ + virBufferTrimChars(buf, "-"); } #undef HOSTNAME_CHARS diff --git a/tests/virsystemdtest.c b/tests/virsystemdtest.c index 9b95ca6789..26876850b8 100644 --- a/tests/virsystemdtest.c +++ b/tests/virsystemdtest.c @@ -740,6 +740,10 @@ mymain(void) "qemu-7-123456789012345678901234567890123456789012345678901234567"); TEST_MACHINE("123456789012345678901234567890123456789012345678901234567890", 8, "qemu-8-123456789012345678901234567890123456789012345678901234567"); + TEST_MACHINE("kstest-network-device-default-httpks_(c9eed63e-981e-48ec-acdc-56b3f8c5f678)", 100, + "qemu-100-kstest-network-device-default-httpksc9eed63e-981e-48ec"); + TEST_MACHINE("kstest-network-device-default-httpks_(c9eed63e-981e-48ec--cdc-56b3f8c5f678)", 10, + "qemu-10-kstest-network-device-default-httpksc9eed63e-981e-48ec"); # define TESTS_PM_SUPPORT_HELPER(name, function) \ do { \ -- 2.21.0

On Tue, Jan 14, 2020 at 08:35:41AM +0100, Ján Tomko wrote:
As of systemd commit:
commit d65652f1f21a4b0c59711320f34266c635393c89 Author: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> CommitDate: 2018-12-10 09:56:56 +0100
Partially unify hostname_is_valid() and dns_name_is_valid()
Dashes are no longer allowed at the end of machine names.
Trim the trailing dashes from the generated name before passing it to machined.
https://bugzilla.redhat.com/show_bug.cgi?id=1790409
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/conf/domain_conf.c | 3 +++ tests/virsystemdtest.c | 4 ++++ 2 files changed, 7 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1290241923..512b7c49d2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -30467,6 +30467,9 @@ virDomainMachineNameAppendValid(virBufferPtr buf,
virBufferAddChar(buf, *name); } + + /* trailing dashes are not allowed */ + virBufferTrimChars(buf, "-"); }
#undef HOSTNAME_CHARS diff --git a/tests/virsystemdtest.c b/tests/virsystemdtest.c index 9b95ca6789..26876850b8 100644 --- a/tests/virsystemdtest.c +++ b/tests/virsystemdtest.c @@ -740,6 +740,10 @@ mymain(void) "qemu-7-123456789012345678901234567890123456789012345678901234567"); TEST_MACHINE("123456789012345678901234567890123456789012345678901234567890", 8, "qemu-8-123456789012345678901234567890123456789012345678901234567"); + TEST_MACHINE("kstest-network-device-default-httpks_(c9eed63e-981e-48ec-acdc-56b3f8c5f678)", 100, + "qemu-100-kstest-network-device-default-httpksc9eed63e-981e-48ec"); + TEST_MACHINE("kstest-network-device-default-httpks_(c9eed63e-981e-48ec--cdc-56b3f8c5f678)", 10, + "qemu-10-kstest-network-device-default-httpksc9eed63e-981e-48ec");
Reviewed-by: Erik Skultety <eskultet@redhat.com>
participants (2)
-
Erik Skultety
-
Ján Tomko