Max Reitz <mre...@redhat.com> writes: > Currently, bdrv_reopen_prepare() assumes that all BDS options are > strings. However, this is not the case if the BDS has been created > through the json: pseudo-protocol or blockdev-add. > > Note that the user-invokable reopen command is an HMP command, so you > can only specify strings there. Therefore, specifying a non-string > option with the "same" value as it was when originally created will now > return an error because the values are supposedly similar (and there is > no way for the user to circumvent this but to just not specify the > option again -- however, this is still strictly better than just > crashing). > > Reviewed-by: Kevin Wolf <kw...@redhat.com> > Signed-off-by: Max Reitz <mre...@redhat.com> > --- > block.c | 15 +++------------ > 1 file changed, 3 insertions(+), 12 deletions(-) > > diff --git a/block.c b/block.c > index fa1d06d..23424d5 100644 > --- a/block.c > +++ b/block.c > @@ -2950,19 +2950,10 @@ int bdrv_reopen_prepare(BDRVReopenState > *reopen_state, BlockReopenQueue *queue, > const QDictEntry *entry = qdict_first(reopen_state->options); > > do { > - QString *new_obj = qobject_to_qstring(entry->value); > - const char *new = qstring_get_str(new_obj); > - /* > - * Caution: while qdict_get_try_str() is fine, getting > - * non-string types would require more care. When > - * bs->options come from -blockdev or blockdev_add, its > - * members are typed according to the QAPI schema, but > - * when they come from -drive, they're all QString. > - */
Your commit message makes me suspect this comment still applies in some form. Does it? > - const char *old = qdict_get_try_str(reopen_state->bs->options, > - entry->key); > + QObject *new = entry->value; > + QObject *old = qdict_get(reopen_state->bs->options, entry->key); > > - if (!old || strcmp(new, old)) { > + if (!qobject_is_equal(new, old)) { > error_setg(errp, "Cannot change the option '%s'", > entry->key); > ret = -EINVAL; > goto error;