Nir Soffer has posted comments on this change.

Change subject: constants: Define temporary volume indicators
......................................................................


Patch Set 8: Code-Review-1

(1 comment)

Check the comments

https://gerrit.ovirt.org/#/c/49544/8/vdsm/storage/constants.py
File vdsm/storage/constants.py:

Line 22: SECTOR_SIZE = 512
Line 23: 
Line 24: # Temporary volume indicators
Line 25: TEMP_VOL_FILEEXT = ".tmp"        # Appended to FileVolume metadata 
filenames
Line 26: TEMP_VOL_TAG = "OVIRT_VOL_TEMP"  # Tag applied to temporary 
BlockVolume LVs
How about TEMP_VOL_LVTAG?

Symmetric with FILEEXT and give even more information.

About the value, we should be more consistent, so:

    TEMP_VOL_FILEEXT = ".temp"
    TEMP_VOL_LVTAG = "OVIRT_VOL_TEMP"

We may like be more formal:

    TEMP_VOL_FILEEXT = ".temporary"
    TEMP_VOL_LVTAG = "OVIRT_VOL_TEMPORARY"

Or maybe:

    TEMP_VOL_FILEEXT = ".volatile"
    TEMP_VOL_LVTAG = "OVIRT_VOLATILE"

I like the last one :-)

See http://www.thesaurus.com/browse/temporary


-- 
To view, visit https://gerrit.ovirt.org/49544
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id14e79d400d6936c8bb0da5dec39e3f9fb60c803
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <ali...@redhat.com>
Gerrit-Reviewer: Adam Litke <ali...@redhat.com>
Gerrit-Reviewer: Ala Hino <ah...@redhat.com>
Gerrit-Reviewer: Amit Aviram <aavi...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Freddy Rolland <froll...@redhat.com>
Gerrit-Reviewer: Greg Padgett <gpadg...@redhat.com>
Gerrit-Reviewer: Idan Shaby <ish...@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot <lara...@redhat.com>
Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com>
Gerrit-Reviewer: Tal Nisan <tni...@redhat.com>
Gerrit-Reviewer: Vered Volansky <vvola...@redhat.com>
Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org>
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to