[libvirt] [PATCH 3/5] virStorageBackendIQNFound: Rework iscsiadm output parsing
Peter Krempa
pkrempa at redhat.com
Mon Jul 2 07:39:35 UTC 2018
On Fri, Jun 29, 2018 at 17:01:49 +0200, Michal Privoznik wrote:
> Firstly, we can utilize virCommandSetOutputBuffer() API which
> will collect the command output for us. Secondly, sscanf()-ing
> through each line is easier to understand (and more robust) than
> jumping over a string with strchr().
>
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
> src/util/viriscsi.c | 85 +++++++++++++++++++++--------------------------------
> 1 file changed, 34 insertions(+), 51 deletions(-)
>
> diff --git a/src/util/viriscsi.c b/src/util/viriscsi.c
> index 2e55b3c10b..44788056fd 100644
> --- a/src/util/viriscsi.c
> +++ b/src/util/viriscsi.c
[...]
> @@ -117,71 +116,56 @@ static int
> virStorageBackendIQNFound(const char *initiatoriqn,
> char **ifacename)
> {
> - int ret = IQN_ERROR, fd = -1;
> - char ebuf[64];
> - FILE *fp = NULL;
> - char *line = NULL, *newline = NULL, *iqn = NULL, *token = NULL;
> + int ret = IQN_ERROR;
> + char *outbuf = NULL;
> + char *line = NULL;
> + char *iface = NULL;
> + char *iqn = NULL;
> virCommandPtr cmd = virCommandNewArgList(ISCSIADM,
> "--mode", "iface", NULL);
>
> *ifacename = NULL;
>
> - if (VIR_ALLOC_N(line, LINE_SIZE) != 0) {
> - virReportError(VIR_ERR_INTERNAL_ERROR,
> - _("Could not allocate memory for output of '%s'"),
> - ISCSIADM);
> + virCommandSetOutputBuffer(cmd, &outbuf);
> + if (virCommandRun(cmd, NULL) < 0)
> goto cleanup;
> - }
>
> - memset(line, 0, LINE_SIZE);
> + /* Example of data we are dealing with:
> + * default tcp,<empty>,<empty>,<empty>,<empty>
> + * iser iser,<empty>,<empty>,<empty>,<empty>
> + * libvirt-iface-253db048 tcp,<empty>,<empty>,<empty>,iqn.2017-03.com.user:client
> + */
>
> - virCommandSetOutputFD(cmd, &fd);
> - if (virCommandRunAsync(cmd, NULL) < 0)
> - goto cleanup;
> + line = outbuf;
> + while (line) {
This is severely misleading and makes this loop technically infinite. Or
just checks that output buffer was not null. Both operations should be
made explicit. Or you meant *line or line[0]
> + char *newline;
> + int num;
>
> - if ((fp = VIR_FDOPEN(fd, "r")) == NULL) {
> - virReportError(VIR_ERR_INTERNAL_ERROR,
> - _("Failed to open stream for file descriptor "
> - "when reading output from '%s': '%s'"),
> - ISCSIADM, virStrerror(errno, ebuf, sizeof(ebuf)));
> - goto cleanup;
> - }
> + if (!(newline = strchr(line, '\n')))
> + break;
This is ther reason why it's working at this point.
>
> - while (fgets(line, LINE_SIZE, fp) != NULL) {
> - newline = strrchr(line, '\n');
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20180702/a6732594/attachment-0001.sig>
More information about the libvir-list
mailing list