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@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