Re: [libvirt] [PATCH 4/6] conf: Move virDomainPCIAddressAsString() to device_conf
On Fri, 2018-08-31 at 15:29 +0100, Daniel P. Berrangé wrote:
> On Fri, Aug 31, 2018 at 04:25:25PM +0200, Andrea Bolognani wrote:
> > On Fri, 2018-08-31 at 15:05 +0100, Daniel P. Berrangé wrote:
> > > This should really be in src/util/virpci.{c,h}, since that's where the
> > > virPCIDeviceAddressPtr struct is declared. There's nothing XML related
> > > about this string conversion, so doesn't belong in src/conf at all.
> >
> > See the commit message :)
> >
> > I can move this to util/virpci instead of conf/device_conf for
> > the time being if you prefer, but at some point we're going to
> > have to pick a place for *all* functions related to PCI addresses
> > and conf/device_conf is the most sensible option IMHO, seeing as
> > all other address types and related functions are defined there.
>
> The device_conf file is dealing with domain device addresses. The
> virPCIDeviceAddressPtr struct is independant of domain device
> addresses. It is used across domain, node device, network and
> storage drivers.
Yeah, it's a mess. I'll move it to util/virpci instead, but that
won't make the mess go away :)
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/6] conf: Move virDomainPCIAddressAsString() to device_conf
On Fri, Aug 31, 2018 at 04:25:25PM +0200, Andrea Bolognani wrote:
> On Fri, 2018-08-31 at 15:05 +0100, Daniel P. Berrangé wrote:
> > On Fri, Aug 31, 2018 at 04:00:45PM +0200, Andrea Bolognani wrote:
> > > Unfortunately, even after this change functions
> > > handling virPCIDeviceAddress are split pretty much
> > > evenly between conf/device_conf and utils/virpci:
> > > ideally everything would be moved to the former,
> > > including the struct declaration itself, and all the
> > > names would be changed to be consistent with the rest
> > > of the virDomainDevice*Address, but that's a cleanup
> > > for another day.
> [...]
> > > +char *
> > > +virDomainPCIAddressAsString(virPCIDeviceAddressPtr addr)
> > > +{
> > > +char *str;
> > > +
> > > +ignore_value(virAsprintf(, "%.4x:%.2x:%.2x.%.1x",
> > > + addr->domain,
> > > + addr->bus,
> > > + addr->slot,
> > > + addr->function));
> > > +return str;
> > > +}
> >
> > This should really be in src/util/virpci.{c,h}, since that's where the
> > virPCIDeviceAddressPtr struct is declared. There's nothing XML related
> > about this string conversion, so doesn't belong in src/conf at all.
>
> See the commit message :)
>
> I can move this to util/virpci instead of conf/device_conf for
> the time being if you prefer, but at some point we're going to
> have to pick a place for *all* functions related to PCI addresses
> and conf/device_conf is the most sensible option IMHO, seeing as
> all other address types and related functions are defined there.
The device_conf file is dealing with domain device addresses. The
virPCIDeviceAddressPtr struct is independant of domain device
addresses. It is used across domain, node device, network and
storage drivers.
For that matter device_conf should probably be domain_device_conf
as a name.
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/6] conf: Move virDomainPCIAddressAsString() to device_conf
On Fri, 2018-08-31 at 15:05 +0100, Daniel P. Berrangé wrote:
> On Fri, Aug 31, 2018 at 04:00:45PM +0200, Andrea Bolognani wrote:
> > Unfortunately, even after this change functions
> > handling virPCIDeviceAddress are split pretty much
> > evenly between conf/device_conf and utils/virpci:
> > ideally everything would be moved to the former,
> > including the struct declaration itself, and all the
> > names would be changed to be consistent with the rest
> > of the virDomainDevice*Address, but that's a cleanup
> > for another day.
[...]
> > +char *
> > +virDomainPCIAddressAsString(virPCIDeviceAddressPtr addr)
> > +{
> > +char *str;
> > +
> > +ignore_value(virAsprintf(, "%.4x:%.2x:%.2x.%.1x",
> > + addr->domain,
> > + addr->bus,
> > + addr->slot,
> > + addr->function));
> > +return str;
> > +}
>
> This should really be in src/util/virpci.{c,h}, since that's where the
> virPCIDeviceAddressPtr struct is declared. There's nothing XML related
> about this string conversion, so doesn't belong in src/conf at all.
See the commit message :)
I can move this to util/virpci instead of conf/device_conf for
the time being if you prefer, but at some point we're going to
have to pick a place for *all* functions related to PCI addresses
and conf/device_conf is the most sensible option IMHO, seeing as
all other address types and related functions are defined there.
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/6] conf: Move virDomainPCIAddressAsString() to device_conf
On Fri, Aug 31, 2018 at 04:00:45PM +0200, Andrea Bolognani wrote:
> It's a better fit than domain_conf.
>
> Unfortunately, even after this change functions
> handling virPCIDeviceAddress are split pretty much
> evenly between conf/device_conf and utils/virpci:
> ideally everything would be moved to the former,
> including the struct declaration itself, and all the
> names would be changed to be consistent with the rest
> of the virDomainDevice*Address, but that's a cleanup
> for another day.
>
> Signed-off-by: Andrea Bolognani
> ---
> src/conf/device_conf.c | 13 +
> src/conf/device_conf.h | 3 +++
> src/conf/domain_addr.c | 14 --
> src/conf/domain_addr.h | 3 ---
> src/libvirt_private.syms | 2 +-
> 5 files changed, 17 insertions(+), 18 deletions(-)
>
> diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c
> index 1565d43fe6..afa06c3cda 100644
> --- a/src/conf/device_conf.c
> +++ b/src/conf/device_conf.c
> @@ -309,6 +309,19 @@ virPCIDeviceAddressFormat(virBufferPtr buf,
> return 0;
> }
>
> +char *
> +virDomainPCIAddressAsString(virPCIDeviceAddressPtr addr)
> +{
> +char *str;
> +
> +ignore_value(virAsprintf(, "%.4x:%.2x:%.2x.%.1x",
> + addr->domain,
> + addr->bus,
> + addr->slot,
> + addr->function));
> +return str;
> +}
This should really be in src/util/virpci.{c,h}, since that's where the
virPCIDeviceAddressPtr struct is declared. There's nothing XML related
about this string conversion, so doesn't belong in src/conf at all.
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/6] conf: Move virDomainPCIAddressAsString() to device_conf
It's a better fit than domain_conf.
Unfortunately, even after this change functions
handling virPCIDeviceAddress are split pretty much
evenly between conf/device_conf and utils/virpci:
ideally everything would be moved to the former,
including the struct declaration itself, and all the
names would be changed to be consistent with the rest
of the virDomainDevice*Address, but that's a cleanup
for another day.
Signed-off-by: Andrea Bolognani
---
src/conf/device_conf.c | 13 +
src/conf/device_conf.h | 3 +++
src/conf/domain_addr.c | 14 --
src/conf/domain_addr.h | 3 ---
src/libvirt_private.syms | 2 +-
5 files changed, 17 insertions(+), 18 deletions(-)
diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c
index 1565d43fe6..afa06c3cda 100644
--- a/src/conf/device_conf.c
+++ b/src/conf/device_conf.c
@@ -309,6 +309,19 @@ virPCIDeviceAddressFormat(virBufferPtr buf,
return 0;
}
+char *
+virDomainPCIAddressAsString(virPCIDeviceAddressPtr addr)
+{
+char *str;
+
+ignore_value(virAsprintf(, "%.4x:%.2x:%.2x.%.1x",
+ addr->domain,
+ addr->bus,
+ addr->slot,
+ addr->function));
+return str;
+}
+
bool
virPCIDeviceAddressEqual(virPCIDeviceAddress *addr1,
virPCIDeviceAddress *addr2)
diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h
index b09d6bcecb..846d12543d 100644
--- a/src/conf/device_conf.h
+++ b/src/conf/device_conf.h
@@ -205,6 +205,9 @@ int virPCIDeviceAddressFormat(virBufferPtr buf,
virPCIDeviceAddress addr,
bool includeTypeInAddr);
+char *virDomainPCIAddressAsString(virPCIDeviceAddressPtr addr)
+ ATTRIBUTE_NONNULL(1);
+
bool virPCIDeviceAddressEqual(virPCIDeviceAddress *addr1,
virPCIDeviceAddress *addr2);
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index b62fd53c66..d2e1142462 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -573,20 +573,6 @@ virDomainPCIAddressSetGrow(virDomainPCIAddressSetPtr addrs,
}
-char *
-virDomainPCIAddressAsString(virPCIDeviceAddressPtr addr)
-{
-char *str;
-
-ignore_value(virAsprintf(, "%.4x:%.2x:%.2x.%.1x",
- addr->domain,
- addr->bus,
- addr->slot,
- addr->function));
-return str;
-}
-
-
/*
* Check if the PCI slot is used by another device.
*/
diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
index 5ad9d8ef3d..2a9af9c00b 100644
--- a/src/conf/domain_addr.h
+++ b/src/conf/domain_addr.h
@@ -124,9 +124,6 @@ struct _virDomainPCIAddressSet {
typedef struct _virDomainPCIAddressSet virDomainPCIAddressSet;
typedef virDomainPCIAddressSet *virDomainPCIAddressSetPtr;
-char *virDomainPCIAddressAsString(virPCIDeviceAddressPtr addr)
- ATTRIBUTE_NONNULL(1);
-
virDomainPCIAddressSetPtr virDomainPCIAddressSetAlloc(unsigned int nbuses);
void virDomainPCIAddressSetFree(virDomainPCIAddressSetPtr addrs);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 78307885dc..77f55b3c40 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -100,6 +100,7 @@ virDomainDeviceAddressTypeToString;
virDomainDeviceCCWAddressIsValid;
virDomainDeviceInfoAddressIsEqual;
virDomainDeviceInfoCopy;
+virDomainPCIAddressAsString;
virInterfaceLinkFormat;
virInterfaceLinkParseXML;
virPCIDeviceAddressEqual;
@@ -113,7 +114,6 @@ virPCIDeviceAddressParseXML;
virDomainCCWAddressAssign;
virDomainCCWAddressSetCreateFromDomain;
virDomainCCWAddressSetFree;
-virDomainPCIAddressAsString;
virDomainPCIAddressBusIsFullyReserved;
virDomainPCIAddressBusSetModel;
virDomainPCIAddressEnsureAddr;
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
