6:11 p.m.
On 04/20/2012 10:01 AM, Dmitry Guryanov wrote:
PVS driver is 'stateless', like vmware or openvz drivers.
It collects information about domains during startup using
command-line utility prlctl. VMs in PVS identified by UUIDs
s/identified/are identified/
or unique names, which can be used as respective fields in
virDomainDef structure. Currently only basic info, like
description, virtual cpus number and memory amount implemented.
s/amount implemented/amount, is implemented/
Quering devices information will be added in the next patches.
s/Quering/Querying/
PVS does't support non-persistent domains - you can't run
s/does't/doesn't/
a domain having only disk image, it must always be registered
in system.
Functions for quering domain info have been just copied from
s/quering/querying/
test driver with some changes - they extract needed data from
previouly created list of virDomainObj objects.
s/previouly/previously/
+++ b/po/POTFILES.in
@@ -165,6 +165,7 @@ src/xenapi/xenapi_driver.c
src/xenapi/xenapi_utils.c
src/xenxs/xen_sxpr.c
src/xenxs/xen_xm.c
+src/pvs/pvs_driver.c
tools/console.c
tools/libvirt-guests.init.sh
tools/virsh.c
I think this hunk belongs in 1/9.
@@ -77,6 +78,14 @@ pvsDefaultConsoleType(const char *ostype
ATTRIBUTE_UNUSED)
return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL;
}
+void
+pvsFreeDomObj(void *p)
+{
+ pvsDomObjPtr pdom = (pvsDomObjPtr) p;
+
+ VIR_FREE(pdom);
Simpler to just write VIR_FREE(p) and avoid the intermediate variable.
Also, this function can safely be marked static.
+/*
+ * Must be called with privconn->lock held
+ */
+static virDomainObjPtr
+pvsLoadDomain(pvsConnPtr privconn, virJSONValuePtr jobj)
+{
Long, but looks okay.
+/*
+ * Must be called with privconn->lock held
+ */
+static int
+pvsLoadDomains(pvsConnPtr privconn, const char *domain_name)
+{
+ int count, i;
+ virJSONValuePtr jobj;
+ virJSONValuePtr jobj2;
+ virDomainObjPtr dom = NULL;
+ int ret = -1;
+
+ jobj = pvsParseOutput(PRLCTL, "list", "-j", "-a",
+ "-i", "-H", domain_name, NULL);
I guess you can call this command with a domain name, to limit to one
output, or with no argument, to list all domains, given...
@@ -150,6 +371,9 @@ pvsOpenDefault(virConnectPtr conn)
if (virDomainObjListInit(&privconn->domains) < 0)
goto error;
+ if (pvsLoadDomains(privconn, NULL))
+ goto error;
this usage.
+static int
+pvsDomainIsPersistent(virDomainPtr dom ATTRIBUTE_UNUSED)
+{
+ return 1;
+}
I'm wondering if we should at least validate that dom still exists in
our hash table. But I can live with this implementation.
+
+static int
+pvsDomainGetState(virDomainPtr domain,
+ int *state, int *reason, unsigned int flags)
+{
+ pvsConnPtr privconn = domain->conn->privateData;
+ virDomainObjPtr privdom;
+ int ret = -1;
+ virCheckFlags(0, -1);
+
+ pvsDriverLock(privconn);
+ privdom = virDomainFindByName(&privconn->domains, domain->name);
+ pvsDriverUnlock(privconn);
+
+ if (privdom == NULL) {
+ pvsError(VIR_ERR_INVALID_ARG, __FUNCTION__);
Use of __FUNCTION__ as the error message is not a good habit, since
pvsError _already_ adds the function name automatically. You should
instead provide a real message, such as _("Domain '%s' not found"),
domain->name.
+static char *
+pvsDomainGetXMLDesc(virDomainPtr domain, unsigned int flags)
+{
+ pvsConnPtr privconn = domain->conn->privateData;
+ virDomainDefPtr def;
+ virDomainObjPtr privdom;
+ char *ret = NULL;
+
+ /* Flags checked by virDomainDefFormat */
+
+ pvsDriverLock(privconn);
+ privdom = virDomainFindByName(&privconn->domains, domain->name);
+ pvsDriverUnlock(privconn);
+
+ if (privdom == NULL) {
+ pvsError(VIR_ERR_INVALID_ARG, __FUNCTION__);
Likewise (and probably throughout the series, so I'll quit pointing it out).
# define pvsError(code, ...)
\
virReportErrorHelper(VIR_FROM_TEST, code, __FILE__, \
__FUNCTION__, __LINE__, __VA_ARGS__)
# define PRLCTL "prlctl"
+# define pvsParseError() \
+ virReportErrorHelper(VIR_FROM_TEST, VIR_ERR_OPERATION_FAILED, __FILE__, \
+ __FUNCTION__, __LINE__, "Can't parse prlctl
output")
Mark this string for translation: _("Can't parse prlctl output")
+
+struct pvsDomObj {
+ int id;
+ char *uuid;
+ char *os;
+};
+typedef struct pvsDomObj *pvsDomObjPtr;
struct _pvsConn {
virMutex lock;
@@ -48,4 +60,6 @@ typedef struct _pvsConn *pvsConnPtr;
int pvsRegister(void);
+virJSONValuePtr pvsParseOutput(const char *binary, ...);
Mark this with ATTRIBUTE_NONNULL(1) ATTRIBUTE_SENTINEL.
+
+static int
+pvsDoCmdRun(char **outbuf, const char *binary, va_list list)
+{
+ virCommandPtr cmd = virCommandNew(binary);
+ const char *arg;
+ int exitstatus;
+ char *scmd = NULL;
+ char *sstatus = NULL;
+ int ret = -1;
+
+ while ((arg = va_arg(list, const char *)) != NULL)
+ virCommandAddArg(cmd, arg);
+
+ if (outbuf)
+ virCommandSetOutputBuffer(cmd, outbuf);
+
+ scmd = virCommandToString(cmd);
+ if (!scmd)
+ goto cleanup;
+
+ if (virCommandRun(cmd, &exitstatus)) {
If you pass NULL as the second argument here,
+ pvsError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to execute command '%s'"), scmd);
+ goto cleanup;
+ }
+
+ if (exitstatus) {
+ sstatus = virCommandTranslateStatus(exitstatus);
+ pvsError(VIR_ERR_INTERNAL_ERROR,
+ _("Command '%s' finished with errors: %s"), scmd,
sstatus);
then virCommand will take care of ensuring a 0 exit status, and with
more uniform error reporting. No need to reinvent the work that someone
else will do for you :) The only reason to ever pass an &exitstatus
parameter is if you validly expect non-zero exit status.
+/*
+ * Run command and parse its JSON output, return
+ * pointer to virJSONValue or NULL in case of error.
+ */
+virJSONValuePtr
+pvsParseOutput(const char *binary, ...)
+{
+ char *outbuf;
+ virJSONValuePtr jobj = NULL;
+ va_list list;
+ int ret;
+
+ va_start(list, binary);
+ ret = pvsDoCmdRun(&outbuf, binary, list);
+ va_end(list);
+ if (ret)
+ return NULL;
+
+ jobj = virJSONValueFromString(outbuf);
+ if (!jobj)
+ pvsError(VIR_ERR_INTERNAL_ERROR, "%s: %s",
+ _("invalid output from prlctl"), outbuf);
+
+ return jobj;
You leak outbuf if you reach the virJSONValueFromString() call.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org