[Sheepdog] [PATCH, RFC] collie: use exit codes to distinguish between errors

[Chris Webb]

MORITA Kazutaka <morita.kazutaka at lab.ntt.co.jp> writes:

> > @@ -928,12 +932,13 @@ reread:
> >  	if (ret) {
> >  		if (ret == SD_RES_VDI_EXIST) {
> >  			fprintf(stderr, "the attribute already exists, %s\n", key);
> > +			return EXIT_BUSY;
> 
> Is EXIT_BUSY suitable for this?  We can use vdi attributes for other
> purposes than locking vdi, so I think it is better to create other
> status like EXIT_EXIST.

This is the only place I've used EXIT_BUSY, in fact, so I should probably
just rename EXIT_BUSY to EXIT_EXISTS.

The only other place I can imagine this error code being returned would also
fit the description of 'already exists': on VDI create with an already
existing VDI name. (collie doesn't have a create operation yet, though; you
can only do this via qemu-img, so it's outside the scope of this patch.)

> >  		} else if (ret == SD_RES_NO_OBJ) {
> >  			fprintf(stderr, "no such attribute, %s\n", key);
> >  		} else
> >  			fprintf(stderr, "failed to find attr oid, %s\n",
> >  				sd_strerror(ret));
> > -		return 1;
> > +		return EXIT_MISSING;
> 
> Should we return EXIT_FAILURE if (ret != SD_RES_NO_OBJ)?

I picked EXIT_MISSING because both branches quoted above report an error
about something not being found to stderr, but I'm happy to replace with
EXIT_FAILURE for the 'attr oid not found' case if you think that's more
appropriate? What circumstances can this happen in?

> >  	}
> >  
> >  	oid = attr_oid;
> > @@ -976,16 +981,16 @@ reread:
> >  
> >  		if (ret) {
> >  			fprintf(stderr, "failed to set attribute\n");
> > -			return 1;
> > +			return EXIT_FAILURE;
> 
> Should be EXIT_SYSFAIL?

Yes, you're quite right.

> >  static int cluster_shutdown(int argc, char **argv)
> >  {
> > -	shutdown_sheepdog();
> > -	return 0;
> > +	return shutdown_sheepdog();
> 
> If we use the return value of shutdown_sheepdog() here,
> shutdown_sheepdog() should return EXIT_* in all cases.

Ah yes, I missed the connect_to() error, sorry. My intention was indeed that
shutdown_sheepdog() always would return EXIT_*.

Best wishes,

Chris.