All of lore.kernel.org
 help / color / mirror / Atom feed
From: Zdenek Kabelac <zkabelac@redhat.com>
To: lvm-devel@redhat.com
Subject: [PATCH] dmsetup: treat no devices found as error
Date: Wed, 17 Feb 2016 11:08:22 +0100	[thread overview]
Message-ID: <56C44696.8020806@redhat.com> (raw)
In-Reply-To: <1455695471-5710-1-git-send-email-hare@suse.de>

Dne 17.2.2016 v 08:51 Hannes Reinecke napsal(a):
> When calling 'dmsetup ls' and no devices are found the program will
> print out 'No devices found' and exit normally.
> This makes it really hard for the calling application to determine
> if the output 'No devices found' is a valid device or not.
> This patch moves the 'No devices found' string to stderr and
> sets the return code to non-0 to allow calling applications to
> better differentiate here.
>
> Signed-off-by: Hannes Reinecke <hare@suse.com>
> ---
>   tools/dmsetup.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/tools/dmsetup.c b/tools/dmsetup.c
> index 4db6004..3629931 100644
> --- a/tools/dmsetup.c
> +++ b/tools/dmsetup.c
> @@ -1835,7 +1835,8 @@ static int _process_all(const struct command *cmd, const char *subcommand, int a
>
>   	if (!names->dev) {
>   		if (!silent)
> -			printf("No devices found\n");
> +			fprintf(stderr, "No devices found\n");
> +		r = 0;
>   		goto out;
>   	}
>

Hi

NACKing this patch.

dmsetup cannot change behavior this way - it'd cause major regression for 
existing script using dmsetup (rule #1 - no regressions).


'No devices found'  when you list empty table  is correct & expected state.

So return code is  'success' in this case
(and as success output goes to stdout).


I'm actually not even sure why would you want to base ANY script on error code 
as empty dm table is not an error (btw with this logic   'dmsetup table'  for 
empty table would also need to return error and so on...)


You may get mapping pairs   major:minor with 'dmsetup ls -o devno' :

"vg-lvol0	(253:0)"
...


or with 'dmsetup ls -o blkdevname' pairs like this:

"vg-lvol0	(dm-0)"
...


and when no device exists:
"No devices found"


and all you need to do is just check for string on output line (stdout).
(i.e. line does not have '(', ':', ')'  --> so it's empty table --> no device...)

Technically I don't see much difference in checking string or error code 
(which is just not an error).


On the other hand - when 'dmsetup' has a real error - you should take correct 
steps to work around some real error (e.g. out-of-mem system).
But usually 'error' code behavior is 'exception' state - and as exception
it's typically more 'expensive' in code handling...


Regards

Zdenek



      reply	other threads:[~2016-02-17 10:08 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-17  7:51 [PATCH] dmsetup: treat no devices found as error Hannes Reinecke
2016-02-17 10:08 ` Zdenek Kabelac [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56C44696.8020806@redhat.com \
    --to=zkabelac@redhat.com \
    --cc=lvm-devel@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.