[Libstoragemgmt-devel] Please review errors for LSM methods idempotent issue.

Discussion:

Gris Ge

2014-08-07 08:43:44 UTC

Hi Guys,

I gathered all change methods and listed with error name when getting
duplicate invoke.

Please review and share with us your comments.
Thank you in advance.

======
# Raise NAME_CONFLICT which require user to check existing objects
# to make sure whether existing object is what they want or not.
volume_create() NAME_CONFLICT
volume_replicate() NAME_CONFLICT
access_group_create() NAME_CONFLICT
fs_create() NAME_CONFLICT
fs_snapshot_create() NAME_CONFLICT

# Raise ALREADY_DONE to indicate requested action already finished by
# previous call. User can simply ignore this error.

fs_resize() ALREADY_DONE
volume_resize() ALREADY_DONE
volume_online() ALREADY_DONE
volume_offline() ALREADY_DONE
volume_mask() ALREADY_DONE
volume_unmask() ALREADY_DONE
access_group_initiator_add() ALREADY_DONE
access_group_initiator_del() ALREADY_DONE
fs_child_dependency_rm() ALREADY_DONE

# Raise NOT_FOUND_XXX to indicate requested object is not exist. User has to
# decide ignore this error or do more check.
volume_delete() NOT_FOUND_VOLUME
access_group_delete() NOT_FOUND_ACCESS_GROUP
fs_delete() NOT_FOUND_FS
fs_snapshot_delete() NOT_FOUND_FS_SS
export_remove() NOT_FOUND_NFS_EXPORT

# Others.
fs_snapshot_restore() # just do the restore again.

export_fs() # does this method allowing overriding
# exist export?

iscsi_chap_auth() # does this method allowing overriding
# exist config?
======

--
Gris Ge

Tony Asleson

2014-08-07 14:02:16 UTC

Permalink

Post by Gris Ge
Hi Guys,
I gathered all change methods and listed with error name when getting
duplicate invoke.
Please review and share with us your comments.
Thank you in advance.
======
# Raise NAME_CONFLICT which require user to check existing objects
# to make sure whether existing object is what they want or not.
volume_create() NAME_CONFLICT
volume_replicate() NAME_CONFLICT
access_group_create() NAME_CONFLICT
fs_create() NAME_CONFLICT
fs_snapshot_create() NAME_CONFLICT

These seem reasonable.

Post by Gris Ge
# Raise ALREADY_DONE to indicate requested action already finished by
# previous call. User can simply ignore this error.
fs_resize() ALREADY_DONE
volume_resize() ALREADY_DONE
volume_online() ALREADY_DONE
volume_offline() ALREADY_DONE
volume_mask() ALREADY_DONE
volume_unmask() ALREADY_DONE
access_group_initiator_add() ALREADY_DONE
access_group_initiator_del() ALREADY_DONE
fs_child_dependency_rm() ALREADY_DONE

You are making the assumption that the user incorrectly made a duplicate
call. Any of these could happen just because the user selected the
wrong item from a list. I do agree that these all could be called and
provide no action because the array is already at the state the user is
requesting, but it's important we return that fact and I don't think the
user should just ignore it. It could be a bug in a user program or script.

Maybe a better error wording would be NO_OP, NO_OPERATION or
NO_STATE_CHANGE, to indicate that no change was made to the array.

Post by Gris Ge
# Raise NOT_FOUND_XXX to indicate requested object is not exist. User has to
# decide ignore this error or do more check.
volume_delete() NOT_FOUND_VOLUME
access_group_delete() NOT_FOUND_ACCESS_GROUP
fs_delete() NOT_FOUND_FS
fs_snapshot_delete() NOT_FOUND_FS_SS
export_remove() NOT_FOUND_NFS_EXPORT

Yes we are doing this today. We are also returning these in cases where
one of the object types is an argument and it isn't found.

Post by Gris Ge
# Others.
fs_snapshot_restore() # just do the restore again.

We really don't have the ability to know if the user did a restore at
some point in the past, so yes just do the operation if valid. This is
why we prompt the user form the command line as data loss could occur
for this operation.

Post by Gris Ge
export_fs() # does this method allowing overriding
# exist export?

Yes, you supply the entire config in one call and it replaces what you
had before.

Post by Gris Ge
iscsi_chap_auth() # does this method allowing overriding
# exist config?

Yes, this is how you change and disable chap authentication with a
single call interface.

Thanks!

Regards,
Tony

Gris Ge

2014-08-11 13:30:33 UTC

Permalink

Post by Tony Asleson

Post by Gris Ge
# Raise ALREADY_DONE to indicate requested action already finished by
# previous call. User can simply ignore this error.

Maybe a better error wording would be NO_OP, NO_OPERATION or
NO_STATE_CHANGE, to indicate that no change was made to the array.

I choose NO_STATE_CHANGE if that's OK for you.

Post by Tony Asleson
Thanks!
Regards,
Tony

Thank you Tony.
Best regards.

--
Gris Ge

Tony Asleson

2014-08-15 21:52:49 UTC

Permalink

Post by Gris Ge
# Raise NAME_CONFLICT which require user to check existing objects
# to make sure whether existing object is what they want or not.
volume_create() NAME_CONFLICT
volume_replicate() NAME_CONFLICT
access_group_create() NAME_CONFLICT
fs_create() NAME_CONFLICT
fs_snapshot_create() NAME_CONFLICT

I started a change to support this today and I'm wondering how well we
can support this.

For example, if an array doesn't care about duplicate names or doesn't
give back a specific error code stating that, we will be forced to check
all the volume names and enforce them to be unique.

We can certainly do this, but the performance ramifications could become
quite signification for large configurations.

For the library to be useful it needs to behave the same regardless of
array, but enforcing that behavior for some arrays could be ugly.

Regards,
Tony

Gris Ge

2014-08-18 14:24:57 UTC

Permalink

Post by Tony Asleson

Which one you mean:
A. Array silently ignore creation when duplicate name.
B. Array allow duplicate name and create a new volume.

For A), I have never seen any array or SDK doing that.
For B), any real array we are talking about?

Post by Tony Asleson
We can certainly do this, but the performance ramifications could become
quite signification for large configurations.

I think user application can easily handle NAME_CONFLICT by choosing
another name.
We just invoke create command to array and check return error, no
extra overload will needed for this.

Any user case can detail your concern?
In my test code, I just use time stamp(like test_vol_20140808222301).

Post by Tony Asleson
Regards,
Tony

Thank you.

--
Gris Ge

Tony Asleson

2014-08-18 15:28:47 UTC

Permalink

Post by Gris Ge

Post by Tony Asleson
I started a change to support this today and I'm wondering how well we
can support this.
For example, if an array doesn't care about duplicate names or doesn't
give back a specific error code stating that, we will be forced to check
all the volume names and enforce them to be unique.

A. Array silently ignore creation when duplicate name.
B. Array allow duplicate name and create a new volume.
For A), I have never seen any array or SDK doing that.

I haven't seen this case yet either.

Post by Gris Ge
For B), any real array we are talking about?

Dell/Compellent SMI-S provider creates a new one and the names are
identical.

For SMI-S providers that do duplicate checks we get different error
messages:

1. EMC
PLUGIN_ERROR(303): C:ERROR_CLASS_SOFTWARE F:ERROR_FAMILY_FAILED
R:1000006 An error from a sub-library call occurred : 2 : 20027 : "The
specified LUN name is already in use."

2. Dot hill
PLUGIN_ERROR(303): A duplicate name was specified.

3. Fujitsu
PLUGIN_ERROR(303): Error: volume_create rc= 32788

4. 3Par
LSM_PLUGIN_BUG(2): VV name dupe_name_test is already used by VV id 2591

Thus we need to do our own precondition checks if we want to ensure that
we can give back consistent and meaningful results.

Regards,
Tony

Gris Ge

2014-08-19 02:28:31 UTC

Permalink

Post by Tony Asleson

Post by Gris Ge
For B), any real array we are talking about?

Dell/Compellent SMI-S provider creates a new one and the names are
identical.

I just noticed in 'Masking and Mapping' profile, it ExposePaths() use
LUN name which assuming LUN name is unique at system level. Please
check smis.py _volume_mask_old() for detail.

In this case, it might be a problem for Dell array.
Can you contact Dell for this?

Post by Tony Asleson
For SMI-S providers that do duplicate checks we get different error
1. EMC
PLUGIN_ERROR(303): C:ERROR_CLASS_SOFTWARE F:ERROR_FAMILY_FAILED
R:1000006 An error from a sub-library call occurred : 2 : 20027 : "The
specified LUN name is already in use."
2. Dot hill
PLUGIN_ERROR(303): A duplicate name was specified.
3. Fujitsu
PLUGIN_ERROR(303): Error: volume_create rc= 32788
4. 3Par
LSM_PLUGIN_BUG(2): VV name dupe_name_test is already used by VV id 2591

For 1-4, we just do more try and expect(not base on error message,
base on volume querying). I will work on that.

Post by Tony Asleson
Thus we need to do our own precondition checks if we want to ensure that
we can give back consistent and meaningful results.

How about we do pre-check against known array (like: dell) only and
For other array, we do run-if-fail-then-check?

Post by Tony Asleson
Regards,
Tony

Thank you Tony.
Best regards.

--
Gris Ge

Tony Asleson

2014-08-19 13:57:59 UTC

Permalink

Post by Gris Ge

Post by Tony Asleson

Post by Gris Ge
For B), any real array we are talking about?

Dell/Compellent SMI-S provider creates a new one and the names are
identical.

I just noticed in 'Masking and Mapping' profile, it ExposePaths() use
LUN name which assuming LUN name is unique at system level. Please
check smis.py _volume_mask_old() for detail.
In this case, it might be a problem for Dell array.
Can you contact Dell for this?

Yes, I will email my contact.

Post by Gris Ge

For 1-4, we just do more try and expect(not base on error message,
base on volume querying). I will work on that.

I'm going to raise this on the smi-s mailing list. Seems wrong that we
have to resort to these types of things.

Thanks.

Regards.
-Tony