migration/migration.c | 3 ++- monitor/hmp-cmds.c | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-)
run:
(qemu) info migrate_parameters
announce-initial: 50 ms
...
announce-max: 550 ms
multifd-compression: none
xbzrle-cache-size: 4194304
max-postcopy-bandwidth: 0
tls-authz: '(null)'
Migration parameter 'tls-authz' is used to provide the QOM ID
of a QAuthZ subclass instance that provides the access control
check, default is NULL. But the empty string is not a valid
object ID, so use "" instead of the default. Although it will
fail when lookup an object with ID "", it is harmless, just
consistent with tls_creds.
Also fixed the bad indentation on the last line.
Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
---
migration/migration.c | 3 ++-
monitor/hmp-cmds.c | 2 +-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index c1d88ace7f..b060153ef7 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -790,7 +790,8 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
params->has_tls_hostname = true;
params->tls_hostname = g_strdup(s->parameters.tls_hostname);
params->has_tls_authz = true;
- params->tls_authz = g_strdup(s->parameters.tls_authz);
+ params->tls_authz = s->parameters.tls_authz ? \
+ g_strdup(s->parameters.tls_authz) : g_strdup("");
params->has_max_bandwidth = true;
params->max_bandwidth = s->parameters.max_bandwidth;
params->has_downtime_limit = true;
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 58724031ea..f8be6bbb16 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -459,7 +459,7 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict)
monitor_printf(mon, "%s: %" PRIu64 "\n",
MigrationParameter_str(MIGRATION_PARAMETER_MAX_POSTCOPY_BANDWIDTH),
params->max_postcopy_bandwidth);
- monitor_printf(mon, " %s: '%s'\n",
+ monitor_printf(mon, "%s: '%s'\n",
MigrationParameter_str(MIGRATION_PARAMETER_TLS_AUTHZ),
params->has_tls_authz ? params->tls_authz : "");
}
--
2.17.1
* Mao Zhongyi (maozhongyi@cmss.chinamobile.com) wrote: > run: > (qemu) info migrate_parameters > announce-initial: 50 ms > ... > announce-max: 550 ms > multifd-compression: none > xbzrle-cache-size: 4194304 > max-postcopy-bandwidth: 0 > tls-authz: '(null)' > > Migration parameter 'tls-authz' is used to provide the QOM ID > of a QAuthZ subclass instance that provides the access control > check, default is NULL. But the empty string is not a valid > object ID, so use "" instead of the default. Although it will > fail when lookup an object with ID "", it is harmless, just > consistent with tls_creds. Yes, it's probably the best we can do given Dan's explanation that we can't change tls_authz to be non-null. > Also fixed the bad indentation on the last line. > > Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com> > --- > migration/migration.c | 3 ++- > monitor/hmp-cmds.c | 2 +- > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/migration/migration.c b/migration/migration.c > index c1d88ace7f..b060153ef7 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -790,7 +790,8 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp) > params->has_tls_hostname = true; > params->tls_hostname = g_strdup(s->parameters.tls_hostname); > params->has_tls_authz = true; > - params->tls_authz = g_strdup(s->parameters.tls_authz); > + params->tls_authz = s->parameters.tls_authz ? \ > + g_strdup(s->parameters.tls_authz) : g_strdup(""); The \ is unneeded; this isn't a macro; it's also a little shorter to do it as: params->tls_authz = g_strdup(s->parameters.tls_authz ? s->parameters.tls_authz : ""); Dave > params->has_max_bandwidth = true; > params->max_bandwidth = s->parameters.max_bandwidth; > params->has_downtime_limit = true; > diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c > index 58724031ea..f8be6bbb16 100644 > --- a/monitor/hmp-cmds.c > +++ b/monitor/hmp-cmds.c > @@ -459,7 +459,7 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict) > monitor_printf(mon, "%s: %" PRIu64 "\n", > MigrationParameter_str(MIGRATION_PARAMETER_MAX_POSTCOPY_BANDWIDTH), > params->max_postcopy_bandwidth); > - monitor_printf(mon, " %s: '%s'\n", > + monitor_printf(mon, "%s: '%s'\n", > MigrationParameter_str(MIGRATION_PARAMETER_TLS_AUTHZ), > params->has_tls_authz ? params->tls_authz : ""); > } > -- > 2.17.1 > > > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
"Dr. David Alan Gilbert" <dgilbert@redhat.com> writes: > * Mao Zhongyi (maozhongyi@cmss.chinamobile.com) wrote: >> run: >> (qemu) info migrate_parameters >> announce-initial: 50 ms >> ... >> announce-max: 550 ms >> multifd-compression: none >> xbzrle-cache-size: 4194304 >> max-postcopy-bandwidth: 0 >> tls-authz: '(null)' >> >> Migration parameter 'tls-authz' is used to provide the QOM ID >> of a QAuthZ subclass instance that provides the access control >> check, default is NULL. But the empty string is not a valid >> object ID, so use "" instead of the default. Although it will >> fail when lookup an object with ID "", it is harmless, just >> consistent with tls_creds. > > Yes, it's probably the best we can do given Dan's explanation that > we can't change tls_authz to be non-null. As I explained in Message-ID: <878sjv11xm.fsf@dusky.pond.sub.org>, this is actually a crash bug on some systems. The commit message neglects to mention that. Too late to fix now. Next time :) >> Also fixed the bad indentation on the last line. >> >> Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com> >> --- >> migration/migration.c | 3 ++- >> monitor/hmp-cmds.c | 2 +- >> 2 files changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/migration/migration.c b/migration/migration.c >> index c1d88ace7f..b060153ef7 100644 >> --- a/migration/migration.c >> +++ b/migration/migration.c >> @@ -790,7 +790,8 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp) >> params->has_tls_hostname = true; >> params->tls_hostname = g_strdup(s->parameters.tls_hostname); >> params->has_tls_authz = true; >> - params->tls_authz = g_strdup(s->parameters.tls_authz); >> + params->tls_authz = s->parameters.tls_authz ? \ >> + g_strdup(s->parameters.tls_authz) : g_strdup(""); > > The \ is unneeded; this isn't a macro; it's also a little shorter to do > it as: > params->tls_authz = g_strdup(s->parameters.tls_authz ? > s->parameters.tls_authz : ""); Even shorter: params->tls_authz = g_strdup(s->parameters.tls_authz ?: ""); ?: is a GNU C extension. We use it all over the place. Just FYI. I'm *not* asking for the code to be changed.
Hi Markus, On 3/30/20 3:18 PM, Markus Armbruster wrote: > "Dr. David Alan Gilbert" <dgilbert@redhat.com> writes: > >> * Mao Zhongyi (maozhongyi@cmss.chinamobile.com) wrote: >>> run: >>> (qemu) info migrate_parameters >>> announce-initial: 50 ms >>> ... >>> announce-max: 550 ms >>> multifd-compression: none >>> xbzrle-cache-size: 4194304 >>> max-postcopy-bandwidth: 0 >>> tls-authz: '(null)' >>> >>> Migration parameter 'tls-authz' is used to provide the QOM ID >>> of a QAuthZ subclass instance that provides the access control >>> check, default is NULL. But the empty string is not a valid >>> object ID, so use "" instead of the default. Although it will >>> fail when lookup an object with ID "", it is harmless, just >>> consistent with tls_creds. >> >> Yes, it's probably the best we can do given Dan's explanation that >> we can't change tls_authz to be non-null. > > As I explained in Message-ID: <878sjv11xm.fsf@dusky.pond.sub.org>, this > is actually a crash bug on some systems. The commit message neglects to > mention that. Too late to fix now. Next time :) Oops, I will continue to follow up on this issue. > >>> Also fixed the bad indentation on the last line. >>> >>> Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com> >>> --- >>> migration/migration.c | 3 ++- >>> monitor/hmp-cmds.c | 2 +- >>> 2 files changed, 3 insertions(+), 2 deletions(-) >>> >>> diff --git a/migration/migration.c b/migration/migration.c >>> index c1d88ace7f..b060153ef7 100644 >>> --- a/migration/migration.c >>> +++ b/migration/migration.c >>> @@ -790,7 +790,8 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp) >>> params->has_tls_hostname = true; >>> params->tls_hostname = g_strdup(s->parameters.tls_hostname); >>> params->has_tls_authz = true; >>> - params->tls_authz = g_strdup(s->parameters.tls_authz); >>> + params->tls_authz = s->parameters.tls_authz ? \ >>> + g_strdup(s->parameters.tls_authz) : g_strdup(""); >> >> The \ is unneeded; this isn't a macro; it's also a little shorter to do >> it as: >> params->tls_authz = g_strdup(s->parameters.tls_authz ? >> s->parameters.tls_authz : ""); > > Even shorter: > > params->tls_authz = g_strdup(s->parameters.tls_authz ?: ""); > > ?: is a GNU C extension. We use it all over the place. > > Just FYI. I'm *not* asking for the code to be changed. Yes, it does look cooler, thanks for the clarification. I would prefer to use this syntax in future patches. Thanks, Mao > >
© 2016 - 2024 Red Hat, Inc.