Skip to content

add: rawstor boilerplate#1

Draft
VasilyStepanov wants to merge 1 commit into
rawstorfrom
add/rawstor
Draft

add: rawstor boilerplate#1
VasilyStepanov wants to merge 1 commit into
rawstorfrom
add/rawstor

Conversation

@VasilyStepanov

Copy link
Copy Markdown
Member

No description provided.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread src/conf/storage_conf.c
"logical", "disk", "iscsi",
"iscsi-direct", "scsi", "mpath",
"rbd", "sheepdog", "gluster",
"rawstor", "rbd", "sheepdog", "gluster",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

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",

Comment thread src/conf/storage_conf.c
Comment on lines +233 to +239
{.poolType = VIR_STORAGE_POOL_RAWSTOR,
.poolOptions = {
.flags = (VIR_STORAGE_POOL_SOURCE_HOST |
VIR_STORAGE_POOL_SOURCE_NETWORK |
VIR_STORAGE_POOL_SOURCE_NAME),
}
},

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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".

Suggested change
* storage_backend_iscsi.h: storage backend for iSCSI handling
* storage_backend_rawstor.h: storage backend for Rawstor handling

Comment on lines +8 to +10
// .refreshPool = virStorageBackendMyVhostRefreshPool,
// .createVol = virStorageBackendMyVhostCreateVol,
// ... остальные обработчики

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

Suggested change
// .refreshPool = virStorageBackendMyVhostRefreshPool,
// .createVol = virStorageBackendMyVhostCreateVol,
// ... остальные обработчики
/* TODO: implement storage pool/volume inherited/required callbacks */

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant