[libvirt] [PATCH 0/5] Allow to query a guest's hostname

The following patches allow to query a guest's hostname. The last patch might be debatable since it adds yet another option to virsh list but I hope the rest of the serious looks sane. Cheers, -- Guido Guido Günther (5): Add virGetHostname virsh: Add domhostname openvz: Add openvzVEGetStringParam openvz: Implement domainGetHostname virsh: allow to print hostname in domain listings include/libvirt/libvirt.h.in | 2 ++ src/driver.h | 6 ++++ src/libvirt.c | 42 ++++++++++++++++++++++++++++ src/libvirt_openvz.syms | 2 +- src/libvirt_public.syms | 5 ++++ src/openvz/openvz_driver.c | 28 +++++++++++++++++++ src/openvz/openvz_util.c | 31 +++++++++++++++++++++ src/openvz/openvz_util.h | 1 + tools/virsh.c | 62 +++++++++++++++++++++++++++++++++++++++++- tools/virsh.pod | 4 +++ 10 files changed, 181 insertions(+), 2 deletions(-) -- 1.7.10.4

to query a guests's hostname. Containers like LXC and OpenVZ allow to set a hostname different from the hosts name and QEMU's guest agent could provide similar functionality. --- include/libvirt/libvirt.h.in | 2 ++ src/driver.h | 6 ++++++ src/libvirt.c | 42 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 +++++ 4 files changed, 55 insertions(+) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 6e8d5dd..496a478 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1540,6 +1540,8 @@ int virDomainSetMemoryFlags (virDomainPtr domain, int virDomainGetMaxVcpus (virDomainPtr domain); int virDomainGetSecurityLabel (virDomainPtr domain, virSecurityLabelPtr seclabel); +char * virDomainGetHostname (virDomainPtr domain, + unsigned int flags); typedef enum { VIR_DOMAIN_METADATA_DESCRIPTION = 0, /* Operate on <description> */ diff --git a/src/driver.h b/src/driver.h index b3c1740..46d9846 100644 --- a/src/driver.h +++ b/src/driver.h @@ -142,6 +142,11 @@ typedef int unsigned int flags); typedef char * (*virDrvDomainGetOSType) (virDomainPtr domain); + +typedef char * + (*virDrvDomainGetHostname) (virDomainPtr domain, + unsigned int flags); + typedef unsigned long long (*virDrvDomainGetMaxMemory) (virDomainPtr domain); typedef int @@ -904,6 +909,7 @@ struct _virDriver { virDrvDomainDestroy domainDestroy; virDrvDomainDestroyFlags domainDestroyFlags; virDrvDomainGetOSType domainGetOSType; + virDrvDomainGetHostname domainGetHostname; virDrvDomainGetMaxMemory domainGetMaxMemory; virDrvDomainSetMaxMemory domainSetMaxMemory; virDrvDomainSetMemory domainSetMemory; diff --git a/src/libvirt.c b/src/libvirt.c index db6ba15..563482e 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -18973,3 +18973,45 @@ error: virDispatchError(dom->conn); return -1; } + +/** + * virDomainGetHostname: + * @domain: a domain object + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Get the hostname for that domain + * + * + * Returns a pointer to the name or NULL. + */ +char * +virDomainGetHostname(virDomainPtr domain, unsigned int flags) +{ + virConnectPtr conn; + + VIR_DEBUG("domain=%p", domain); + + virResetLastError(); + + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + virDispatchError(NULL); + return NULL; + } + + conn = domain->conn; + + if (conn->driver->domainGetHostname) { + char *ret; + ret = conn->driver->domainGetHostname (domain, flags); + if (!ret) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(domain->conn); + return NULL; +} diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 2913a81..1a8e58a 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -544,4 +544,9 @@ LIBVIRT_0.9.13 { virDomainSnapshotRef; } LIBVIRT_0.9.11; +LIBVIRT_0.9.14 { + global: + virDomainGetHostname; +} LIBVIRT_0.9.13; + # .... define new API here using predicted next version number .... -- 1.7.10.4

In the subject: s/virGetHostname/virDomainGetHostname/ On 07/10/2012 02:46 PM, Guido Günther wrote:
to query a guests's hostname. Containers like LXC and OpenVZ allow to set a hostname different from the hosts name and QEMU's guest agent could provide similar functionality. --- include/libvirt/libvirt.h.in | 2 ++ src/driver.h | 6 ++++++ src/libvirt.c | 42 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 +++++ 4 files changed, 55 insertions(+)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 6e8d5dd..496a478 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1540,6 +1540,8 @@ int virDomainSetMemoryFlags (virDomainPtr domain, int virDomainGetMaxVcpus (virDomainPtr domain); int virDomainGetSecurityLabel (virDomainPtr domain, virSecurityLabelPtr seclabel); +char * virDomainGetHostname (virDomainPtr domain, + unsigned int flags);
Seems like it might be useful.
+ +/** + * virDomainGetHostname: + * @domain: a domain object + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Get the hostname for that domain
Missing an ending '.'. Maybe also mention that for some hypervisors, the guest may need to be running a guest agent.
+ * + *
Extra blank line.
+ * Returns a pointer to the name or NULL.
Mention that caller must free the result.
+ */ +char * +virDomainGetHostname(virDomainPtr domain, unsigned int flags) +{ + virConnectPtr conn; + + VIR_DEBUG("domain=%p", domain);
Better to use: VIR_DOMAIN_DEBUG(domain)
+++ b/src/libvirt_public.syms @@ -544,4 +544,9 @@ LIBVIRT_0.9.13 { virDomainSnapshotRef; } LIBVIRT_0.9.11;
+LIBVIRT_0.9.14 {
We are considering naming the next release 0.10.0; but this can be fixed up globally when we solidify the naming decision.
+ global: + virDomainGetHostname; +} LIBVIRT_0.9.13; + # .... define new API here using predicted next version number ....
-- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

to query the guest's hostname. --- tools/virsh.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 4 ++++ 2 files changed, 48 insertions(+) diff --git a/tools/virsh.c b/tools/virsh.c index 85b1185..591a1ce 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -14131,6 +14131,49 @@ cmdTTYConsole(vshControl *ctl, const vshCmd *cmd) } /* + * "domhostname" command + */ +static const vshCmdInfo info_domhostname[] = { + {"help", N_("print the domain's hostname")}, + {"desc", ""}, + {NULL, NULL} +}; + +static const vshCmdOptDef opts_domhostname[] = { + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {NULL, 0, 0, NULL} +}; + +static bool +cmdDomHostname (vshControl *ctl, const vshCmd *cmd) +{ + char *hostname; + virDomainPtr dom; + bool ret = false; + + if (!vshConnectionUsability(ctl, ctl->conn)) + return false; + + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + return false; + + hostname = virDomainGetHostname (dom, 0); + if (hostname == NULL) { + vshError(ctl, "%s", _("failed to get hostname")); + goto error; + } + + vshPrint (ctl, "%s\n", hostname); + ret = true; + +error: + VIR_FREE(hostname); + virDomainFree(dom); + return ret; +} + + +/* * "attach-device" command */ static const vshCmdInfo info_attach_device[] = { @@ -18169,6 +18212,7 @@ static const vshCmdDef domManagementCmds[] = { {"detach-interface", cmdDetachInterface, opts_detach_interface, info_detach_interface, 0}, {"domdisplay", cmdDomDisplay, opts_domdisplay, info_domdisplay, 0}, + {"domhostname", cmdDomHostname, opts_domhostname, info_domhostname, 0}, {"domid", cmdDomid, opts_domid, info_domid, 0}, {"domif-setlink", cmdDomIfSetLink, opts_domif_setlink, info_domif_setlink, 0}, {"domiftune", cmdDomIftune, opts_domiftune, info_domiftune, 0}, diff --git a/tools/virsh.pod b/tools/virsh.pod index 93fdac7..93ced24 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -826,6 +826,10 @@ Output a URI which can be used to connect to the graphical display of the domain via VNC, SPICE or RDP. If I<--include-password> is specified, the SPICE channel password will be included in the URI. +=item B<domhostname> I<domain-id> + +Returns the hostname of a domain. + =item B<dominfo> I<domain-id> Returns basic information about the domain. -- 1.7.10.4

On 07/10/2012 02:46 PM, Guido Günther wrote:
to query the guest's hostname. --- tools/virsh.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 4 ++++ 2 files changed, 48 insertions(+)
+ +static bool +cmdDomHostname (vshControl *ctl, const vshCmd *cmd)
Style: no space before function (), here...
+{ + char *hostname; + virDomainPtr dom; + bool ret = false; + + if (!vshConnectionUsability(ctl, ctl->conn)) + return false; + + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + return false; + + hostname = virDomainGetHostname (dom, 0);
here...
+ if (hostname == NULL) { + vshError(ctl, "%s", _("failed to get hostname")); + goto error; + } + + vshPrint (ctl, "%s\n", hostname);
and here. Other than that, looks reasonable. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

to match our CodingStyle. --- This avoids c'n'p problems as seen in my recent domhostname patch. Cheers, -- Guido tools/virsh.c | 186 ++++++++++++++++++++++++++++----------------------------- 1 file changed, 93 insertions(+), 93 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 2c0446c..2c2f9f1 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1405,7 +1405,7 @@ cmdList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) VIR_FREE(title); } else if (optHostname) { - if (!(hostname = virDomainGetHostname (dom, 0))) + if (!(hostname = virDomainGetHostname(dom, 0))) goto cleanup; vshPrint(ctl, " %-5s %-30s %-10s %-20s\n", id_buf, @@ -1742,7 +1742,7 @@ static const struct _domblkstat_sequence domblkstat_output[] = { VALUE); static bool -cmdDomblkstat (vshControl *ctl, const vshCmd *cmd) +cmdDomblkstat(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom; const char *name = NULL, *device = NULL; @@ -1800,7 +1800,7 @@ cmdDomblkstat (vshControl *ctl, const vshCmd *cmd) } else { params = vshCalloc(ctl, nparams, sizeof(*params)); - if (virDomainBlockStatsFlags (dom, device, params, &nparams, 0) < 0) { + if (virDomainBlockStatsFlags(dom, device, params, &nparams, 0) < 0) { vshError(ctl, _("Failed to get block stats %s %s"), name, device); goto cleanup; } @@ -1875,52 +1875,52 @@ static const vshCmdOptDef opts_domifstat[] = { }; static bool -cmdDomIfstat (vshControl *ctl, const vshCmd *cmd) +cmdDomIfstat(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom; const char *name = NULL, *device = NULL; struct _virDomainInterfaceStats stats; - if (!vshConnectionUsability (ctl, ctl->conn)) + if (!vshConnectionUsability(ctl, ctl->conn)) return false; - if (!(dom = vshCommandOptDomain (ctl, cmd, &name))) + if (!(dom = vshCommandOptDomain(ctl, cmd, &name))) return false; - if (vshCommandOptString (cmd, "interface", &device) <= 0) { + if (vshCommandOptString(cmd, "interface", &device) <= 0) { virDomainFree(dom); return false; } - if (virDomainInterfaceStats (dom, device, &stats, sizeof(stats)) == -1) { + if (virDomainInterfaceStats(dom, device, &stats, sizeof(stats)) == -1) { vshError(ctl, _("Failed to get interface stats %s %s"), name, device); virDomainFree(dom); return false; } if (stats.rx_bytes >= 0) - vshPrint (ctl, "%s rx_bytes %lld\n", device, stats.rx_bytes); + vshPrint(ctl, "%s rx_bytes %lld\n", device, stats.rx_bytes); if (stats.rx_packets >= 0) - vshPrint (ctl, "%s rx_packets %lld\n", device, stats.rx_packets); + vshPrint(ctl, "%s rx_packets %lld\n", device, stats.rx_packets); if (stats.rx_errs >= 0) - vshPrint (ctl, "%s rx_errs %lld\n", device, stats.rx_errs); + vshPrint(ctl, "%s rx_errs %lld\n", device, stats.rx_errs); if (stats.rx_drop >= 0) - vshPrint (ctl, "%s rx_drop %lld\n", device, stats.rx_drop); + vshPrint(ctl, "%s rx_drop %lld\n", device, stats.rx_drop); if (stats.tx_bytes >= 0) - vshPrint (ctl, "%s tx_bytes %lld\n", device, stats.tx_bytes); + vshPrint(ctl, "%s tx_bytes %lld\n", device, stats.tx_bytes); if (stats.tx_packets >= 0) - vshPrint (ctl, "%s tx_packets %lld\n", device, stats.tx_packets); + vshPrint(ctl, "%s tx_packets %lld\n", device, stats.tx_packets); if (stats.tx_errs >= 0) - vshPrint (ctl, "%s tx_errs %lld\n", device, stats.tx_errs); + vshPrint(ctl, "%s tx_errs %lld\n", device, stats.tx_errs); if (stats.tx_drop >= 0) - vshPrint (ctl, "%s tx_drop %lld\n", device, stats.tx_drop); + vshPrint(ctl, "%s tx_drop %lld\n", device, stats.tx_drop); virDomainFree(dom); return true; @@ -1944,7 +1944,7 @@ static const vshCmdOptDef opts_domif_setlink[] = { }; static bool -cmdDomIfSetLink (vshControl *ctl, const vshCmd *cmd) +cmdDomIfSetLink(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom; const char *iface; @@ -2120,7 +2120,7 @@ static const vshCmdOptDef opts_domif_getlink[] = { }; static bool -cmdDomIfGetLink (vshControl *ctl, const vshCmd *cmd) +cmdDomIfGetLink(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom; const char *iface = NULL; @@ -2138,13 +2138,13 @@ cmdDomIfGetLink (vshControl *ctl, const vshCmd *cmd) xmlNodePtr cur = NULL; xmlXPathObjectPtr obj = NULL; - if (!vshConnectionUsability (ctl, ctl->conn)) + if (!vshConnectionUsability(ctl, ctl->conn)) return false; - if (!(dom = vshCommandOptDomain (ctl, cmd, NULL))) + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return false; - if (vshCommandOptString (cmd, "interface", &iface) <= 0) { + if (vshCommandOptString(cmd, "interface", &iface) <= 0) { virDomainFree(dom); return false; } @@ -2455,7 +2455,7 @@ cmdDomMemStat(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, &name))) return false; - nr_stats = virDomainMemoryStats (dom, stats, VIR_DOMAIN_MEMORY_STAT_NR, 0); + nr_stats = virDomainMemoryStats(dom, stats, VIR_DOMAIN_MEMORY_STAT_NR, 0); if (nr_stats == -1) { vshError(ctl, _("Failed to get memory statistics for domain %s"), name); virDomainFree(dom); @@ -2464,21 +2464,21 @@ cmdDomMemStat(vshControl *ctl, const vshCmd *cmd) for (i = 0; i < nr_stats; i++) { if (stats[i].tag == VIR_DOMAIN_MEMORY_STAT_SWAP_IN) - vshPrint (ctl, "swap_in %llu\n", stats[i].val); + vshPrint(ctl, "swap_in %llu\n", stats[i].val); if (stats[i].tag == VIR_DOMAIN_MEMORY_STAT_SWAP_OUT) - vshPrint (ctl, "swap_out %llu\n", stats[i].val); + vshPrint(ctl, "swap_out %llu\n", stats[i].val); if (stats[i].tag == VIR_DOMAIN_MEMORY_STAT_MAJOR_FAULT) - vshPrint (ctl, "major_fault %llu\n", stats[i].val); + vshPrint(ctl, "major_fault %llu\n", stats[i].val); if (stats[i].tag == VIR_DOMAIN_MEMORY_STAT_MINOR_FAULT) - vshPrint (ctl, "minor_fault %llu\n", stats[i].val); + vshPrint(ctl, "minor_fault %llu\n", stats[i].val); if (stats[i].tag == VIR_DOMAIN_MEMORY_STAT_UNUSED) - vshPrint (ctl, "unused %llu\n", stats[i].val); + vshPrint(ctl, "unused %llu\n", stats[i].val); if (stats[i].tag == VIR_DOMAIN_MEMORY_STAT_AVAILABLE) - vshPrint (ctl, "available %llu\n", stats[i].val); + vshPrint(ctl, "available %llu\n", stats[i].val); if (stats[i].tag == VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON) - vshPrint (ctl, "actual %llu\n", stats[i].val); + vshPrint(ctl, "actual %llu\n", stats[i].val); if (stats[i].tag == VIR_DOMAIN_MEMORY_STAT_RSS) - vshPrint (ctl, "rss %llu\n", stats[i].val); + vshPrint(ctl, "rss %llu\n", stats[i].val); } virDomainFree(dom); @@ -2514,7 +2514,7 @@ cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return false; - if (vshCommandOptString (cmd, "device", &device) <= 0) { + if (vshCommandOptString(cmd, "device", &device) <= 0) { virDomainFree(dom); return false; } @@ -3541,7 +3541,7 @@ doSave(void *opaque) out: pthread_sigmask(SIG_SETMASK, &oldsigmask, NULL); out_sig: - if (dom) virDomainFree (dom); + if (dom) virDomainFree(dom); VIR_FREE(xml); ignore_value(safewrite(data->writefd, &ret, sizeof(ret))); } @@ -3820,7 +3820,7 @@ out: pthread_sigmask(SIG_SETMASK, &oldsigmask, NULL); out_sig: if (dom) - virDomainFree (dom); + virDomainFree(dom); ignore_value(safewrite(data->writefd, &ret, sizeof(ret))); } @@ -4297,7 +4297,7 @@ out: pthread_sigmask(SIG_SETMASK, &oldsigmask, NULL); out_sig: if (dom) - virDomainFree (dom); + virDomainFree(dom); ignore_value(safewrite(data->writefd, &ret, sizeof(ret))); } @@ -7066,18 +7066,18 @@ static const vshCmdInfo info_capabilities[] = { }; static bool -cmdCapabilities (vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) +cmdCapabilities(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) { char *caps; if (!vshConnectionUsability(ctl, ctl->conn)) return false; - if ((caps = virConnectGetCapabilities (ctl->conn)) == NULL) { + if ((caps = virConnectGetCapabilities(ctl->conn)) == NULL) { vshError(ctl, "%s", _("failed to get capabilities")); return false; } - vshPrint (ctl, "%s\n", caps); + vshPrint(ctl, "%s\n", caps); VIR_FREE(caps); return true; @@ -7364,7 +7364,7 @@ static const vshCmdOptDef opts_migrate[] = { }; static void -doMigrate (void *opaque) +doMigrate(void *opaque) { char ret = '1'; virDomainPtr dom = NULL; @@ -7384,10 +7384,10 @@ doMigrate (void *opaque) if (pthread_sigmask(SIG_BLOCK, &sigmask, &oldsigmask) < 0) goto out_sig; - if (!vshConnectionUsability (ctl, ctl->conn)) + if (!vshConnectionUsability(ctl, ctl->conn)) goto out; - if (!(dom = vshCommandOptDomain (ctl, cmd, NULL))) + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) goto out; if (vshCommandOptString(cmd, "desturi", &desturi) <= 0 || @@ -7452,7 +7452,7 @@ doMigrate (void *opaque) virConnectPtr dconn = NULL; virDomainPtr ddom = NULL; - dconn = virConnectOpenAuth (desturi, virConnectAuthPtrDefault, 0); + dconn = virConnectOpenAuth(desturi, virConnectAuthPtrDefault, 0); if (!dconn) goto out; ddom = virDomainMigrate2(dom, dconn, xml, flags, dname, migrateuri, 0); @@ -7460,13 +7460,13 @@ doMigrate (void *opaque) virDomainFree(ddom); ret = '0'; } - virConnectClose (dconn); + virConnectClose(dconn); } out: pthread_sigmask(SIG_SETMASK, &oldsigmask, NULL); out_sig: - if (dom) virDomainFree (dom); + if (dom) virDomainFree(dom); VIR_FREE(xml); ignore_value(safewrite(data->writefd, &ret, sizeof(ret))); } @@ -9378,7 +9378,7 @@ cmdInterfaceDefine(vshControl *ctl, const vshCmd *cmd) if (iface != NULL) { vshPrint(ctl, _("Interface %s defined from %s\n"), virInterfaceGetName(iface), from); - virInterfaceFree (iface); + virInterfaceFree(iface); } else { vshError(ctl, _("Failed to define interface from %s"), from); ret = false; @@ -12073,7 +12073,7 @@ cmdVolUploadSource(virStreamPtr st ATTRIBUTE_UNUSED, } static bool -cmdVolUpload (vshControl *ctl, const vshCmd *cmd) +cmdVolUpload(vshControl *ctl, const vshCmd *cmd) { const char *file = NULL; virStorageVolPtr vol = NULL; @@ -12164,7 +12164,7 @@ static const vshCmdOptDef opts_vol_download[] = { }; static bool -cmdVolDownload (vshControl *ctl, const vshCmd *cmd) +cmdVolDownload(vshControl *ctl, const vshCmd *cmd) { const char *file = NULL; virStorageVolPtr vol = NULL; @@ -13655,7 +13655,7 @@ static const vshCmdOptDef opts_node_device_detach[] = { }; static bool -cmdNodeDeviceDetach (vshControl *ctl, const vshCmd *cmd) +cmdNodeDeviceDetach(vshControl *ctl, const vshCmd *cmd) { const char *name = NULL; virNodeDevicePtr device; @@ -13698,7 +13698,7 @@ static const vshCmdOptDef opts_node_device_reattach[] = { }; static bool -cmdNodeDeviceReAttach (vshControl *ctl, const vshCmd *cmd) +cmdNodeDeviceReAttach(vshControl *ctl, const vshCmd *cmd) { const char *name = NULL; virNodeDevicePtr device; @@ -13739,7 +13739,7 @@ static const vshCmdOptDef opts_node_device_reset[] = { }; static bool -cmdNodeDeviceReset (vshControl *ctl, const vshCmd *cmd) +cmdNodeDeviceReset(vshControl *ctl, const vshCmd *cmd) { const char *name = NULL; virNodeDevicePtr device; @@ -13774,14 +13774,14 @@ static const vshCmdInfo info_hostname[] = { }; static bool -cmdHostname (vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) +cmdHostname(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) { char *hostname; if (!vshConnectionUsability(ctl, ctl->conn)) return false; - hostname = virConnectGetHostname (ctl->conn); + hostname = virConnectGetHostname(ctl->conn); if (hostname == NULL) { vshError(ctl, "%s", _("failed to get hostname")); return false; @@ -13803,20 +13803,20 @@ static const vshCmdInfo info_uri[] = { }; static bool -cmdURI (vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) +cmdURI(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) { char *uri; if (!vshConnectionUsability(ctl, ctl->conn)) return false; - uri = virConnectGetURI (ctl->conn); + uri = virConnectGetURI(ctl->conn); if (uri == NULL) { vshError(ctl, "%s", _("failed to get URI")); return false; } - vshPrint (ctl, "%s\n", uri); + vshPrint(ctl, "%s\n", uri); VIR_FREE(uri); return true; @@ -13833,20 +13833,20 @@ static const vshCmdInfo info_sysinfo[] = { }; static bool -cmdSysinfo (vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) +cmdSysinfo(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) { char *sysinfo; if (!vshConnectionUsability(ctl, ctl->conn)) return false; - sysinfo = virConnectGetSysinfo (ctl->conn, 0); + sysinfo = virConnectGetSysinfo(ctl->conn, 0); if (sysinfo == NULL) { vshError(ctl, "%s", _("failed to get sysinfo")); return false; } - vshPrint (ctl, "%s", sysinfo); + vshPrint(ctl, "%s", sysinfo); VIR_FREE(sysinfo); return true; @@ -14161,7 +14161,7 @@ static const vshCmdOptDef opts_domhostname[] = { }; static bool -cmdDomHostname (vshControl *ctl, const vshCmd *cmd) +cmdDomHostname(vshControl *ctl, const vshCmd *cmd) { char *hostname; virDomainPtr dom; @@ -14173,13 +14173,13 @@ cmdDomHostname (vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return false; - hostname = virDomainGetHostname (dom, 0); + hostname = virDomainGetHostname(dom, 0); if (hostname == NULL) { vshError(ctl, "%s", _("failed to get hostname")); goto error; } - vshPrint (ctl, "%s\n", hostname); + vshPrint(ctl, "%s\n", hostname); ret = true; error: @@ -15962,7 +15962,7 @@ no_memory: /* Common code for the edit / net-edit / pool-edit functions which follow. */ static char * -editWriteToTempFile (vshControl *ctl, const char *doc) +editWriteToTempFile(vshControl *ctl, const char *doc) { char *ret; const char *tmpdir; @@ -15982,18 +15982,18 @@ editWriteToTempFile (vshControl *ctl, const char *doc) return NULL; } - if (safewrite (fd, doc, strlen (doc)) == -1) { + if (safewrite(fd, doc, strlen(doc)) == -1) { vshError(ctl, _("write: %s: failed to write to temporary file: %s"), ret, strerror(errno)); VIR_FORCE_CLOSE(fd); - unlink (ret); + unlink(ret); VIR_FREE(ret); return NULL; } if (VIR_CLOSE(fd) < 0) { vshError(ctl, _("close: %s: failed to write or close temporary file: %s"), ret, strerror(errno)); - unlink (ret); + unlink(ret); VIR_FREE(ret); return NULL; } @@ -16007,7 +16007,7 @@ editWriteToTempFile (vshControl *ctl, const char *doc) "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-/_.:@" static int -editFile (vshControl *ctl, const char *filename) +editFile(vshControl *ctl, const char *filename) { const char *editor; virCommandPtr cmd; @@ -16015,9 +16015,9 @@ editFile (vshControl *ctl, const char *filename) int outfd = STDOUT_FILENO; int errfd = STDERR_FILENO; - editor = getenv ("VISUAL"); + editor = getenv("VISUAL"); if (!editor) - editor = getenv ("EDITOR"); + editor = getenv("EDITOR"); if (!editor) editor = "vi"; /* could be cruel & default to ed(1) here */ @@ -16029,8 +16029,8 @@ editFile (vshControl *ctl, const char *filename) * is why sudo scrubs it by default). Conversely, if the editor * is safe, we can run it directly rather than wasting a shell. */ - if (strspn (editor, ACCEPTED_CHARS) != strlen (editor)) { - if (strspn (filename, ACCEPTED_CHARS) != strlen (filename)) { + if (strspn(editor, ACCEPTED_CHARS) != strlen(editor)) { + if (strspn(filename, ACCEPTED_CHARS) != strlen(filename)) { vshError(ctl, _("%s: temporary filename contains shell meta or other " "unacceptable characters (is $TMPDIR wrong?)"), @@ -16059,7 +16059,7 @@ cleanup: } static char * -editReadBackFile (vshControl *ctl, const char *filename) +editReadBackFile(vshControl *ctl, const char *filename) { char *ret; @@ -16135,7 +16135,7 @@ cmdPwd(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) strerror(errno)); ret = false; } else { - vshPrint (ctl, _("%s\n"), cwd); + vshPrint(ctl, _("%s\n"), cwd); VIR_FREE(cwd); } @@ -16163,7 +16163,7 @@ static const vshCmdOptDef opts_echo[] = { * quotes for later evaluation. */ static bool -cmdEcho (vshControl *ctl, const vshCmd *cmd) +cmdEcho(vshControl *ctl, const vshCmd *cmd) { bool shell = false; bool xml = false; @@ -16248,7 +16248,7 @@ cmdEdit(vshControl *ctl, const vshCmd *cmd) #define EDIT_GET_XML virDomainGetXMLDesc(dom, flags) #define EDIT_NOT_CHANGED \ vshPrint(ctl, _("Domain %s XML configuration not changed.\n"), \ - virDomainGetName (dom)); \ + virDomainGetName(dom)); \ ret = true; goto edit_cleanup; #define EDIT_DEFINE \ (dom_edited = virDomainDefineXML(ctl->conn, doc_edited)) @@ -16320,7 +16320,7 @@ cmdNetworkEdit(vshControl *ctl, const vshCmd *cmd) #define EDIT_GET_XML vshNetworkGetXMLDesc(network) #define EDIT_NOT_CHANGED \ vshPrint(ctl, _("Network %s XML configuration not changed.\n"), \ - virNetworkGetName (network)); \ + virNetworkGetName(network)); \ ret = true; goto edit_cleanup; #define EDIT_DEFINE \ (network_edited = virNetworkDefineXML(ctl->conn, doc_edited)) @@ -16389,7 +16389,7 @@ cmdPoolEdit(vshControl *ctl, const vshCmd *cmd) #define EDIT_GET_XML virStoragePoolGetXMLDesc(pool, flags) #define EDIT_NOT_CHANGED \ vshPrint(ctl, _("Pool %s XML configuration not changed.\n"), \ - virStoragePoolGetName (pool)); \ + virStoragePoolGetName(pool)); \ ret = true; goto edit_cleanup; #define EDIT_DEFINE \ (pool_edited = virStoragePoolDefineXML(ctl->conn, doc_edited, 0)) @@ -19192,14 +19192,14 @@ vshCommandOptArgv(const vshCmd *cmd, const vshCmdOpt *opt) If not, give a diagnostic and return false. If so, return true. */ static bool -cmd_has_option (vshControl *ctl, const vshCmd *cmd, const char *optname) +cmd_has_option(vshControl *ctl, const vshCmd *cmd, const char *optname) { /* Iterate through cmd->opts, to ensure that there is an entry with name OPTNAME and type VSH_OT_DATA. */ bool found = false; const vshCmdOpt *opt; for (opt = cmd->opts; opt; opt = opt->next) { - if (STREQ (opt->def->name, optname) && opt->def->type == VSH_OT_DATA) { + if (STREQ(opt->def->name, optname) && opt->def->type == VSH_OT_DATA) { found = true; break; } @@ -19219,7 +19219,7 @@ vshCommandOptDomainBy(vshControl *ctl, const vshCmd *cmd, const char *n = NULL; int id; const char *optname = "domain"; - if (!cmd_has_option (ctl, cmd, optname)) + if (!cmd_has_option(ctl, cmd, optname)) return NULL; if (vshCommandOptString(cmd, optname, &n) <= 0) @@ -19266,7 +19266,7 @@ vshCommandOptNetworkBy(vshControl *ctl, const vshCmd *cmd, virNetworkPtr network = NULL; const char *n = NULL; const char *optname = "network"; - if (!cmd_has_option (ctl, cmd, optname)) + if (!cmd_has_option(ctl, cmd, optname)) return NULL; if (vshCommandOptString(cmd, optname, &n) <= 0) @@ -19305,7 +19305,7 @@ vshCommandOptNWFilterBy(vshControl *ctl, const vshCmd *cmd, virNWFilterPtr nwfilter = NULL; const char *n = NULL; const char *optname = "nwfilter"; - if (!cmd_has_option (ctl, cmd, optname)) + if (!cmd_has_option(ctl, cmd, optname)) return NULL; if (vshCommandOptString(cmd, optname, &n) <= 0) @@ -19346,7 +19346,7 @@ vshCommandOptInterfaceBy(vshControl *ctl, const vshCmd *cmd, if (!optname) optname = "interface"; - if (!cmd_has_option (ctl, cmd, optname)) + if (!cmd_has_option(ctl, cmd, optname)) return NULL; if (vshCommandOptString(cmd, optname, &n) <= 0) @@ -19479,7 +19479,7 @@ vshCommandOptSecret(vshControl *ctl, const vshCmd *cmd, const char **name) const char *n = NULL; const char *optname = "secret"; - if (!cmd_has_option (ctl, cmd, optname)) + if (!cmd_has_option(ctl, cmd, optname)) return NULL; if (vshCommandOptString(cmd, optname, &n) <= 0) @@ -19565,7 +19565,7 @@ typedef enum { } vshCommandToken; typedef struct __vshCommandParser { - vshCommandToken (*getNextArg)(vshControl *, struct __vshCommandParser *, + vshCommandToken(*getNextArg)(vshControl *, struct __vshCommandParser *, char **); /* vshCommandStringGetArg() */ char *pos; @@ -20681,7 +20681,7 @@ vshReadlineInit(vshControl *ctl) } static void -vshReadlineDeinit (vshControl *ctl) +vshReadlineDeinit(vshControl *ctl) { if (ctl->historyfile != NULL) { if (mkdir(ctl->historydir, 0755) < 0 && errno != EEXIST) { @@ -20698,43 +20698,43 @@ vshReadlineDeinit (vshControl *ctl) } static char * -vshReadline (vshControl *ctl ATTRIBUTE_UNUSED, const char *prompt) +vshReadline(vshControl *ctl ATTRIBUTE_UNUSED, const char *prompt) { - return readline (prompt); + return readline(prompt); } #else /* !USE_READLINE */ static int -vshReadlineInit (vshControl *ctl ATTRIBUTE_UNUSED) +vshReadlineInit(vshControl *ctl ATTRIBUTE_UNUSED) { /* empty */ return 0; } static void -vshReadlineDeinit (vshControl *ctl ATTRIBUTE_UNUSED) +vshReadlineDeinit(vshControl *ctl ATTRIBUTE_UNUSED) { /* empty */ } static char * -vshReadline (vshControl *ctl, const char *prompt) +vshReadline(vshControl *ctl, const char *prompt) { char line[1024]; char *r; int len; - fputs (prompt, stdout); - r = fgets (line, sizeof(line), stdin); + fputs(prompt, stdout); + r = fgets(line, sizeof(line), stdin); if (r == NULL) return NULL; /* EOF */ /* Chomp trailing \n */ - len = strlen (r); + len = strlen(r); if (len > 0 && r[len-1] == '\n') r[len-1] = '\0'; - return vshStrdup (ctl, r); + return vshStrdup(ctl, r); } #endif /* !USE_READLINE */ -- 1.7.10.4

On 07/11/12 10:39, Guido Günther wrote:
to match our CodingStyle. --- This avoids c'n'p problems as seen in my recent domhostname patch. Cheers, -- Guido
tools/virsh.c | 186 ++++++++++++++++++++++++++++----------------------------- 1 file changed, 93 insertions(+), 93 deletions(-)
I couldn't apply this patch to current head, but it looks OK to me and it's a nice cleanup. Peter

On Wed, Jul 11, 2012 at 11:11:46AM +0200, Peter Krempa wrote:
On 07/11/12 10:39, Guido Günther wrote:
to match our CodingStyle. --- This avoids c'n'p problems as seen in my recent domhostname patch. Cheers, -- Guido
tools/virsh.c | 186 ++++++++++++++++++++++++++++----------------------------- 1 file changed, 93 insertions(+), 93 deletions(-)
I couldn't apply this patch to current head, but it looks OK to me and it's a nice cleanup.
I rebased on top of current master and pushed this. Thanks, -- Guido

to retrieve a VEs config parameters as a single string. This will be used by the upcoming domainGetHostname implementation. --- src/libvirt_openvz.syms | 2 +- src/openvz/openvz_util.c | 31 +++++++++++++++++++++++++++++++ src/openvz/openvz_util.h | 1 + 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/src/libvirt_openvz.syms b/src/libvirt_openvz.syms index 11c5587..1993eb5 100644 --- a/src/libvirt_openvz.syms +++ b/src/libvirt_openvz.syms @@ -1,7 +1,7 @@ # # These symbols are dependent upon --with-openvz via WITH_OPENVZ # - openvzReadConfigParam; openvzReadNetworkConf; openvzLocateConfFile; +openvzVEGetStringParam; diff --git a/src/openvz/openvz_util.c b/src/openvz/openvz_util.c index 61b55de..b172104 100644 --- a/src/openvz/openvz_util.c +++ b/src/openvz/openvz_util.c @@ -26,6 +26,9 @@ #include "internal.h" #include "virterror_internal.h" +#include "command.h" +#include "datatypes.h" +#include "memory.h" #include "openvz_conf.h" #include "openvz_util.h" @@ -49,3 +52,31 @@ openvzKBPerPages(void) } return kb_per_pages; } + +char* +openvzVEGetStringParam(virDomainPtr domain, const char* param) +{ + int status, len; + char *output = NULL; + + virCommandPtr cmd = virCommandNewArgList(VZLIST, + "-o", + param, + domain->name, + "-H" , NULL); + + virCommandSetOutputBuffer(cmd, &output); + if (virCommandRun(cmd, &status)) { + openvzError(VIR_ERR_OPERATION_FAILED, + _("Failed to get %s for %s: %d"), param, domain->name, + status); + VIR_FREE(output); + } + len = strlen(output); + /* delete trailing newline */ + if (len) + output[len-1] = '\0'; + + virCommandFree(cmd); + return output; +} diff --git a/src/openvz/openvz_util.h b/src/openvz/openvz_util.h index a0d9bbb..6a991f3 100644 --- a/src/openvz/openvz_util.h +++ b/src/openvz/openvz_util.h @@ -24,5 +24,6 @@ # define OPENVZ_UTIL_H long openvzKBPerPages(void); +char* openvzVEGetStringParam(virDomainPtr dom, const char *param); #endif -- 1.7.10.4

On 07/10/2012 02:46 PM, Guido Günther wrote:
+char* +openvzVEGetStringParam(virDomainPtr domain, const char* param) +{ + int status, len; + char *output = NULL; + + virCommandPtr cmd = virCommandNewArgList(VZLIST, + "-o", + param, + domain->name, + "-H" , NULL); + + virCommandSetOutputBuffer(cmd, &output); + if (virCommandRun(cmd, &status)) {
This should check for virCommandRun() < 0. Also, virCommandRun() does not guarantee that 'status' is populated unless it returns 0; if it returns -1, you may be left with outputting uninitilized data (maybe we could argue that it is a bug in virCommandRun that should be fixed...). Outputting the raw exit status isn't very handy; I think it would be simpler to just pass NULL for the second argument (which lets virCommandRun issue a reasonable error message about any failure to run the command, including the entire command line attempted, which already includes param and domain->name).
+ openvzError(VIR_ERR_OPERATION_FAILED, + _("Failed to get %s for %s: %d"), param, domain->name, + status); + VIR_FREE(output); + } + len = strlen(output);
Crash-tastic! If the command fails, then you end up calling strlen(NULL). Are you missing a goto or early return?
+ /* delete trailing newline */ + if (len) + output[len-1] = '\0';
Shouldn't you at least check that output[len-1] is '\n' before nuking it? Also, our style prefers spaces on either side of '-'. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

If the container doesn't have the hostname parameter set an empty string ("") is returned. --- src/openvz/openvz_driver.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index c9150e0..469d043 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -234,6 +234,33 @@ cleanup: } +static char* openvzDomainGetHostname(virDomainPtr dom, unsigned int flags) +{ + char *hostname = NULL; + + virCheckFlags(0, NULL); + + hostname = openvzVEGetStringParam(dom, "hostname"); + if (hostname == NULL) + goto cleanup; + + /* vzlist prints an unset hostname as '-' */ + if (strlen(hostname) == 1 && hostname[0] == '-') { + VIR_FREE(hostname); + hostname = strdup(""); + if (hostname == NULL) { + virReportOOMError(); + goto cleanup; + } + } + return hostname; + +cleanup: + VIR_FREE(hostname); + return NULL; +} + + static virDomainPtr openvzDomainLookupByID(virConnectPtr conn, int id) { struct openvz_driver *driver = conn->privateData; @@ -2127,6 +2154,7 @@ static virDriver openvzDriver = { .domainIsUpdated = openvzDomainIsUpdated, /* 0.8.6 */ .isAlive = openvzIsAlive, /* 0.9.8 */ .domainUpdateDeviceFlags = openvzDomainUpdateDeviceFlags, /* 0.9.13 */ + .domainGetHostname = openvzDomainGetHostname, /* 0.9.14 */ }; int openvzRegister(void) { -- 1.7.10.4

On 07/10/2012 02:46 PM, Guido Günther wrote:
If the container doesn't have the hostname parameter set an empty string ("") is returned.
I'd almost rather return NULL and an error code (either invent a new error, or maybe reuse VIR_ERR_OPERATION_FAILED) than an empty string if we fail to get the information.
+static char* openvzDomainGetHostname(virDomainPtr dom, unsigned int flags) +{ + char *hostname = NULL; + + virCheckFlags(0, NULL); + + hostname = openvzVEGetStringParam(dom, "hostname"); + if (hostname == NULL) + goto cleanup; + + /* vzlist prints an unset hostname as '-' */ + if (strlen(hostname) == 1 && hostname[0] == '-') {
More efficient to write: if (STREQ(hostname, "-")) {
@@ -2127,6 +2154,7 @@ static virDriver openvzDriver = { .domainIsUpdated = openvzDomainIsUpdated, /* 0.8.6 */ .isAlive = openvzIsAlive, /* 0.9.8 */ .domainUpdateDeviceFlags = openvzDomainUpdateDeviceFlags, /* 0.9.13 */ + .domainGetHostname = openvzDomainGetHostname, /* 0.9.14 */
Again, a global version cleanup would fix this to 0.10.0. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

--- tools/virsh.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/tools/virsh.c b/tools/virsh.c index 591a1ce..2c0446c 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1292,6 +1292,7 @@ static const vshCmdOptDef opts_list[] = { {"managed-save", VSH_OT_BOOL, 0, N_("mark inactive domains with managed save state")}, {"title", VSH_OT_BOOL, 0, N_("show short domain description")}, + {"hostname", VSH_OT_BOOL, 0, N_("show domain hostname")}, {NULL, 0, 0, NULL} }; @@ -1307,8 +1308,9 @@ cmdList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) bool optTable = vshCommandOptBool(cmd, "table"); bool optUUID = vshCommandOptBool(cmd, "uuid"); bool optName = vshCommandOptBool(cmd, "name"); + bool optHostname = vshCommandOptBool(cmd, "hostname"); int i; - char *title; + char *title, *hostname; char uuid[VIR_UUID_STRING_BUFLEN]; int state; bool ret = false; @@ -1366,6 +1368,11 @@ cmdList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) _("Id"), _("Name"), _("State"), _("Title"), "-----------------------------------------" "-----------------------------------------"); + else if (optHostname) + vshPrintExtra(ctl, " %-5s %-30s %-10s %-20s\n%s\n", + _("Id"), _("Name"), _("State"), _("Hostname"), + "-----------------------------------------" + "-----------------------------------------"); else vshPrintExtra(ctl, " %-5s %-30s %s\n%s\n", _("Id"), _("Name"), _("State"), @@ -1397,6 +1404,15 @@ cmdList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) title); VIR_FREE(title); + } else if (optHostname) { + if (!(hostname = virDomainGetHostname (dom, 0))) + goto cleanup; + + vshPrint(ctl, " %-5s %-30s %-10s %-20s\n", id_buf, + virDomainGetName(dom), + state == -2 ? _("saved") : _(vshDomainStateToString(state)), + hostname); + VIR_FREE(hostname); } else { vshPrint(ctl, " %-5s %-30s %s\n", id_buf, virDomainGetName(dom), -- 1.7.10.4

On 07/10/12 22:46, Guido Günther wrote:
--- tools/virsh.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/tools/virsh.c b/tools/virsh.c index 591a1ce..2c0446c 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1292,6 +1292,7 @@ static const vshCmdOptDef opts_list[] = { {"managed-save", VSH_OT_BOOL, 0, N_("mark inactive domains with managed save state")}, {"title", VSH_OT_BOOL, 0, N_("show short domain description")}, + {"hostname", VSH_OT_BOOL, 0, N_("show domain hostname")}, {NULL, 0, 0, NULL} };
@@ -1307,8 +1308,9 @@ cmdList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) bool optTable = vshCommandOptBool(cmd, "table"); bool optUUID = vshCommandOptBool(cmd, "uuid"); bool optName = vshCommandOptBool(cmd, "name"); + bool optHostname = vshCommandOptBool(cmd, "hostname"); int i; - char *title; + char *title, *hostname; char uuid[VIR_UUID_STRING_BUFLEN]; int state; bool ret = false; @@ -1366,6 +1368,11 @@ cmdList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) _("Id"), _("Name"), _("State"), _("Title"), "-----------------------------------------" "-----------------------------------------"); + else if (optHostname) + vshPrintExtra(ctl, " %-5s %-30s %-10s %-20s\n%s\n", + _("Id"), _("Name"), _("State"), _("Hostname"), + "-----------------------------------------" + "-----------------------------------------"); else vshPrintExtra(ctl, " %-5s %-30s %s\n%s\n", _("Id"), _("Name"), _("State"), @@ -1397,6 +1404,15 @@ cmdList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) title);
VIR_FREE(title); + } else if (optHostname) { + if (!(hostname = virDomainGetHostname (dom, 0))) + goto cleanup; + + vshPrint(ctl, " %-5s %-30s %-10s %-20s\n", id_buf, + virDomainGetName(dom), + state == -2 ? _("saved") : _(vshDomainStateToString(state)), + hostname); + VIR_FREE(hostname); } else { vshPrint(ctl, " %-5s %-30s %s\n", id_buf, virDomainGetName(dom),
This approach has a problem: If you specify "list --title --hostname" you'll get just the title. When I was adding the Title option I figured that this situation would happen sooner or later. I thought about a universal table printer that could dynamicaly print desired columns, but that seemed a bit of overkill at that time. A quick option would be to make those option mutually exclusive, but that's not elegant. Also this patch misses adding docs for the new flag. Peter

On 07/10/2012 02:46 PM, Guido Günther wrote:
--- tools/virsh.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/tools/virsh.c b/tools/virsh.c index 591a1ce..2c0446c 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1292,6 +1292,7 @@ static const vshCmdOptDef opts_list[] = { {"managed-save", VSH_OT_BOOL, 0, N_("mark inactive domains with managed save state")}, {"title", VSH_OT_BOOL, 0, N_("show short domain description")}, + {"hostname", VSH_OT_BOOL, 0, N_("show domain hostname")}, {NULL, 0, 0, NULL}
I'm worried that we're trying to cram too much into 'list'. Would 'dominfo' be a better fit for this type of information, without needing a command line flag?
@@ -1366,6 +1368,11 @@ cmdList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) _("Id"), _("Name"), _("State"), _("Title"), "-----------------------------------------" "-----------------------------------------"); + else if (optHostname) + vshPrintExtra(ctl, " %-5s %-30s %-10s %-20s\n%s\n", + _("Id"), _("Name"), _("State"), _("Hostname"), + "-----------------------------------------" + "-----------------------------------------");
If we're going to support this in 'list', then we really need to implement a more generic way to format data into columns, with as many columns as needed per row.
else vshPrintExtra(ctl, " %-5s %-30s %s\n%s\n", _("Id"), _("Name"), _("State"), @@ -1397,6 +1404,15 @@ cmdList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) title);
VIR_FREE(title); + } else if (optHostname) { + if (!(hostname = virDomainGetHostname (dom, 0)))
No space before () in function calls. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On Tue, Jul 10, 2012 at 03:18:19PM -0600, Eric Blake wrote:
On 07/10/2012 02:46 PM, Guido Günther wrote:
--- tools/virsh.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/tools/virsh.c b/tools/virsh.c index 591a1ce..2c0446c 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1292,6 +1292,7 @@ static const vshCmdOptDef opts_list[] = { {"managed-save", VSH_OT_BOOL, 0, N_("mark inactive domains with managed save state")}, {"title", VSH_OT_BOOL, 0, N_("show short domain description")}, + {"hostname", VSH_OT_BOOL, 0, N_("show domain hostname")}, {NULL, 0, 0, NULL}
I'm worried that we're trying to cram too much into 'list'. Would 'dominfo' be a better fit for this type of information, without needing a command line flag?
This list output is useful so I'd rather go for a more generic table formatting. Since "virsh domhostname" is there to query the hostname, I'll drop this patch for now until I have time to work in this again. Cheers, -- Guido
@@ -1366,6 +1368,11 @@ cmdList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) _("Id"), _("Name"), _("State"), _("Title"), "-----------------------------------------" "-----------------------------------------"); + else if (optHostname) + vshPrintExtra(ctl, " %-5s %-30s %-10s %-20s\n%s\n", + _("Id"), _("Name"), _("State"), _("Hostname"), + "-----------------------------------------" + "-----------------------------------------");
If we're going to support this in 'list', then we really need to implement a more generic way to format data into columns, with as many columns as needed per row.
else vshPrintExtra(ctl, " %-5s %-30s %s\n%s\n", _("Id"), _("Name"), _("State"), @@ -1397,6 +1404,15 @@ cmdList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) title);
VIR_FREE(title); + } else if (optHostname) { + if (!(hostname = virDomainGetHostname (dom, 0)))
No space before () in function calls.
-- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 07/10/2012 02:45 PM, Guido Günther wrote:
The following patches allow to query a guest's hostname. The last patch might be debatable since it adds yet another option to virsh list but I hope the rest of the serious looks sane. Cheers, -- Guido
Guido Günther (5): Add virGetHostname virsh: Add domhostname openvz: Add openvzVEGetStringParam openvz: Implement domainGetHostname virsh: allow to print hostname in domain listings
Missing a patch to implement the remote protocol glue for this API. Even if openvz doesn't use it, and even if we don't code up a qemu or lxc implementation at this time, the rpc code should be pretty easy to add; having it now will let an early client (such as virsh) automatically be able to use the API when a later server finally implements it. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (3)
-
Eric Blake
-
Guido Günther
-
Peter Krempa