7:29 a.m.
Luke Yue (3):
test_driver: Implement virDomainGetSecurityLabelList
virsh: Add '--full-seclabels' option for dominfo
tests: Add test for virDomainGetSecurityLabelList
docs/manpages/virsh.rst | 5 +-
src/test/test_driver.c | 42 +++++++++++++++++
tests/virsh-undefine | 8 ++--
tests/virshtest.c | 74 +++++++++++++++---------------
tools/virsh-domain-monitor.c | 89 ++++++++++++++++++++++++------------
5 files changed, 146 insertions(+), 72 deletions(-)
--
2.33.0
7:29 a.m.
New subject: [PATCH 1/3] test_driver: Implement virDomainGetSecurityLabelList
Signed-off-by: Luke Yue <lukedyue(a)gmail.com>
---
src/test/test_driver.c | 42 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 42 insertions(+)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 2f19b7c520..1b5914c890 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -137,6 +137,7 @@ VIR_ONCE_GLOBAL_INIT(testDriver);
#define TEST_MODEL "i686"
#define TEST_EMULATOR "/usr/bin/test-hv"
+#define TEST_SECURITY_LABEL_LIST_LENGTH 2
static const virNodeInfo defaultNodeInfo = {
TEST_MODEL,
@@ -5261,6 +5262,46 @@ testDomainGetSecurityLabel(virDomainPtr dom,
return ret;
}
+static int
+testDomainGetSecurityLabelList(virDomainPtr dom,
+ virSecurityLabelPtr* seclabels)
+{
+ virDomainObj *vm;
+ size_t i;
+ int ret = -1;
+
+ if (!(vm = testDomObjFromDomain(dom)))
+ return -1;
+
+ if (!virDomainObjIsActive(vm)) {
+ /* No seclabels */
+ *seclabels = NULL;
+ ret = 0;
+ } else {
+ int len = TEST_SECURITY_LABEL_LIST_LENGTH;
+
+ (*seclabels) = g_new0(virSecurityLabel, len);
+ memset(*seclabels, 0, sizeof(**seclabels) * len);
+
+ /* Fill the array */
+ for (i = 0; i < len; i++) {
+ if (virStrcpyStatic((*seclabels)[i].label, "libvirt-test") < 0)
{
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("security label exceeds maximum: %zu"),
+ sizeof((*seclabels)[i].label) - 1);
+ g_clear_pointer(seclabels, g_free);
+ goto cleanup;
+ }
+ (*seclabels)[i].enforcing = 1;
+ }
+ ret = len;
+ }
+
+ cleanup:
+ virDomainObjEndAPI(&vm);
+ return ret;
+}
+
static int
testNodeGetSecurityModel(virConnectPtr conn,
virSecurityModelPtr secmodel)
@@ -9615,6 +9656,7 @@ static virHypervisorDriver testHypervisorDriver = {
.domainGetVcpuPinInfo = testDomainGetVcpuPinInfo, /* 1.2.18 */
.domainGetMaxVcpus = testDomainGetMaxVcpus, /* 0.7.3 */
.domainGetSecurityLabel = testDomainGetSecurityLabel, /* 7.5.0 */
+ .domainGetSecurityLabelList = testDomainGetSecurityLabelList, /* 7.8.0 */
.nodeGetSecurityModel = testNodeGetSecurityModel, /* 7.5.0 */
.domainGetXMLDesc = testDomainGetXMLDesc, /* 0.1.4 */
.domainSetMemoryParameters = testDomainSetMemoryParameters, /* 5.6.0 */
--
2.33.0
10:14 a.m.
New subject: [PATCH 1/3] test_driver: Implement virDomainGetSecurityLabelList
On Thu, Sep 02, 2021 at 08:29:34PM +0800, Luke Yue wrote:
Signed-off-by: Luke Yue <lukedyue(a)gmail.com>
---
src/test/test_driver.c | 42 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 42 insertions(+)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 2f19b7c520..1b5914c890 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -137,6 +137,7 @@ VIR_ONCE_GLOBAL_INIT(testDriver);
#define TEST_MODEL "i686"
#define TEST_EMULATOR "/usr/bin/test-hv"
+#define TEST_SECURITY_LABEL_LIST_LENGTH 2
static const virNodeInfo defaultNodeInfo = {
TEST_MODEL,
@@ -5261,6 +5262,46 @@ testDomainGetSecurityLabel(virDomainPtr dom,
return ret;
}
+static int
+testDomainGetSecurityLabelList(virDomainPtr dom,
+ virSecurityLabelPtr* seclabels)
+{
+ virDomainObj *vm;
+ size_t i;
+ int ret = -1;
+
+ if (!(vm = testDomObjFromDomain(dom)))
+ return -1;
+
+ if (!virDomainObjIsActive(vm)) {
+ /* No seclabels */
+ *seclabels = NULL;
+ ret = 0;
+ } else {
+ int len = TEST_SECURITY_LABEL_LIST_LENGTH;
+
+ (*seclabels) = g_new0(virSecurityLabel, len);
+ memset(*seclabels, 0, sizeof(**seclabels) * len);
+
+ /* Fill the array */
+ for (i = 0; i < len; i++) {
+ if (virStrcpyStatic((*seclabels)[i].label, "libvirt-test") < 0)
{
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("security label exceeds maximum: %zu"),
+ sizeof((*seclabels)[i].label) - 1);
+ g_clear_pointer(seclabels, g_free);
+ goto cleanup;
+ }
+ (*seclabels)[i].enforcing = 1;
There could be some variety here, like:
if (i == 0)
(*seclabels)[i].enforcing = 1
But that's just a nitpick.
+ }
+ ret = len;
+ }
+
+ cleanup:
+ virDomainObjEndAPI(&vm);
+ return ret;
+}
+
static int
testNodeGetSecurityModel(virConnectPtr conn,
virSecurityModelPtr secmodel)
@@ -9615,6 +9656,7 @@ static virHypervisorDriver testHypervisorDriver = {
.domainGetVcpuPinInfo = testDomainGetVcpuPinInfo, /* 1.2.18 */
.domainGetMaxVcpus = testDomainGetMaxVcpus, /* 0.7.3 */
.domainGetSecurityLabel = testDomainGetSecurityLabel, /* 7.5.0 */
+ .domainGetSecurityLabelList = testDomainGetSecurityLabelList, /* 7.8.0 */
.nodeGetSecurityModel = testNodeGetSecurityModel, /* 7.5.0 */
.domainGetXMLDesc = testDomainGetXMLDesc, /* 0.1.4 */
.domainSetMemoryParameters = testDomainSetMemoryParameters, /* 5.6.0 */
--
2.33.0
7:29 a.m.
New subject: [PATCH 2/3] virsh: Add '--full-seclabels' option for dominfo
There is no virsh command uses virDomainGetSecurityLabelList API, so add
an option for dominfo to call it and print full list of security labels.
Also realign some outputs as it's now "Security labels:" instead of
"Security label:".
Signed-off-by: Luke Yue <lukedyue(a)gmail.com>
---
docs/manpages/virsh.rst | 5 +-
tests/virsh-undefine | 8 ++--
tests/virshtest.c | 70 ++++++++++++++--------------
tools/virsh-domain-monitor.c | 89 ++++++++++++++++++++++++------------
4 files changed, 101 insertions(+), 71 deletions(-)
diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index 2204bed3bb..183033049d 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -2008,9 +2008,10 @@ dominfo
::
- dominfo domain
+ dominfo domain [--full-seclabels]
-Returns basic information about the domain.
+Returns basic information about the domain. *--full-seclabels* tells virsh
+to print full list of security labels.
domjobabort
diff --git a/tests/virsh-undefine b/tests/virsh-undefine
index dbbb367391..7feefdec79 100755
--- a/tests/virsh-undefine
+++ b/tests/virsh-undefine
@@ -35,11 +35,11 @@ $abs_top_builddir/tools/virsh -c test:///default \
test $? = 0 || fail=1
sed '/^Persistent/n; /:/d' < out1 > out
cat <<\EOF > exp || fail=1
-Persistent: yes
+Persistent: yes
Domain 'test' has been undefined
-Persistent: no
+Persistent: no
EOF
compare exp out || fail=1
@@ -50,11 +50,11 @@ $abs_top_builddir/tools/virsh -c test:///default \
test $? = 0 || fail=1
sed '/^Persistent/n; /:/d' < out1 > out
cat <<\EOF > exp || fail=1
-Persistent: yes
+Persistent: yes
Domain '1' has been undefined
-Persistent: no
+Persistent: no
EOF
compare exp out || fail=1
diff --git a/tests/virshtest.c b/tests/virshtest.c
index 87da1f5889..0d703f3765 100644
--- a/tests/virshtest.c
+++ b/tests/virshtest.c
@@ -25,51 +25,51 @@ main(void)
# define SECURITY_LABEL "libvirt-test (enforcing)"
# define FC4_MESSAGES "tainted: network configuration using opaque shell
scripts"
# define FC5_MESSAGES "tainted: running with undesirable elevated privileges\n\
- tainted: network configuration using opaque shell scripts\n\
- tainted: use of host cdrom passthrough\n\
- tainted: custom device tree blob used\n\
- tainted: use of deprecated configuration settings\n\
- deprecated configuration: CPU model Deprecated-Test"
+ tainted: network configuration using opaque shell scripts\n\
+ tainted: use of host cdrom passthrough\n\
+ tainted: custom device tree blob used\n\
+ tainted: use of deprecated configuration settings\n\
+ deprecated configuration: CPU model Deprecated-Test"
# define GET_BLKIO_PARAMETER "/dev/hda,700"
# define SET_BLKIO_PARAMETER "/dev/hda,1000"
static const char *dominfo_fc4 = "\
-Id: 2\n\
-Name: fc4\n\
-UUID: " DOM_FC4_UUID "\n\
-OS Type: linux\n\
-State: running\n\
-CPU(s): 1\n\
-Max memory: 261072 KiB\n\
-Used memory: 131072 KiB\n\
-Persistent: yes\n\
-Autostart: disable\n\
-Managed save: no\n\
-Security model: testSecurity\n\
-Security DOI: \n\
-Security label: " SECURITY_LABEL "\n\
-Messages: " FC4_MESSAGES "\n\
+Id: 2\n\
+Name: fc4\n\
+UUID: " DOM_FC4_UUID "\n\
+OS Type: linux\n\
+State: running\n\
+CPU(s): 1\n\
+Max memory: 261072 KiB\n\
+Used memory: 131072 KiB\n\
+Persistent: yes\n\
+Autostart: disable\n\
+Managed save: no\n\
+Security model: testSecurity\n\
+Security DOI: \n\
+Security labels: " SECURITY_LABEL "\n\
+Messages: " FC4_MESSAGES "\n\
\n";
static const char *domuuid_fc4 = DOM_FC4_UUID "\n\n";
static const char *domid_fc4 = "2\n\n";
static const char *domname_fc4 = "fc4\n\n";
static const char *domstate_fc4 = "running\n\n";
static const char *dominfo_fc5 = "\
-Id: 3\n\
-Name: fc5\n\
-UUID: " DOM_FC5_UUID "\n\
-OS Type: linux\n\
-State: running\n\
-CPU(s): 4\n\
-Max memory: 2097152 KiB\n\
-Used memory: 2097152 KiB\n\
-Persistent: yes\n\
-Autostart: disable\n\
-Managed save: no\n\
-Security model: testSecurity\n\
-Security DOI: \n\
-Security label: " SECURITY_LABEL "\n\
-Messages: " FC5_MESSAGES "\n\
+Id: 3\n\
+Name: fc5\n\
+UUID: " DOM_FC5_UUID "\n\
+OS Type: linux\n\
+State: running\n\
+CPU(s): 4\n\
+Max memory: 2097152 KiB\n\
+Used memory: 2097152 KiB\n\
+Persistent: yes\n\
+Autostart: disable\n\
+Managed save: no\n\
+Security model: testSecurity\n\
+Security DOI: \n\
+Security labels: " SECURITY_LABEL "\n\
+Messages: " FC5_MESSAGES "\n\
\n";
static const char *get_blkio_parameters = "\
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index f7cf82acdf..2b2746e713 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -1202,6 +1202,10 @@ static const vshCmdInfo info_dominfo[] = {
static const vshCmdOptDef opts_dominfo[] = {
VIRSH_COMMON_OPT_DOMAIN_FULL(0),
+ {.name = "full-seclabels",
+ .type = VSH_OT_BOOL,
+ .help = N_("Show full list of security labels of a domain")
+ },
{.name = NULL}
};
@@ -1221,45 +1225,46 @@ cmdDominfo(vshControl *ctl, const vshCmd *cmd)
int has_managed_save = 0;
virshControl *priv = ctl->privData;
g_auto(GStrv) messages = NULL;
+ bool fullseclabels = vshCommandOptBool(cmd, "full-seclabels");
if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
return false;
id = virDomainGetID(dom);
if (id == ((unsigned int)-1))
- vshPrint(ctl, "%-15s %s\n", _("Id:"), "-");
+ vshPrint(ctl, "%-16s %s\n", _("Id:"), "-");
else
- vshPrint(ctl, "%-15s %d\n", _("Id:"), id);
- vshPrint(ctl, "%-15s %s\n", _("Name:"), virDomainGetName(dom));
+ vshPrint(ctl, "%-16s %d\n", _("Id:"), id);
+ vshPrint(ctl, "%-16s %s\n", _("Name:"), virDomainGetName(dom));
if (virDomainGetUUIDString(dom, &uuid[0]) == 0)
- vshPrint(ctl, "%-15s %s\n", _("UUID:"), uuid);
+ vshPrint(ctl, "%-16s %s\n", _("UUID:"), uuid);
if ((ostype = virDomainGetOSType(dom)))
- vshPrint(ctl, "%-15s %s\n", _("OS Type:"), ostype);
+ vshPrint(ctl, "%-16s %s\n", _("OS Type:"), ostype);
if (virDomainGetInfo(dom, &info) == 0) {
- vshPrint(ctl, "%-15s %s\n", _("State:"),
+ vshPrint(ctl, "%-16s %s\n", _("State:"),
virshDomainStateToString(info.state));
- vshPrint(ctl, "%-15s %d\n", _("CPU(s):"), info.nrVirtCpu);
+ vshPrint(ctl, "%-16s %d\n", _("CPU(s):"), info.nrVirtCpu);
if (info.cpuTime != 0) {
double cpuUsed = info.cpuTime;
cpuUsed /= 1000000000.0;
- vshPrint(ctl, "%-15s %.1lfs\n", _("CPU time:"),
cpuUsed);
+ vshPrint(ctl, "%-16s %.1lfs\n", _("CPU time:"),
cpuUsed);
}
if (info.maxMem != UINT_MAX)
- vshPrint(ctl, "%-15s %lu KiB\n", _("Max memory:"),
+ vshPrint(ctl, "%-16s %lu KiB\n", _("Max memory:"),
info.maxMem);
else
- vshPrint(ctl, "%-15s %s\n", _("Max memory:"),
+ vshPrint(ctl, "%-16s %s\n", _("Max memory:"),
_("no limit"));
- vshPrint(ctl, "%-15s %lu KiB\n", _("Used memory:"),
+ vshPrint(ctl, "%-16s %lu KiB\n", _("Used memory:"),
info.memory);
} else {
@@ -1271,21 +1276,21 @@ cmdDominfo(vshControl *ctl, const vshCmd *cmd)
vshDebug(ctl, VSH_ERR_DEBUG, "Domain persistent flag value: %d\n",
persistent);
if (persistent < 0)
- vshPrint(ctl, "%-15s %s\n", _("Persistent:"),
_("unknown"));
+ vshPrint(ctl, "%-16s %s\n", _("Persistent:"),
_("unknown"));
else
- vshPrint(ctl, "%-15s %s\n", _("Persistent:"), persistent ?
_("yes") : _("no"));
+ vshPrint(ctl, "%-16s %s\n", _("Persistent:"), persistent ?
_("yes") : _("no"));
/* Check and display whether the domain autostarts or not */
if (!virDomainGetAutostart(dom, &autostart)) {
- vshPrint(ctl, "%-15s %s\n", _("Autostart:"),
+ vshPrint(ctl, "%-16s %s\n", _("Autostart:"),
autostart ? _("enable") : _("disable"));
}
has_managed_save = virDomainHasManagedSaveImage(dom, 0);
if (has_managed_save < 0)
- vshPrint(ctl, "%-15s %s\n", _("Managed save:"),
_("unknown"));
+ vshPrint(ctl, "%-16s %s\n", _("Managed save:"),
_("unknown"));
else
- vshPrint(ctl, "%-15s %s\n", _("Managed save:"),
+ vshPrint(ctl, "%-16s %s\n", _("Managed save:"),
has_managed_save ? _("yes") : _("no"));
/* Security model and label information */
@@ -1299,29 +1304,53 @@ cmdDominfo(vshControl *ctl, const vshCmd *cmd)
} else {
/* Only print something if a security model is active */
if (secmodel.model[0] != '\0') {
- vshPrint(ctl, "%-15s %s\n", _("Security model:"),
secmodel.model);
- vshPrint(ctl, "%-15s %s\n", _("Security DOI:"),
secmodel.doi);
-
- /* Security labels are only valid for active domains */
- seclabel = g_new0(virSecurityLabel, 1);
+ vshPrint(ctl, "%-16s %s\n", _("Security model:"),
secmodel.model);
+ vshPrint(ctl, "%-16s %s\n", _("Security DOI:"),
secmodel.doi);
+
+ if (fullseclabels) {
+ int len;
+ size_t i;
+
+ if ((len = virDomainGetSecurityLabelList(dom, &seclabel)) < 0) {
+ g_clear_pointer(&(seclabel), g_free);
+ return false;
+ } else {
+ for (i = 0; i < len; i++)
+ if (seclabel[i].label[0] != '\0')
+ vshPrint(ctl, "%-16s %s (%s)\n",
+ i == 0 ? _("Security labels:") :
"",
+ seclabel[i].label,
+ seclabel[i].enforcing ?
+ "enforcing" :
+ "permissive");
+ }
- if (virDomainGetSecurityLabel(dom, seclabel) == -1) {
- VIR_FREE(seclabel);
- return false;
+ g_clear_pointer(&seclabel, g_free);
} else {
- if (seclabel->label[0] != '\0')
- vshPrint(ctl, "%-15s %s (%s)\n", _("Security
label:"),
- seclabel->label, seclabel->enforcing ?
"enforcing" : "permissive");
- }
+ /* Security labels are only valid for active domains */
+ seclabel = g_new0(virSecurityLabel, 1);
+
+ if (virDomainGetSecurityLabel(dom, seclabel) == -1) {
+ g_clear_pointer(&seclabel, g_free);
+ return false;
+ } else {
+ if (seclabel->label[0] != '\0')
+ vshPrint(ctl, "%-16s %s (%s)\n", _("Security
labels:"),
+ seclabel->label,
+ seclabel->enforcing ?
+ "enforcing" :
+ "permissive");
+ }
- VIR_FREE(seclabel);
+ g_clear_pointer(&seclabel, g_free);
+ }
}
}
if (virDomainGetMessages(dom, &messages, 0) > 0) {
size_t i;
for (i = 0; messages[i] != NULL; i++) {
- vshPrint(ctl, "%-15s %s\n",
+ vshPrint(ctl, "%-16s %s\n",
i == 0 ? _("Messages:") : "", messages[i]);
}
}
--
2.33.0
10:52 a.m.
New subject: [PATCH 2/3] virsh: Add '--full-seclabels' option for dominfo
On Thu, Sep 02, 2021 at 08:29:35PM +0800, Luke Yue wrote:
There is no virsh command uses virDomainGetSecurityLabelList API, so
add
an option for dominfo to call it and print full list of security labels.
Also realign some outputs as it's now "Security labels:" instead of
"Security label:".
This should go into separate patch as that makes it nicer to review and
the separation of changes makes it more clear.
Signed-off-by: Luke Yue <lukedyue(a)gmail.com>
---
docs/manpages/virsh.rst | 5 +-
tests/virsh-undefine | 8 ++--
tests/virshtest.c | 70 ++++++++++++++--------------
tools/virsh-domain-monitor.c | 89 ++++++++++++++++++++++++------------
4 files changed, 101 insertions(+), 71 deletions(-)
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index f7cf82acdf..2b2746e713 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -1299,29 +1304,53 @@ cmdDominfo(vshControl *ctl, const vshCmd *cmd)
} else {
/* Only print something if a security model is active */
if (secmodel.model[0] != '\0') {
- vshPrint(ctl, "%-15s %s\n", _("Security model:"),
secmodel.model);
- vshPrint(ctl, "%-15s %s\n", _("Security DOI:"),
secmodel.doi);
-
- /* Security labels are only valid for active domains */
- seclabel = g_new0(virSecurityLabel, 1);
+ vshPrint(ctl, "%-16s %s\n", _("Security model:"),
secmodel.model);
+ vshPrint(ctl, "%-16s %s\n", _("Security DOI:"),
secmodel.doi);
+
+ if (fullseclabels) {
+ int len;
+ size_t i;
+
+ if ((len = virDomainGetSecurityLabelList(dom, &seclabel)) < 0) {
+ g_clear_pointer(&(seclabel), g_free);
+ return false;
+ } else {
No need for else branch when the first branch returns.
+ for (i = 0; i < len; i++)
+ if (seclabel[i].label[0] != '\0')
+ vshPrint(ctl, "%-16s %s (%s)\n",
+ i == 0 ? _("Security labels:") :
"",
+ seclabel[i].label,
+ seclabel[i].enforcing ?
+ "enforcing" :
+ "permissive");
+ }
- if (virDomainGetSecurityLabel(dom, seclabel) == -1) {
- VIR_FREE(seclabel);
- return false;
+ g_clear_pointer(&seclabel, g_free);
} else {
- if (seclabel->label[0] != '\0')
- vshPrint(ctl, "%-15s %s (%s)\n", _("Security
label:"),
- seclabel->label, seclabel->enforcing ?
"enforcing" : "permissive");
- }
+ /* Security labels are only valid for active domains */
+ seclabel = g_new0(virSecurityLabel, 1);
+
+ if (virDomainGetSecurityLabel(dom, seclabel) == -1) {
You properly used `< 0` before, this should do the same then.
+ g_clear_pointer(&seclabel, g_free);
+ return false;
+ } else {
Again, no need for an else branch when if branch returns.
Otherwise it looks good.
5:26 a.m.
New subject: [PATCH 2/3] virsh: Add '--full-seclabels' option for dominfo
On Mon, 2021-11-29 at 17:52 +0100, Martin Kletzander wrote:
On Thu, Sep 02, 2021 at 08:29:35PM +0800, Luke Yue wrote:
> There is no virsh command uses virDomainGetSecurityLabelList API,
> so add
> an option for dominfo to call it and print full list of security
> labels.
>
> Also realign some outputs as it's now "Security labels:" instead of
> "Security label:".
>
This should go into separate patch as that makes it nicer to review
and
the separation of changes makes it more clear.
Thanks for the review! I will split this in next version.
> Signed-off-by: Luke Yue <lukedyue(a)gmail.com>
> ---
> docs/manpages/virsh.rst | 5 +-
> tests/virsh-undefine | 8 ++--
> tests/virshtest.c | 70 ++++++++++++++--------------
> tools/virsh-domain-monitor.c | 89 ++++++++++++++++++++++++---------
> ---
> 4 files changed, 101 insertions(+), 71 deletions(-)
>
> diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-
> monitor.c
> index f7cf82acdf..2b2746e713 100644
> --- a/tools/virsh-domain-monitor.c
> +++ b/tools/virsh-domain-monitor.c
> @@ -1299,29 +1304,53 @@ cmdDominfo(vshControl *ctl, const vshCmd
> *cmd)
> } else {
> /* Only print something if a security model is active */
> if (secmodel.model[0] != '\0') {
> - vshPrint(ctl, "%-15s %s\n", _("Security model:"),
> secmodel.model);
> - vshPrint(ctl, "%-15s %s\n", _("Security DOI:"),
> secmodel.doi);
> -
> - /* Security labels are only valid for active domains
> */
> - seclabel = g_new0(virSecurityLabel, 1);
> + vshPrint(ctl, "%-16s %s\n", _("Security model:"),
> secmodel.model);
> + vshPrint(ctl, "%-16s %s\n", _("Security DOI:"),
> secmodel.doi);
> +
> + if (fullseclabels) {
> + int len;
> + size_t i;
> +
> + if ((len = virDomainGetSecurityLabelList(dom,
> &seclabel)) < 0) {
> + g_clear_pointer(&(seclabel), g_free);
> + return false;
> + } else {
No need for else branch when the first branch returns.
> + for (i = 0; i < len; i++)
> + if (seclabel[i].label[0] != '\0')
> + vshPrint(ctl, "%-16s %s (%s)\n",
> + i == 0 ? _("Security
> labels:") : "",
> + seclabel[i].label,
> + seclabel[i].enforcing ?
> + "enforcing" :
> + "permissive");
> + }
>
> - if (virDomainGetSecurityLabel(dom, seclabel) == -1) {
> - VIR_FREE(seclabel);
> - return false;
> + g_clear_pointer(&seclabel, g_free);
> } else {
> - if (seclabel->label[0] != '\0')
> - vshPrint(ctl, "%-15s %s (%s)\n", _("Security
> label:"),
> - seclabel->label, seclabel->enforcing
> ? "enforcing" : "permissive");
> - }
> + /* Security labels are only valid for active
> domains */
> + seclabel = g_new0(virSecurityLabel, 1);
> +
> + if (virDomainGetSecurityLabel(dom, seclabel) == -
> 1) {
You properly used `< 0` before, this should do the same then.
> + g_clear_pointer(&seclabel, g_free);
> + return false;
> + } else {
Again, no need for an else branch when if branch returns.
OK, I will fix these and take the suggestion for 1/3
Otherwise it looks good.
Thanks,
Luke
7:29 a.m.
New subject: [PATCH 3/3] tests: Add test for virDomainGetSecurityLabelList
Signed-off-by: Luke Yue <lukedyue(a)gmail.com>
---
tests/virshtest.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tests/virshtest.c b/tests/virshtest.c
index 0d703f3765..4fb4799456 100644
--- a/tests/virshtest.c
+++ b/tests/virshtest.c
@@ -69,6 +69,7 @@ Managed save: no\n\
Security model: testSecurity\n\
Security DOI: \n\
Security labels: " SECURITY_LABEL "\n\
+ " SECURITY_LABEL "\n\
Messages: " FC5_MESSAGES "\n\
\n";
@@ -237,7 +238,8 @@ static int testCompareDominfoByName(const void *data G_GNUC_UNUSED)
static int testCompareTaintedDominfoByName(const void *data G_GNUC_UNUSED)
{
- const char *const argv[] = { VIRSH_CUSTOM, "dominfo", "fc5", NULL
};
+ const char *const argv[] = { VIRSH_CUSTOM, "dominfo", "fc5",
+ "--full-seclabels", NULL };
const char *exp = dominfo_fc5;
return testCompareOutputLit(exp, "\nCPU time:", argv);
}
--
2.33.0
1120
days inactive
1209
days old
6 comments
2 participants
participants (2)
-
Luke Yue -
Martin Kletzander