MORITA Kazutaka <[email protected]> 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.
--
sheepdog mailing list
[email protected]
http://lists.wpkg.org/mailman/listinfo/sheepdog
