On Mon, Feb 13, 2017 at 6:57 AM, Stefan Hajnoczi <stefa...@gmail.com> wrote:
> On Tue, Feb 07, 2017 at 08:18:13PM -0800, Ashish Mittal wrote:
>> +static int vxhs_parse_uri(const char *filename, QDict *options)
>> +{
>> + URI *uri = NULL;
>> + char *hoststr, *portstr;
>> + char *port;
>> + int ret = 0;
>> +
>> + trace_vxhs_parse_uri_filename(filename);
>> + uri = uri_parse(filename);
>> + if (!uri || !uri->server || !uri->path) {
>> + uri_free(uri);
>> + return -EINVAL;
>> + }
>> +
>> + hoststr = g_strdup(VXHS_OPT_SERVER".host");
>> + qdict_put(options, hoststr, qstring_from_str(uri->server));
>> + g_free(hoststr);
>> +
>> + portstr = g_strdup(VXHS_OPT_SERVER".port");
>> + if (uri->port) {
>> + port = g_strdup_printf("%d", uri->port);
>> + qdict_put(options, portstr, qstring_from_str(port));
>> + g_free(port);
>> + }
>> + g_free(portstr);
>> +
>> + if (strstr(uri->path, "vxhs") == NULL) {
>> + qdict_put(options, "vdisk-id", qstring_from_str(uri->path));
>> + }
>> +
>> + trace_vxhs_parse_uri_hostinfo(1, uri->server, uri->port);
>
> What is the purpose of the first argument?
>
It used to be a placeholder for the host index, which is now only 1. I
will remove it.
>> + str = g_strdup_printf(VXHS_OPT_SERVER".");
>> + qdict_extract_subqdict(options, &backing_options, str);
>> +
>> + /* Create opts info from runtime_tcp_opts list */
>> + tcp_opts = qemu_opts_create(&runtime_tcp_opts, NULL, 0, &error_abort);
>> + qemu_opts_absorb_qdict(tcp_opts, backing_options, &local_err);
>> + if (local_err) {
>> + qdict_del(backing_options, str);
>
> What is qdict_del(backing_options, VXHS_OPT_SERVER".") supposed to do?
> The same call is made further down too.
>
Per my understanding, qdict_del() is to free the 'server.' entries
within the subqdict.
qdict_extract_subqdict() allocates a subqdict and populates it with
the entries based on the pattern we pass. In this case 'server.'.
>> + qemu_opts_del(tcp_opts);
>> + ret = -EINVAL;
>> + goto out;
>> + }
>> +
>> + server_host_opt = qemu_opt_get(tcp_opts, VXHS_OPT_HOST);
>> + if (!server_host_opt) {
>> + error_setg(&local_err, QERR_MISSING_PARAMETER,
>> + VXHS_OPT_SERVER"."VXHS_OPT_HOST);
>> + ret = -EINVAL;
>> + goto out;
>
> Missing qemu_opts_del(tcp_opts).
>
Will fix this!
>> + }
>> +
>> + if (strlen(server_host_opt) > MAXHOSTNAMELEN) {
>> + error_setg(errp, "server.host cannot be more than %d characters",
>> + MAXHOSTNAMELEN);
>> + ret = -EINVAL;
>> + goto out;
>
> Missing qemu_opts_del(tcp_opts).
>
Will fix this!
>> @@ -5114,6 +5147,7 @@ echo "tcmalloc support $tcmalloc"
>> echo "jemalloc support $jemalloc"
>> echo "avx2 optimization $avx2_opt"
>> echo "replication support $replication"
>> +echo "VxHS block device $vxhs"
>>
>> if test "$sdl_too_old" = "yes"; then
>> echo "-> Your SDL version is too old - please upgrade to have SDL support"
>> @@ -5729,6 +5763,12 @@ if test "$pthread_setname_np" = "yes" ; then
>> echo "CONFIG_PTHREAD_SETNAME_NP=y" >> $config_host_mak
>> fi
>>
>> +if test "$vxhs" = "yes" ; then
>> + echo "CONFIG_VXHS=y" >> $config_host_mak
>> + echo "VXHS_CFLAGS=$vxhs_cflags" >> $config_host_mak
>
> Please drop this unused variable.
Will fix this!
Thanks,
Ashish
