Am 08.12.2016 um 14:55 hat Daniel P. Berrange geschrieben:
> On Thu, Dec 08, 2016 at 02:23:05PM +0100, Kevin Wolf wrote:
> > This adds blockdev-add support to the iscsi block driver.
> >
> > Note that this is only compile tested at this point. Jeff is going to
> > take over from here and bring the series to a mergable state.
> >
> > Kevin Wolf (6):
> > iscsi: Split URL into individual options
> > iscsi: Handle -iscsi user/password in bdrv_parse_filename()
> > iscsi: Add initiator-name option
> > iscsi: Add header-digest option
> > iscsi: Add timeout option
> > iscsi: Add blockdev-add support
> >
> > block/iscsi.c | 342
> > ++++++++++++++++++++++++++++++---------------------
> > qapi/block-core.json | 74 ++++++++++-
> > 2 files changed, 272 insertions(+), 144 deletions(-)
>
> This series works as well as my series does, once you apply this fix
> for the crash bug I mention:
>
> diff --git a/block/iscsi.c b/block/iscsi.c
> index 6a11cdd..320e56a 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -1524,7 +1524,7 @@ static void iscsi_parse_iscsi_option(const char
> *target, QDict *options)
> {
> QemuOptsList *list;
> QemuOpts *opts;
> - const char *user, *initiator_name, *header_digest, *timeout;
> + const char *user, *initiator_name, *header_digest, *timeout, *password,
> *password_secret;
>
> list = qemu_find_opts("iscsi");
> if (!list) {
> @@ -1542,10 +1542,14 @@ static void iscsi_parse_iscsi_option(const char
> *target, QDict *options)
> user = qemu_opt_get(opts, "user");
> if (user) {
> qdict_set_default_str(options, "user", user);
> - qdict_set_default_str(options, "password",
> - qemu_opt_get(opts, "password"));
> - qdict_set_default_str(options, "password-secret",
> - qemu_opt_get(opts, "password-secret"));
> + }
> + password = qemu_opt_get(opts, "password");
> + if (password) {
> + qdict_set_default_str(options, "password", password);
> + }
> + password_secret = qemu_opt_get(opts, "password-secret");
> + if (password_secret) {
> + qdict_set_default_str(options, "password-secret", password_secret);
> }
Looks good to me. Though I would keep these inside the 'if (user)'
block.
The driver silently ignores password/password-secret if user isn't given
(this is how it worked even before this patch). The interesting case
where it makes a difference is if you give one option directly to the
block driver and the other one with -iscsi. I don't think we want to
allow that, both options should come from the same place.
Kevin
