[libvirt] [PATCH 3/8] util: Convert virStoragePoolSourceAdapter to virStorageAdapter

[John Ferlan]

On 02/27/2017 10:36 AM, Michal Privoznik wrote:
> On 20.02.2017 14:18, John Ferlan wrote:
>> Introduce src/util/virstoragedevice.{c,h}. Rather than have virstoragefile
>> be a repository for more things - create a new module to share the adapter
>> definitions between the storage pool and eventually the domain. The devices
>> will need device address functions (no need to pollute more code for that).
>>
>> Move the virStoragePoolSourceAdapter from storage_conf into the new
>> module and parcel out the the structure a bit more into 'fchost' and
>> 'scsi_host' specific pieces creating virStoragePoolSourceAdapterSCSIHost
>> and virStoragePoolSourceAdapterFCHost structures for easier access.
>>
>> Modify code in the various places which formerly used the pool structure
>> to use the new model. This includes some changes that will use the Ptr
>> rather than just the struct (try to shorten the number of times one
>> has to type adapter.data.fchost or adapter.data.scsi_host as well as
>> [pool->]def->source.adapter.
>>
>> Signed-off-by: John Ferlan <jferlan at redhat.com>
>> ---
>>  po/POTFILES.in                     |   1 +
>>  src/Makefile.am                    |   1 +
>>  src/conf/storage_conf.c            | 338 ++++++++-----------------------------
>>  src/conf/storage_conf.h            |  35 +---
>>  src/libvirt_private.syms           |  16 +-
>>  src/libxl/libxl_conf.c             |   1 +
>>  src/phyp/phyp_driver.c             |   3 +-
>>  src/storage/storage_backend_scsi.c | 131 +++++++-------
>>  src/test/test_driver.c             |   4 +-
>>  src/util/virscsihost.c             |  28 +--
>>  src/util/virscsihost.h             |   8 +-
>>  src/util/virstoragedevice.c        | 292 ++++++++++++++++++++++++++++++++
>>  src/util/virstoragedevice.h        |  89 ++++++++++
>>  13 files changed, 548 insertions(+), 399 deletions(-)
>>  create mode 100644 src/util/virstoragedevice.c
>>  create mode 100644 src/util/virstoragedevice.h
>>
> 
>> diff --git a/src/util/virstoragedevice.c b/src/util/virstoragedevice.c
>> new file mode 100644
>> index 0000000..0d9db34
>> --- /dev/null
>> +++ b/src/util/virstoragedevice.c
>> @@ -0,0 +1,292 @@
>> +/*
>> + * virstoragedevice.c: utility functions to share storage device mgmt
>> + *                     between storage pools and domains
>> + *
>> + * This library is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Lesser General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2.1 of the License, or (at your option) any later version.
>> + *
>> + * This library is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * Lesser General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Lesser General Public
>> + * License along with this library.  If not, see
>> + * <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#include <config.h>
>> +
>> +#include "device_conf.h"
> 
> After Peter's b4c73106333c0ede this will not fly. Initially I was going
> to suggest:
> 
>   #include "conf/device_conf.h"
> 
> but that will not help either. IIUC you need device_conf just for
> virPCIDeviceAddressParseXML(). Well, I guess we need to move that
> ParseXML() into src/util/virpci.c so that we can re-use it in src/util/.
> Or even better - why virstoragedevie needs to live in src/util if it is
> really just a set of XML parse/format functions? I'd expect it to live
> under src/conf.
> 

Yes, virPCIDeviceAddressParseXML and also virPCIDeviceAddressFormat

The "thought process" is/was similarity with virstoragefile and
virstorageencryption... Then yes, that pesky PCI address stuff got in
the way. Much easier to include *conf than move it.

If storagedevice moves to conf, then theory would say that parts of the
others should follow (auth and encryption Parse/Format) - essentially
things that were shared between storage and domain definitions

Let's see what I can come up with for this. Of course my long term
vision had a second reason - to be able to share the <adapter> parsing
with the domain code which was going to get a vHBA, but that may not
become a reality now based on Daniel's feelings over the event mechanism
between qemu and nodedev.

John

> Kudos for rolling up your sleeves and putting all these functions in one
> place.
> 
> Michal
>