<div dir="ltr">Thanks for the comments Michal..<br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Jan 12, 2018 at 3:41 PM, Michal Privoznik <span dir="ltr"><<a href="mailto:mprivozn@redhat.com" target="_blank">mprivozn@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="gmail-m_-1326462571647475558HOEnZb"><div class="gmail-m_-1326462571647475558h5">On 01/11/2018 11:09 AM, Shivaprasad G Bhat wrote:<br>
> The virt-aa-helper fails to parse the xmls with the memory/cpu<br>
> hotplug features or user assigned aliases. Set the features in<br>
> xmlopt->config for the parsing to succeed.<br>
><br>
> Signed-off-by: Shivaprasad G Bhat <<a href="mailto:sbhat@linux.vnet.ibm.com" target="_blank">sbhat@linux.vnet.ibm.com</a>><br>
> ---<br>
> src/conf/domain_conf.c | 21 ---------------------<br>
> src/conf/domain_conf.h | 21 +++++++++++++++++++++<br>
> src/security/virt-aa-helper.c | 7 +++++++<br>
> 3 files changed, 28 insertions(+), 21 deletions(-)<br>
><br>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c<br>
> index a1c2506..20ce83e 100644<br>
> --- a/src/conf/domain_conf.c<br>
> +++ b/src/conf/domain_conf.c<br>
> @@ -64,27 +64,6 @@<br>
><br>
> VIR_LOG_INIT("conf.domain_conf<wbr>");<br>
><br>
> -/* This structure holds various callbacks and data needed<br>
> - * while parsing and creating domain XMLs */<br>
> -struct _virDomainXMLOption {<br>
> - virObject parent;<br>
> -<br>
> - /* XML parser callbacks and defaults */<br>
> - virDomainDefParserConfig config;<br>
> -<br>
> - /* domain private data management callbacks */<br>
> - virDomainXMLPrivateDataCallbac<wbr>ks privateData;<br>
> -<br>
> - /* XML namespace callbacks */<br>
> - virDomainXMLNamespace ns;<br>
> -<br>
> - /* ABI stability callbacks */<br>
> - virDomainABIStability abi;<br>
> -<br>
> - /* Private data for save image stored in snapshot XML */<br>
> - virSaveCookieCallbacks saveCookie;<br>
> -};<br>
> -<br>
> #define VIR_DOMAIN_DEF_FORMAT_COMMON_F<wbr>LAGS \<br>
> (VIR_DOMAIN_DEF_FORMAT_SECURE | \<br>
> VIR_DOMAIN_DEF_FORMAT_<wbr>INACTIVE | \<br>
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h<br>
> index 6f7f96b..aacb88a 100644<br>
> --- a/src/conf/domain_conf.h<br>
> +++ b/src/conf/domain_conf.h<br>
> @@ -2662,6 +2662,27 @@ struct _virDomainABIStability {<br>
> virDomainABIStabilityDomain domain;<br>
> };<br>
><br>
> +/* This structure holds various callbacks and data needed<br>
> + * while parsing and creating domain XMLs */<br>
> +struct _virDomainXMLOption {<br>
> + virObject parent;<br>
> +<br>
> + /* XML parser callbacks and defaults */<br>
> + virDomainDefParserConfig config;<br>
> +<br>
> + /* domain private data management callbacks */<br>
> + virDomainXMLPrivateDataCallbac<wbr>ks privateData;<br>
> +<br>
> + /* XML namespace callbacks */<br>
> + virDomainXMLNamespace ns;<br>
> +<br>
> + /* ABI stability callbacks */<br>
> + virDomainABIStability abi;<br>
> +<br>
> + /* Private data for save image stored in snapshot XML */<br>
> + virSaveCookieCallbacks saveCookie;<br>
> +};<br>
<br>
<br>
</div></div>No. We want to keep this struct private and use just the accessors.</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<span><br>
> +<br>
> virDomainXMLOptionPtr virDomainXMLOptionNew(virDomai<wbr>nDefParserConfigPtr config,<br>
> virDomainXMLPrivateDataCallbac<wbr>ksPtr priv,<br>
> virDomainXMLNamespacePtr xmlns,<br>
> diff --git a/src/security/virt-aa-helper.<wbr>c b/src/security/virt-aa-helper.<wbr>c<br>
> index f7ccae0..8b0ca46 100644<br>
> --- a/src/security/virt-aa-helper.<wbr>c<br>
> +++ b/src/security/virt-aa-helper.<wbr>c<br>
> @@ -699,6 +699,13 @@ get_definition(vahControl * ctl, const char *xmlStr)<br>
> goto exit;<br>
> }<br>
><br>
> + if (virtType == VIR_DOMAIN_VIRT_QEMU || virtType == VIR_DOMAIN_VIRT_KVM) {<br>
<br>
</span>Firstly, I don't really understand why this needs to be just limited to<br>
qemu/kvm virt types.<br></blockquote><div><br></div><div>I figured, its only QEMU/KVM and LXC drivers which allow/support the security driver to be set</div><div>to "apparmor". Though I see lxc.conf not documenting anything about apparmor, the virt-aa-helper.c</div><div>does handle LXC driver. I wanted virt-aa-helper to fail for LXC for these unsupported features.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<span><br>
> + ctl->xmlopt->config.features = VIR_DOMAIN_DEF_FEATURE_MEMORY_<wbr>HOTPLUG |<br>
> + VIR_DOMAIN_DEF_FEATURE_OFFLIN<wbr>E_VCPUPIN |<br>
> + VIR_DOMAIN_DEF_FEATURE_INDIVI<wbr>DUAL_VCPUS |<br>
> + VIR_DOMAIN_DEF_FEATURE_USER_A<wbr>LIAS;<br>
<br>
</span>Secondly, you can set these features when creating the xmlopt object<br>
just a few lines above (not to be seen in the context though). I mean,<br>
what we can have is:</blockquote><div><br></div><div>Yes. This works for the way virt-aa-helper is intended to be used .That is, I see the binary</div><div>virt-aa-helper is kind of "private", and used only by libvirt after "define" and before "start".</div><div>So, the virt-aa-helper is not called by LXC driver in first place if there is an unsupported</div><div>feature as the "define" itself fails. So, its fine to have like you suggested.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> <br></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
diff --git i/src/security/virt-aa-helper.<wbr>c w/src/security/virt-aa-helper.<wbr>c<br>
index f7ccae0b0..e45ccc8b4 100644<br>
--- i/src/security/virt-aa-helper.<wbr>c<br>
+++ w/src/security/virt-aa-helper.<wbr>c<br>
@@ -654,6 +654,12 @@ caps_mockup(vahControl * ctl, const char *xmlStr)<br>
return rc;<br>
}<br>
<br>
+virDomainDefParserConfig virAAHelperDomainDefParserConf<wbr>ig = {<br>
+ .features = VIR_DOMAIN_DEF_FEATURE_MEMORY_<wbr>HOTPLUG |<br>
+ VIR_DOMAIN_DEF_FEATURE_OFFLINE<wbr>_VCPUPIN |<br>
+ VIR_DOMAIN_DEF_FEATURE_INDIVID<wbr>UAL_VCPUS |<br>
+ VIR_DOMAIN_DEF_FEATURE_USER_AL<wbr>IAS,<br></blockquote><div><br></div><div>I realize we need not set the VIR_DOMAIN_DEF_FEATURE_USER_AL<wbr>IAS here.</div><div><br></div><div><div>Sending V2 as you suggested with this correction. </div></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+};<br>
<br>
static int<br>
<span> get_definition(vahControl * ctl, const char *xmlStr)<br>
</span>@@ -673,7 +679,8 @@ get_definition(vahControl * ctl, const char *xmlStr)<br>
goto exit;<br>
}<br>
<br>
- if (!(ctl->xmlopt = virDomainXMLOptionNew(NULL, NULL, NULL, NULL,<br>
NULL))) {<br>
+ if (!(ctl->xmlopt =<br>
virDomainXMLOptionNew(&virAAHe<wbr>lperDomainDefParserConfig,<br>
+ NULL, NULL, NULL, NULL))) {<br>
vah_error(ctl, 0, _("Failed to create XML config object"));<br>
goto exit;<br>
}<br>
<span class="gmail-m_-1326462571647475558HOEnZb"><font color="#888888"><br>
Michal<br>
</font></span><div class="gmail-m_-1326462571647475558HOEnZb"><div class="gmail-m_-1326462571647475558h5"><br>
--<br>
libvir-list mailing list<br>
<a href="mailto:libvir-list@redhat.com" target="_blank">libvir-list@redhat.com</a><br>
<a href="https://www.redhat.com/mailman/listinfo/libvir-list" rel="noreferrer" target="_blank">https://www.redhat.com/mailman<wbr>/listinfo/libvir-list</a><br>
</div></div></blockquote></div><br></div></div>