add: rawstor boilerplate#1
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces support for a new 'Rawstor' storage pool backend, including build configuration options, enum definitions, registration logic, and boilerplate driver files. The code review highlights several critical and minor issues: a mismatch between the virStoragePool enum and its string representation in VIR_ENUM_IMPL which will break XML parsing, minor formatting and indentation issues in the storage pool options, copy-paste remnants in the header comments of the new backend, and commented-out placeholder code with non-English comments that should be cleaned up.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| "logical", "disk", "iscsi", | ||
| "iscsi-direct", "scsi", "mpath", | ||
| "rbd", "sheepdog", "gluster", | ||
| "rawstor", "rbd", "sheepdog", "gluster", |
There was a problem hiding this comment.
The order of elements in VIR_ENUM_IMPL must exactly match the virStoragePoolType enum in src/conf/storage_conf.h. Currently, VIR_STORAGE_POOL_RAWSTOR is placed at the end of the enum (index 14), but "rawstor" is inserted here at index 9 (before "rbd"). This mismatch will break the mapping between the enum and its string representation, causing bugs in XML parsing/formatting and API calls.
Please move "rawstor" to the end of the VIR_ENUM_IMPL list (after "vstorage").
"rbd", "sheepdog", "gluster",| {.poolType = VIR_STORAGE_POOL_RAWSTOR, | ||
| .poolOptions = { | ||
| .flags = (VIR_STORAGE_POOL_SOURCE_HOST | | ||
| VIR_STORAGE_POOL_SOURCE_NETWORK | | ||
| VIR_STORAGE_POOL_SOURCE_NAME), | ||
| } | ||
| }, |
There was a problem hiding this comment.
The indentation of the closing brace for .poolOptions is incorrect (6 spaces instead of 5), and it is missing a trailing comma. To match the style of other entries in this array, it should be indented with 5 spaces and have a trailing comma.
{.poolType = VIR_STORAGE_POOL_RAWSTOR,
.poolOptions = {
.flags = (VIR_STORAGE_POOL_SOURCE_HOST |
VIR_STORAGE_POOL_SOURCE_NETWORK |
VIR_STORAGE_POOL_SOURCE_NAME),
},
},| @@ -0,0 +1,23 @@ | |||
| /* | |||
| * storage_backend_iscsi.h: storage backend for iSCSI handling | |||
There was a problem hiding this comment.
The header comment block contains copy-paste remnants from storage_backend_iscsi.h. It mentions storage_backend_iscsi.h and "iSCSI handling" instead of storage_backend_rawstor.h and "Rawstor handling".
| * storage_backend_iscsi.h: storage backend for iSCSI handling | |
| * storage_backend_rawstor.h: storage backend for Rawstor handling |
| // .refreshPool = virStorageBackendMyVhostRefreshPool, | ||
| // .createVol = virStorageBackendMyVhostCreateVol, | ||
| // ... остальные обработчики |
There was a problem hiding this comment.
The boilerplate code contains commented-out placeholder fields with Russian comments (// ... остальные обработчики). These should be removed or replaced with a standard English TODO comment to maintain code quality and consistency in the codebase.
| // .refreshPool = virStorageBackendMyVhostRefreshPool, | |
| // .createVol = virStorageBackendMyVhostCreateVol, | |
| // ... остальные обработчики | |
| /* TODO: implement storage pool/volume inherited/required callbacks */ |
No description provided.