On 07/24/2018 06:19 PM, Clementine Hayat wrote:
> 2018-07-24 14:24 GMT+02:00 Michal Privoznik <mprivozn@xxxxxxxxxx>:
>> On 07/23/2018 08:43 PM, clem@xxxxxxxxxxxx wrote:
>>> From: Clementine Hayat <clem@xxxxxxxxxxxx>
>>>
>>> Introducing the pool as a noop. Integration inside the build
>>> system. Implementation will be in the following commits.
>>>
>>> Signed-off-by: Clementine Hayat <clem@xxxxxxxxxxxx>
>>> ---
>>> configure.ac | 6 ++-
>>> m4/virt-storage-iscsi-direct.m4 | 41 +++++++++++++++
>>> src/conf/domain_conf.c | 4 ++
>>> src/conf/storage_conf.c | 33 ++++++++++--
>>> src/conf/storage_conf.h | 1 +
>>> src/conf/virstorageobj.c | 2 +
>>> src/storage/Makefile.inc.am | 22 ++++++++
>>> src/storage/storage_backend.c | 6 +++
>>> src/storage/storage_backend_iscsi_direct.c | 58 ++++++++++++++++++++++
>>> src/storage/storage_backend_iscsi_direct.h | 6 +++
>>> src/storage/storage_driver.c | 1 +
>>> tools/virsh-pool.c | 3 ++
>>> 12 files changed, 178 insertions(+), 5 deletions(-)
>>> create mode 100644 m4/virt-storage-iscsi-direct.m4
>>> create mode 100644 src/storage/storage_backend_iscsi_direct.c
>>> create mode 100644 src/storage/storage_backend_iscsi_direct.h
>>
>>> @@ -803,6 +814,19 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt)
>>> goto error;
>>> }
>>>
>>> + if (ret->type == VIR_STORAGE_POOL_ISCSI_DIRECT) {
>>> + if (!ret->source.initiator.iqn) {
>>> + virReportError(VIR_ERR_XML_ERROR, "%s",
>>> + _("missing storage pool initiator iqn"));
>>> + goto error;
>>> + }
>>> + if (!ret->source.ndevice) {
>>> + virReportError(VIR_ERR_XML_ERROR, "%s",
>>> + _("missing storage pool device path"));
>>> + goto error;
>>> + }
>>> + }
>>
>> So the wole idea of poolOptions and volOptions is to specify which parts
>> of pool/volume XML are required so that we don't have to base checks
>> like this on ret->type rather than flags.
>> On the other hand, comment just above VIR_STORAGE_POOL_SOURCE_* enum
>> says it declares mandatory components which it clearly doesn't. So after
>> all I think we are safe here.
>
> That actually isn't the case for the initiator iqn flag.
> Is it on purpose or should I patch it in another thread?
I think saving that for a separate patch is okay.
Speaking of threads, I forgot to mention, feel free to send v3 as a
completely new thread. We don't really thread versions under v1.
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
