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@xxxxxxxxxx>
>> ---
>> 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
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
