Re: [libvirt] [PATCH v2 1/2] security_manager: Don't manipulate domain XML in virDomainDefGetSecurityLabelDef

On 28.03.2013 10:15, Daniel P. Berrange wrote: > On Thu, Mar 21, 2013 at 04:35:10PM +0100, Michal Privoznik wrote: >> The virDomainDefGetSecurityLabelDef was modifying the domain XML. >> It tried to find a seclabel corresponding to given sec driver. If the >> label wasn't found, the function created one which is wrong. In fact >> it's security manager which should modify this part of domain XML. >> --- >> src/conf/domain_conf.c | 56 +++++++++++------------------------------ >> src/conf/domain_conf.h | 7 ++++-- >> src/libvirt_private.syms | 1 - >> src/security/security_manager.c | 40 ++++++++++++++++++++--------- >> src/security/security_selinux.c | 8 ++++-- >> 5 files changed, 53 insertions(+), 59 deletions(-) > >> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h >> index a750a1f..2540bca 100644 >> --- a/src/conf/domain_conf.h >> +++ b/src/conf/domain_conf.h >> @@ -2297,10 +2297,13 @@ virSecurityDeviceLabelDefPtr >> virDomainChrDefGetSecurityLabelDef(virDomainChrDefPtr def, const char *model); >> >> virSecurityLabelDefPtr >> -virDomainDefAddSecurityLabelDef(virDomainDefPtr def, const char *model); >> +virDomainDefGenSecurityLabelDef(const char *model); >> >> virSecurityDeviceLabelDefPtr >> -virDomainDiskDefAddSecurityLabelDef(virDomainDiskDefPtr def, const char *model); >> +virDomainDiskDefGenSecurityLabelDef(const char *model); >> + >> +void virSecurityLabelDefFree(virSecurityLabelDefPtr def); >> +void virSecurityDeviceLabelDefFree(virSecurityDeviceLabelDefPtr def); >> >> typedef const char* (*virEventActionToStringFunc)(int type); >> typedef int (*virEventActionFromStringFunc)(const char *type); >> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms >> index 21bc615..e8085a9 100644 >> --- a/src/libvirt_private.syms >> +++ b/src/libvirt_private.syms >> @@ -108,7 +108,6 @@ virDomainControllerTypeToString; >> virDomainCpuPlacementModeTypeFromString; >> virDomainCpuPlacementModeTypeToString; >> virDomainDefAddImplicitControllers; >> -virDomainDefAddSecurityLabelDef; >> virDomainDefCheckABIStability; >> virDomainDefClearCCWAddresses; >> virDomainDefClearDeviceAliases; > > 2 APIs renamed + 2 APIs added in the header, but only one > delete here. I'd expect 6 changes in this file - 2 deletes > and 4 additions. > > > ACK if you fix that. > > Daniel > Woops, I've already pushed prior seeing your reply. However, There can be only 1 deletion, the virDomainDiskDefGenSecurityLabelDef() wasn't exported in libvirt_private.syms. I am pushing this follow up patch:

commit a919e6f7769b27168b9217fd2fd5143259f63173 Author: Michal Privoznik <mprivozn(a)redhat.com&gt; AuthorDate: Thu Mar 28 10:39:25 2013 +0100 Commit: Michal Privoznik <mprivozn(a)redhat.com&gt; CommitDate: Thu Mar 28 10:39:25 2013 +0100 libvirt_private.syms: Correctly export seclabel APIs One of my previous patches manipulated virSecurityLabel* APIs, some were added to header files, and some were renamed. However, these changes were not reflected in libvirt_private.syms.