[LU-5757] hsm: userspace can set about any HSM flags. Checks are inexistant.

• Status: Resolved
• Project: Lustre
• Affects Version/s: Lustre 2.7.0, Lustre 2.5.3
• Fix Version/s: Lustre 2.8.0
• Type: Bug
• Priority: Minor
• Reporter: Frank Zago (Inactive)
• Assignee: Bruno Faccini (Inactive)
• Resolution: Fixed
• Labels: hsm
• Severity: 3

Description

Creating, opening and executing the following commands on a regular file:

rc = llapi_hsm_state_set_fd(fd, HS_EXISTS, 0, -1789);
rc = llapi_hsm_state_set_fd(fd, HS_DIRTY, 0, 0);
rc = llapi_hsm_state_set_fd(fd, 0, HS_DIRTY, 0);
rc = llapi_hsm_state_set_fd(fd, HS_ARCHIVED, 0, 0);
rc = llapi_hsm_state_set_fd(fd, HS_RELEASED, 0, 0);
rc = llapi_hsm_state_set_fd(fd, HS_NORELEASE, 0, 0);
rc = llapi_hsm_state_set_fd(fd, HS_NOARCHIVE, 0, 0);
rc = llapi_hsm_state_set_fd(fd, HS_LOST, 0, 0);
rc = llapi_hsm_state_set_fd(fd, 0x00080000, 0, 0);
rc = llapi_hsm_state_set_fd(fd, 0x80000000, 0, 0);

results in this:

# ../utils/lfs hsm_state /mnt/lustre/hsm_check_test
 /mnt/lustre/hsm_check_test: (0x8008007d) released exists archived never_release never_archive lost_from_hsm, archive_id:-1789

Some of these flags should not be user settable. Unknown flags should not be set. The archive_id should not be anything but >=0 (or 1?) and <=32.

Issue Links

• is related to: LU-6453 Interop 2.7.0<->2.8 sanity-hsm test_5...

Comments

[Andreas Dilger]

Just to confirm - this can only be done on files a user owns, or by root? It definitely makes sense to add better checks for the HSM input (see also LU-2714), so a general review of all such input would be welcome.

[Frank Zago (Inactive)]

This test was done as root.

I checked with a user, and most of these commands are reject with -EPERM. I think that's wrong too because -EPERM should only be used when the user doesn't have the correct permissions on that file.

[Robert Read (Inactive)]

Why is archive_id limited to max 32?

[Robert Read (Inactive)]

I believe the LOST flag is intended to be set by userspace, since this reflects backend state that the kernel is unaware of. It is also useful to be able to clear the flags from user space to allow a file to be moved to a different HSM archive backend. (Although it would be better to clear flags automatically when the archive_id is changed, though that would be a behavior change and might break something.)

[Frank Zago (Inactive)]

Robert,

I think the 32 archive ids is a limitation of Lustre. lhsmtool_posix.c has this:

     60 /* copytool uses a 32b bitmask field to register with kuc
     61  * archive num = 0 => all
     62  * archive num from 1 to 32
     63  */
     64 #define MAX_ARCHIVE_CNT (sizeof(__u32) * 8)

[Robert Read (Inactive)]

Interesting. This appears to only be a limitation of the kuc interface. If a copytool doesn't specify an archive id when it calls register (i.e. passes a 0), then it seems it can handle any archive id.

[jacques-charles lafoucriere]

I confirm this limit to 32 comes from kuc protocol/structures. Seeing the use cases, 32 looks ok and mainly all the site will have one backend archive system and so will always use the default (0 = all)

[Frank Zago (Inactive)]

Test for this case (test51): http://review.whamcloud.com/12836/

[Gerrit Updater]

Faccini Bruno ([e-mail]) uploaded a new patch: http://review.whamcloud.com/13337
Subject: LU-5757 hsm: strengthen checks for flags and archive id
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 9379ce022fe908f3168987bfbc12214426f57fe8

[Bruno Faccini (Inactive)]

Just pushed master pach http://review.whamcloud.com/13337 which should allow for HSM flags and archive-id values to be checked.

[Frank Zago (Inactive)]

Thanks Bruno. I've updated test test in LU-5757 (not pushed yet) and it's better.

But should root even be allowed to set/unset the following flags: HS_DIRTY, HS_ARCHIVED and HS_RELEASED?

[Frank Zago (Inactive)]

Patch to add since LU-5732 went upstream

[Bruno Faccini (Inactive)]

Thanks to warn me (and to provide your add-on proposal) about the fact that some of your new tests in your patch for LU-5732 will fail. I don't know why but I was wrongly assuming that you had already anticipated the final/expected behavior and error returns from this patch in your new tests ... BTW, since I was planning to either run your new tests against this patch, or apply/relate mine on top of your or now that yours has been integrated to rebase mine, hopefully auto-tests would have also warned me!

Concerning your remark about root being or not allowed to set/unset HS_DIRTY, HS_ARCHIVED and HS_RELEASED, my feeling is that yes it should be allowed, mainly to help fixing unusual/inconsistent situations. But this can be discussed!

[Robert Read (Inactive)]

I agree there are use cases for manually setting HS_DIRTY and HS_ARCHIVED, but just setting the HS_RELEASED flag is not enough to really make a file "released". The layout also needs to changed, and I don't see a way to do that directly. However, if one can clear HS_DIRTY and manually set HS_ARCHIVED, then in theory the HUA_RELEASE request should properly mark the file as released. If the file is somehow corrupted then it's also possible to use llapi_hsm_import() to recreate the released file.

[Bruno Faccini (Inactive)]

Robert, thanks to detail this. And Yes, using the import feature along with manual flags setting was some of the repair actions I thought about to be helpful in some cases.

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch http://review.whamcloud.com/13337/
Subject: LU-5757 hsm: strengthen checks for flags and archive id
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 32bd5051a518c57e35f51b7f3c7f739b5ef91b25

[Peter Jones]

Landed for 2.8