openstack
/
cinder-specs
Code Issues Proposed changes

Update patch set 5 

Patch Set 5: Code-Review-1

(23 comments)

Patch-set: 5
Reviewer: Gerrit User 27615 <27615@4a232e18-c5a9-48ee-94c0-e04e7cca6543>
Label: Code-Review=-1, 30fdb5a79dbd1e462e3c2093456e41252ccb21a1
This commit is contained in:
Gerrit User 27615 2023-06-09 15:21:30 +00:00 committed by Gerrit Code Review
parent 87220b73ce
commit c3490c3add
1 changed files with 499 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

499
39b8be033e6eb2767022944faed5c1762647ef3b
View File

@ -50,6 +50,505 @@
"message": "Needs technical review.",
"revId": "39b8be033e6eb2767022944faed5c1762647ef3b",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": false,
"key": {
"uuid": "ea4327b4_d003b51d",
"filename": "/PATCHSET_LEVEL",
"patchSetId": 5
},
"lineNbr": 0,
"author": {
"id": 27615
},
"writtenOn": "2023-06-09T15:21:30Z",
"side": 1,
"message": "few comments inline",
"revId": "39b8be033e6eb2767022944faed5c1762647ef3b",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "95b2860e_919c50bc",
"filename": "specs/2023.2/dedicated-volume-backup-status-field.rst",
"patchSetId": 5
},
"lineNbr": 13,
"author": {
"id": 27615
},
"writtenOn": "2023-06-09T15:21:30Z",
"side": 1,
"message": "what do we mean by re-attachments here?\nI think what we\u0027re trying to convey is the backup status \u0027backing-up\u0027 disallows volume to be used for attachment workflow i.e. \u0027reserved\u0027, \u0027attaching\u0027, \u0027in-use\u0027 etc for other operations",
"range": {
"startLine": 13,
"startChar": 24,
"endLine": 13,
"endChar": 38
},
"revId": "39b8be033e6eb2767022944faed5c1762647ef3b",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "2f2432dd_6f9934ca",
"filename": "specs/2023.2/dedicated-volume-backup-status-field.rst",
"patchSetId": 5
},
"lineNbr": 15,
"author": {
"id": 27615
},
"writtenOn": "2023-06-09T15:21:30Z",
"side": 1,
"message": "nit: too many blank lines",
"revId": "39b8be033e6eb2767022944faed5c1762647ef3b",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "be21016d_97a095ef",
"filename": "specs/2023.2/dedicated-volume-backup-status-field.rst",
"patchSetId": 5
},
"lineNbr": 20,
"author": {
"id": 27615
},
"writtenOn": "2023-06-09T15:21:30Z",
"side": 1,
"message": "do we have this as a convention anywhere? I think it\u0027s the \u0027status\u0027 field so we can say it as\n\nvolume\u0027s `status` field",
"range": {
"startLine": 20,
"startChar": 36,
"endLine": 20,
"endChar": 49
},
"revId": "39b8be033e6eb2767022944faed5c1762647ef3b",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "a9b9235f_de9c6b2d",
"filename": "specs/2023.2/dedicated-volume-backup-status-field.rst",
"patchSetId": 5
},
"lineNbr": 21,
"author": {
"id": 27615
},
"writtenOn": "2023-06-09T15:21:30Z",
"side": 1,
"message": "maybe it\u0027s just me but this sounds like we are launching a service, container or something. Can we reword this sentence as,\n\nCurrently all cinder tasks use the `status` field to check for suitable\nvolume status while performing any particular operation.",
"range": {
"startLine": 21,
"startChar": 29,
"endLine": 21,
"endChar": 37
},
"revId": "39b8be033e6eb2767022944faed5c1762647ef3b",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "fc417167_2cdf34a7",
"filename": "specs/2023.2/dedicated-volume-backup-status-field.rst",
"patchSetId": 5
},
"lineNbr": 23,
"author": {
"id": 27615
},
"writtenOn": "2023-06-09T15:21:30Z",
"side": 1,
"message": "This is not always the case. we also have attached-status and migration-status fields that are for specific operations. Although the modify the volume status, the volume status alone can\u0027t provide details about the whole operation.",
"range": {
"startLine": 23,
"startChar": 5,
"endLine": 23,
"endChar": 65
},
"revId": "39b8be033e6eb2767022944faed5c1762647ef3b",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "a14d7164_8eac39b2",
"filename": "specs/2023.2/dedicated-volume-backup-status-field.rst",
"patchSetId": 5
},
"lineNbr": 30,
"author": {
"id": 27615
},
"writtenOn": "2023-06-09T15:21:30Z",
"side": 1,
"message": "can we also mention the state as an example here,\n\nbackups (backing-up)",
"range": {
"startLine": 30,
"startChar": 69,
"endLine": 30,
"endChar": 76
},
"revId": "39b8be033e6eb2767022944faed5c1762647ef3b",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "d836a361_24bab9d3",
"filename": "specs/2023.2/dedicated-volume-backup-status-field.rst",
"patchSetId": 5
},
"lineNbr": 32,
"author": {
"id": 27615
},
"writtenOn": "2023-06-09T15:21:30Z",
"side": 1,
"message": "actually we create a volume or snapshot from the main volume depending on the value of config option \u0027backup_use_temp_snapshot\u0027.\nIf it\u0027s true we create snapshots else we clone the volume.\n\nNot that it matters a lot here but i think it\u0027s useful to provide correct information relating to the workflow.",
"range": {
"startLine": 32,
"startChar": 47,
"endLine": 32,
"endChar": 72
},
"revId": "39b8be033e6eb2767022944faed5c1762647ef3b",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "5b736a50_fa2534ed",
"filename": "specs/2023.2/dedicated-volume-backup-status-field.rst",
"patchSetId": 5
},
"lineNbr": 50,
"author": {
"id": 27615
},
"writtenOn": "2023-06-09T15:21:30Z",
"side": 1,
"message": "I think we should avoid using this term since it\u0027s not a known cinder terminology. We can probably describe it briefly as,\n\nCurrently detaching volume from source compute node and attaching it to the destination compute node during an instance live-migration is...",
"range": {
"startLine": 50,
"startChar": 39,
"endLine": 50,
"endChar": 52
},
"revId": "39b8be033e6eb2767022944faed5c1762647ef3b",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "2ac677df_d69ac9e4",
"filename": "specs/2023.2/dedicated-volume-backup-status-field.rst",
"patchSetId": 5
},
"lineNbr": 72,
"author": {
"id": 27615
},
"writtenOn": "2023-06-09T15:21:30Z",
"side": 1,
"message": "it\u0027s the \u0027status\u0027 field",
"range": {
"startLine": 72,
"startChar": 28,
"endLine": 72,
"endChar": 41
},
"revId": "39b8be033e6eb2767022944faed5c1762647ef3b",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "8a5e157f_0dc57fa2",
"filename": "specs/2023.2/dedicated-volume-backup-status-field.rst",
"patchSetId": 5
},
"lineNbr": 79,
"author": {
"id": 27615
},
"writtenOn": "2023-06-09T15:21:30Z",
"side": 1,
"message": "status",
"range": {
"startLine": 79,
"startChar": 64,
"endLine": 79,
"endChar": 77
},
"revId": "39b8be033e6eb2767022944faed5c1762647ef3b",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "85ab925c_9c763cc3",
"filename": "specs/2023.2/dedicated-volume-backup-status-field.rst",
"patchSetId": 5
},
"lineNbr": 107,
"author": {
"id": 27615
},
"writtenOn": "2023-06-09T15:21:30Z",
"side": 1,
"message": "nit: volume status",
"range": {
"startLine": 107,
"startChar": 23,
"endLine": 107,
"endChar": 36
},
"revId": "39b8be033e6eb2767022944faed5c1762647ef3b",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "f0111c30_2246c9bd",
"filename": "specs/2023.2/dedicated-volume-backup-status-field.rst",
"patchSetId": 5
},
"lineNbr": 127,
"author": {
"id": 27615
},
"writtenOn": "2023-06-09T15:21:30Z",
"side": 1,
"message": "I don\u0027t completely understand this translation? can you explain\nIn the old microversion, only the status field will be there and in new MV, there will be two fields so the translation should be from backup_status field to status field.",
"range": {
"startLine": 127,
"startChar": 0,
"endLine": 127,
"endChar": 71
},
"revId": "39b8be033e6eb2767022944faed5c1762647ef3b",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "0e8dca2b_295b09ac",
"filename": "specs/2023.2/dedicated-volume-backup-status-field.rst",
"patchSetId": 5
},
"lineNbr": 134,
"author": {
"id": 27615
},
"writtenOn": "2023-06-09T15:21:30Z",
"side": 1,
"message": "I think this can be covered in the reset-state command?",
"range": {
"startLine": 132,
"startChar": 59,
"endLine": 134,
"endChar": 42
},
"revId": "39b8be033e6eb2767022944faed5c1762647ef3b",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "bbefe469_b48fdf1e",
"filename": "specs/2023.2/dedicated-volume-backup-status-field.rst",
"patchSetId": 5
},
"lineNbr": 165,
"author": {
"id": 27615
},
"writtenOn": "2023-06-09T15:21:30Z",
"side": 1,
"message": "can we also add a volume response JSON body?\nIt should be same as volume show but with additional backup_status field",
"revId": "39b8be033e6eb2767022944faed5c1762647ef3b",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "2d4058d0_adda3419",
"filename": "specs/2023.2/dedicated-volume-backup-status-field.rst",
"patchSetId": 5
},
"lineNbr": 198,
"author": {
"id": 27615
},
"writtenOn": "2023-06-09T15:21:30Z",
"side": 1,
"message": "you mean backup_status field?",
"range": {
"startLine": 198,
"startChar": 39,
"endLine": 198,
"endChar": 50
},
"revId": "39b8be033e6eb2767022944faed5c1762647ef3b",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "ef6b2a24_61aa23be",
"filename": "specs/2023.2/dedicated-volume-backup-status-field.rst",
"patchSetId": 5
},
"lineNbr": 243,
"author": {
"id": 27615
},
"writtenOn": "2023-06-09T15:21:30Z",
"side": 1,
"message": "are we only planning to move backup states in the backup_status field? also currently we have the \u0027restoring\u0027 state in volume stats, did you mean backup_status?",
"range": {
"startLine": 242,
"startChar": 0,
"endLine": 243,
"endChar": 25
},
"revId": "39b8be033e6eb2767022944faed5c1762647ef3b",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "afa83431_cd700200",
"filename": "specs/2023.2/dedicated-volume-backup-status-field.rst",
"patchSetId": 5
},
"lineNbr": 248,
"author": {
"id": 27615
},
"writtenOn": "2023-06-09T15:21:30Z",
"side": 1,
"message": "nit: too many blank lines",
"revId": "39b8be033e6eb2767022944faed5c1762647ef3b",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "5065221b_6151a836",
"filename": "specs/2023.2/dedicated-volume-backup-status-field.rst",
"patchSetId": 5
},
"lineNbr": 267,
"author": {
"id": 27615
},
"writtenOn": "2023-06-09T15:21:30Z",
"side": 1,
"message": "can we be specific with the work items? like adding a backup_status field in the volumes table",
"range": {
"startLine": 265,
"startChar": 0,
"endLine": 267,
"endChar": 34
},
"revId": "39b8be033e6eb2767022944faed5c1762647ef3b",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "8983b128_1fd2afed",
"filename": "specs/2023.2/dedicated-volume-backup-status-field.rst",
"patchSetId": 5
},
"lineNbr": 280,
"author": {
"id": 27615
},
"writtenOn": "2023-06-09T15:21:30Z",
"side": 1,
"message": "i still think this should be a part of reset-state operation",
"range": {
"startLine": 280,
"startChar": 0,
"endLine": 280,
"endChar": 53
},
"revId": "39b8be033e6eb2767022944faed5c1762647ef3b",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "f947a1cd_ffeba53c",
"filename": "specs/2023.2/dedicated-volume-backup-status-field.rst",
"patchSetId": 5
},
"lineNbr": 281,
"author": {
"id": 27615
},
"writtenOn": "2023-06-09T15:21:30Z",
"side": 1,
"message": "One item: Introduce a new microversion to allow backups to be performed in isolation from other operations that use volume\u0027s status field as a locking mechanism.",
"revId": "39b8be033e6eb2767022944faed5c1762647ef3b",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "c400920a_3d52f2a9",
"filename": "specs/2023.2/dedicated-volume-backup-status-field.rst",
"patchSetId": 5
},
"lineNbr": 311,
"author": {
"id": 27615
},
"writtenOn": "2023-06-09T15:21:30Z",
"side": 1,
"message": "status",
"range": {
"startLine": 311,
"startChar": 58,
"endLine": 311,
"endChar": 71
},
"revId": "39b8be033e6eb2767022944faed5c1762647ef3b",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "c8d0cfb7_9d96d23d",
"filename": "specs/2023.2/dedicated-volume-backup-status-field.rst",
"patchSetId": 5
},
"lineNbr": 317,
"author": {
"id": 27615
},
"writtenOn": "2023-06-09T15:21:30Z",
"side": 1,
"message": "nit: status",
"range": {
"startLine": 317,
"startChar": 3,
"endLine": 317,
"endChar": 16
},
"revId": "39b8be033e6eb2767022944faed5c1762647ef3b",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
}
]
}