[libvirt] [libvirt-qpid PATCH 0/3] Conversion of libvirt-qpid to matahari
The following series converts libvirt-qpid into a matahari agent using the QMFv2 APIs. Since the patches are rather large, I have also pushed them to GitHub for easier reviewing: https://github.com/zaneb/libvirt-qpid/commits/review Any and all comments are welcome. thanks, Zane. --- Zane Bitter (3): Convert to QMFv2 APIs Convert class names to lower case Make libvirt-qpid a matahari agent AUTHORS | 4 configure.ac | 1 libvirt-qpid.spec | 6 src/DomainWrap.cpp | 444 ++++++++++++++++++--------------- src/DomainWrap.h | 49 +--- src/Error.h | 8 + src/Exception.cpp | 38 +++ src/Exception.h | 30 ++ src/LibvirtAgent.cpp | 97 +++++++ src/LibvirtAgent.h | 36 +++ src/Makefile.am | 34 +-- src/ManagedObject.h | 78 ++++++ src/NodeWrap.cpp | 643 ++++++++++++++++++++---------------------------- src/NodeWrap.h | 64 ++--- src/PoolWrap.cpp | 365 +++++++++++++++------------ src/PoolWrap.h | 60 ++-- src/VolumeWrap.cpp | 153 ++++++----- src/VolumeWrap.h | 58 +--- src/libvirt-schema.xml | 8 - 19 files changed, 1198 insertions(+), 978 deletions(-) create mode 100644 src/Exception.cpp create mode 100644 src/Exception.h create mode 100644 src/LibvirtAgent.cpp create mode 100644 src/LibvirtAgent.h create mode 100644 src/ManagedObject.h
Note that this version removes GSS support. In the future, authentication will be handled through matahari. --- AUTHORS | 4 libvirt-qpid.spec | 4 src/DomainWrap.cpp | 444 +++++++++++++++++++++++------------------ src/DomainWrap.h | 49 ++--- src/Error.h | 8 + src/Exception.cpp | 38 ++++ src/Exception.h | 30 +++ src/Makefile.am | 29 +-- src/ManagedObject.h | 78 +++++++ src/NodeWrap.cpp | 552 ++++++++++++++++++++++++++++----------------------- src/NodeWrap.h | 76 ++++--- src/PoolWrap.cpp | 365 +++++++++++++++++++--------------- src/PoolWrap.h | 60 ++---- src/VolumeWrap.cpp | 153 +++++++------- src/VolumeWrap.h | 58 ++--- 15 files changed, 1100 insertions(+), 848 deletions(-) create mode 100644 src/Exception.cpp create mode 100644 src/Exception.h create mode 100644 src/ManagedObject.h diff --git a/AUTHORS b/AUTHORS index d042b71..367ea49 100644 --- a/AUTHORS +++ b/AUTHORS @@ -8,3 +8,7 @@ The libvirt-qpid project was initiated by: Patches have also been contributed by: Ted Ross <tross@redhat.com> + +Conversion to QMFv2 API by: + + Zane Bitter <zbitter@redhat.com> diff --git a/libvirt-qpid.spec b/libvirt-qpid.spec index 5de1fd2..d64552d 100644 --- a/libvirt-qpid.spec +++ b/libvirt-qpid.spec @@ -8,12 +8,12 @@ License: LGPLv2+ Group: Applications/System Requires: libxml2 >= 2.7.1 Requires: qmf >= 0.5.790661 -Requires: qpid-cpp-client >= 0.5.790661 +Requires: qpid-cpp-client >= 0.10 Requires: libvirt >= 0.4.4 Requires(post): /sbin/chkconfig Requires(preun): /sbin/chkconfig Requires(preun): initscripts -BuildRequires: qpid-cpp-client-devel >= 0.5.790661 +BuildRequires: qpid-cpp-client-devel >= 0.10 BuildRequires: libxml2-devel >= 2.7.1 BuildRequires: libvirt-devel >= 0.5.0 BuildRequires: qmf-devel >= 0.5.790661 diff --git a/src/DomainWrap.cpp b/src/DomainWrap.cpp index 47876de..eab6bbc 100644 --- a/src/DomainWrap.cpp +++ b/src/DomainWrap.cpp @@ -2,266 +2,310 @@ #include "NodeWrap.h" #include "DomainWrap.h" #include "Error.h" +#include "Exception.h" -#include "ArgsDomainMigrate.h" -#include "ArgsDomainRestore.h" -#include "ArgsDomainSave.h" -#include "ArgsDomainGetXMLDesc.h" +#include <cstdio> -namespace _qmf = qmf::com::redhat::libvirt; -DomainWrap::DomainWrap(ManagementAgent *agent, NodeWrap *parent, - virDomainPtr _domain_ptr, virConnectPtr _conn) : - domain_ptr(_domain_ptr), conn(_conn) +DomainWrap::DomainWrap( + NodeWrap *parent, + virDomainPtr domain_ptr, + virConnectPtr conn): + PackageOwner<NodeWrap::PackageDefinition>(parent), + ManagedObject(package().data_Domain), + _domain_ptr(domain_ptr), _conn(conn) { char dom_uuid[VIR_UUID_STRING_BUFLEN]; - domain = NULL; - - if (virDomainGetUUIDString(domain_ptr, dom_uuid) < 0) { - REPORT_ERR(conn, "DomainWrap: Unable to get UUID string of domain."); + if (virDomainGetUUIDString(_domain_ptr, dom_uuid) < 0) { + REPORT_ERR(_conn, "DomainWrap: Unable to get UUID string of domain."); throw 1; } - domain_uuid = dom_uuid; + _domain_uuid = dom_uuid; - const char *dom_name = virDomainGetName(domain_ptr); + const char *dom_name = virDomainGetName(_domain_ptr); if (!dom_name) { - REPORT_ERR(conn, "Unable to get domain name!\n"); + REPORT_ERR(_conn, "Unable to get domain name!\n"); throw 1; } - domain_name = dom_name; + _domain_name = dom_name; + + _data.setProperty("uuid", _domain_uuid); + _data.setProperty("name", _domain_name); + _data.setProperty("node", parent->objectID()); + + // Set defaults + _data.setProperty("state", ""); + _data.setProperty("numVcpus", 0); + _data.setProperty("maximumMemory", 0); + _data.setProperty("memory", (uint64_t)0); + _data.setProperty("cpuTime", (uint64_t)0); + + addData(_data); - domain = new _qmf::Domain(agent, this, parent, domain_uuid, domain_name); - printf("Creating new domain object for %s\n", domain_name.c_str()); - agent->addObject(domain); + printf("Initialised new domain object for %s\n", _domain_name.c_str()); } DomainWrap::~DomainWrap() { - if (domain) { - domain->resourceDestroy(); - } - virDomainFree(domain_ptr); + virDomainFree(_domain_ptr); + delData(_data); } - void DomainWrap::update() { virDomainInfo info; int ret; - ret = virDomainGetInfo(domain_ptr, &info); + ret = virDomainGetInfo(_domain_ptr, &info); if (ret < 0) { - REPORT_ERR(conn, "Domain get info failed."); + REPORT_ERR(_conn, "Domain get info failed."); /* Next domainSync() will take care of this in the node wrapper if the domain is * indeed dead. */ return; } else { + const char *state = NULL; switch (info.state) { - case VIR_DOMAIN_NOSTATE: - domain->set_state("nostate"); + state = "nostate"; break; case VIR_DOMAIN_RUNNING: - domain->set_state("running"); + state = "running"; break; case VIR_DOMAIN_BLOCKED: - domain->set_state("blocked"); + state = "blocked"; break; case VIR_DOMAIN_PAUSED: - domain->set_state("paused"); + state = "paused"; break; case VIR_DOMAIN_SHUTDOWN: - domain->set_state("shutdown"); + state = "shutdown"; break; case VIR_DOMAIN_SHUTOFF: - domain->set_state("shutoff"); + state = "shutoff"; break; case VIR_DOMAIN_CRASHED: - domain->set_state("crashed"); + state = "crashed"; break; } - domain->set_numVcpus(info.nrVirtCpu); - domain->set_maximumMemory(info.maxMem); - domain->set_memory(info.memory); - domain->set_cpuTime(info.cpuTime); + if (state) { + _data.setProperty("state", state); + } + _data.setProperty("numVcpus", info.nrVirtCpu); + _data.setProperty("maximumMemory", info.maxMem); + _data.setProperty("memory", (uint64_t)info.memory); + _data.setProperty("cpuTime", (uint64_t)info.cpuTime); } - ret = virDomainGetID(domain_ptr); + int id = virDomainGetID(_domain_ptr); // Just set it directly, if there's an error, -1 will be right. - domain->set_id(ret); + _data.setProperty("id", id); - if (ret > 0) { - domain->set_active("true"); - } else { - domain->set_active("false"); - } - domain->set_id(ret); + _data.setProperty("active", (id > 0) ? "true" : "false"); } -Manageable::status_t -DomainWrap::ManagementMethod(uint32_t methodId, Args& args, std::string &errstr) +bool +DomainWrap::handleMethod(qmf::AgentSession& session, qmf::AgentEvent& event) { int ret; - switch (methodId) { - case _qmf::Domain::METHOD_CREATE: - ret = virDomainCreate(domain_ptr); - update(); - if (ret < 0) { - errstr = FORMAT_ERR(conn, "Error creating new domain (virDomainCreate).", &ret); - return STATUS_USER + ret; - } - return STATUS_OK; - - case _qmf::Domain::METHOD_DESTROY: - ret = virDomainDestroy(domain_ptr); - update(); - if (ret < 0) { - errstr = FORMAT_ERR(conn, "Error destroying domain (virDomainDestroy).", &ret); - return STATUS_USER + ret; - } - - return STATUS_OK; - - case _qmf::Domain::METHOD_UNDEFINE: - ret = virDomainUndefine(domain_ptr); - - if (ret < 0) { - errstr = FORMAT_ERR(conn, "Error undefining domain (virDomainUndefine).", &ret); - return STATUS_USER + ret; - } - - /* We now wait for domainSync() to clean this up. */ - return STATUS_OK; - - case _qmf::Domain::METHOD_SUSPEND: - ret = virDomainSuspend(domain_ptr); - update(); - if (ret < 0) { - errstr = FORMAT_ERR(conn, "Error suspending domain (virDomainSuspend).", &ret); - return STATUS_USER + ret; - } - return STATUS_OK; - - case _qmf::Domain::METHOD_RESUME: - ret = virDomainResume(domain_ptr); - update(); - if (ret < 0) { - errstr = FORMAT_ERR(conn, "Error resuming domain (virDomainResume).", &ret); - return STATUS_USER + ret; - } - return STATUS_OK; - - case _qmf::Domain::METHOD_SAVE: - { - _qmf::ArgsDomainSave *io_args = (_qmf::ArgsDomainSave *) &args; - - ret = virDomainSave(domain_ptr, io_args->i_filename.c_str()); - if (ret < 0) { - errstr = FORMAT_ERR(conn, "Error saving domain (virDomainSave).", &ret); - return STATUS_USER + ret; - } - return STATUS_OK; - } - - case _qmf::Domain::METHOD_RESTORE: - { - _qmf::ArgsDomainRestore *io_args = (_qmf::ArgsDomainRestore *) &args; - - ret = virDomainRestore(conn, io_args->i_filename.c_str()); - update(); - if (ret < 0) { - errstr = FORMAT_ERR(conn, "Error restoring domain (virDomainRestore).", &ret); - return STATUS_USER + ret; - } - return STATUS_OK; - } - - case _qmf::Domain::METHOD_SHUTDOWN: - ret = virDomainShutdown(domain_ptr); - update(); - if (ret < 0) { - errstr = FORMAT_ERR(conn, "Error shutting down domain (virDomainShutdown).", &ret); - return STATUS_USER + ret; - } - return STATUS_OK; - - case _qmf::Domain::METHOD_REBOOT: - ret = virDomainReboot(domain_ptr, 0); - update(); - if (ret < 0) { - errstr = FORMAT_ERR(conn, "Error rebooting domain (virDomainReboot).", &ret); - return STATUS_USER + ret; - } - return STATUS_OK; - - case _qmf::Domain::METHOD_GETXMLDESC: - { - _qmf::ArgsDomainGetXMLDesc *io_args = (_qmf::ArgsDomainGetXMLDesc *) &args; - char *desc; - desc = virDomainGetXMLDesc(domain_ptr, VIR_DOMAIN_XML_SECURE | VIR_DOMAIN_XML_INACTIVE); - if (desc) { - io_args->o_description = desc; - } else { - errstr = FORMAT_ERR(conn, "Error getting XML description of domain(virDomainGetXMLDesc).", &ret); - return STATUS_USER + ret; - } - return STATUS_OK; - } - - case _qmf::Domain::METHOD_MIGRATE: - { - virConnectPtr dest_conn; - virDomainPtr rem_dom; - _qmf::ArgsDomainMigrate *io_args = (_qmf::ArgsDomainMigrate *) &args; - - // This is actually quite broken. Most setups won't allow unauthorized connection - // from one node to another directly like this. - dest_conn = virConnectOpen(io_args->i_destinationUri.c_str()); - if (!dest_conn) { - errstr = FORMAT_ERR(dest_conn, "Unable to connect to remote system for migration: virConnectOpen", &ret); - return STATUS_USER + ret; - } - - const char *new_dom_name = NULL; - if (io_args->i_newDomainName.size() > 0) { - new_dom_name = io_args->i_newDomainName.c_str(); - } - - const char *uri = NULL; - if (io_args->i_uri.size() > 0) { - uri = io_args->i_uri.c_str(); - } - - printf ("calling migrate, new_dom_name: %s, uri: %s, flags: %d (live is %d)\n", - new_dom_name ? new_dom_name : "NULL", - uri ? uri : "NULL", - io_args->i_flags, - VIR_MIGRATE_LIVE); - - rem_dom = virDomainMigrate(domain_ptr, dest_conn, io_args->i_flags, - new_dom_name, - uri, - io_args->i_bandwidth); - - virConnectClose(dest_conn); - - if (!rem_dom) { - errstr = FORMAT_ERR(conn, "virDomainMigrate", &ret); - return STATUS_USER + ret; - } - virDomainFree(rem_dom); - - return STATUS_OK; - } + if (*this != event.getDataAddr()) { + return false; + } + + const std::string& methodName(event.getMethodName()); + qpid::types::Variant::Map args(event.getArguments()); + + if (methodName == "create") { + int ret = virDomainCreate(_domain_ptr); + update(); + if (ret < 0) { + std::string err = FORMAT_ERR(_conn, "Error creating new domain (virDomainCreate).", &ret); + raiseException(session, event, err, STATUS_USER + ret); + } else { + session.methodSuccess(event); + } + return true; + } + + else if (methodName == "destroy") { + int ret = virDomainDestroy(_domain_ptr); + update(); + if (ret < 0) { + std::string err = FORMAT_ERR(_conn, "Error destroying domain (virDomainDestroy).", &ret); + raiseException(session, event, err, STATUS_USER + ret); + } else { + session.methodSuccess(event); + } + return true; + } + + else if (methodName == "undefine") { + int ret = virDomainUndefine(_domain_ptr); + if (ret < 0) { + std::string err = FORMAT_ERR(_conn, "Error undefining domain (virDomainUndefine).", &ret); + raiseException(session, event, err, STATUS_USER + ret); + } else { + session.methodSuccess(event); + } + /* We now wait for domainSync() to clean this up. */ + return true; } - return STATUS_NOT_IMPLEMENTED; + else if (methodName == "suspend") { + int ret = virDomainSuspend(_domain_ptr); + update(); + if (ret < 0) { + std::string err = FORMAT_ERR(_conn, "Error suspending domain (virDomainSuspend).", &ret); + raiseException(session, event, err, STATUS_USER + ret); + } else { + session.methodSuccess(event); + } + return true; + } + + else if (methodName == "resume") { + int ret = virDomainResume(_domain_ptr); + update(); + if (ret < 0) { + std::string err = FORMAT_ERR(_conn, "Error resuming domain (virDomainResume).", &ret); + raiseException(session, event, err, STATUS_USER + ret); + } else { + session.methodSuccess(event); + } + return true; + } + + else if (methodName == "save") { + const std::string filename = args["filename"].asString(); + int ret = virDomainSave(_domain_ptr, filename.c_str()); + if (ret < 0) { + std::string err = FORMAT_ERR(_conn, "Error saving domain (virDomainSave).", &ret); + raiseException(session, event, err, STATUS_USER + ret); + } else { + session.methodSuccess(event); + } + return true; + } + + else if (methodName == "restore") { + const std::string filename = args["filename"].asString(); + int ret = virDomainRestore(_conn, filename.c_str()); + update(); + if (ret < 0) { + std::string err = FORMAT_ERR(_conn, "Error saving domain (virDomainSave).", &ret); + raiseException(session, event, err, STATUS_USER + ret); + } else { + session.methodSuccess(event); + } + return true; + } + + else if (methodName == "shutdown") { + int ret = virDomainShutdown(_domain_ptr); + update(); + if (ret < 0) { + std::string err = FORMAT_ERR(_conn, "Error shutting down domain (virDomainShutdown).", &ret); + raiseException(session, event, err, STATUS_USER + ret); + } else { + session.methodSuccess(event); + } + return true; + } + + else if (methodName == "reboot") { + int ret = virDomainReboot(_domain_ptr, 0); + update(); + if (ret < 0) { + std::string err = FORMAT_ERR(_conn, "Error rebooting domain (virDomainReboot).", &ret); + raiseException(session, event, err, STATUS_USER + ret); + } else { + session.methodSuccess(event); + } + return true; + } + + else if (methodName == "getXMLDesc") { + const char *desc = virDomainGetXMLDesc(_domain_ptr, + VIR_DOMAIN_XML_SECURE | VIR_DOMAIN_XML_INACTIVE); + if (!desc) { + std::string err = FORMAT_ERR(_conn, "Error rebooting domain (virDomainReboot).", &ret); + raiseException(session, event, err, STATUS_USER + ret); + } else { + event.addReturnArgument("description", desc); + session.methodSuccess(event); + } + return true; + } + + else if (methodName == "migrate") { + if (migrate(session, event)) { + session.methodSuccess(event); + } + return true; + } + + raiseException(session, event, + ERROR_UNKNOWN_METHOD, STATUS_UNKNOWN_METHOD); + return true; } +bool +DomainWrap::migrate(qmf::AgentSession& session, qmf::AgentEvent& event) +{ + virConnectPtr dest_conn; + virDomainPtr rem_dom; + qpid::types::Variant::Map args(event.getArguments()); + int ret; + + // This is actually quite broken. Most setups won't allow unauthorized + // connection from one node to another directly like this. + dest_conn = virConnectOpen(args["destinationUri"].asString().c_str()); + if (!dest_conn) { + std::string err = FORMAT_ERR(dest_conn, "Unable to connect to remote system for migration: virConnectOpen", &ret); + raiseException(session, event, err, STATUS_USER + ret); + return false; + } + + const std::string newDomainName_arg(args["newDomainName"]); + const char *new_dom_name = NULL; + if (newDomainName_arg.size() > 0) { + new_dom_name = newDomainName_arg.c_str(); + } + + const std::string uri_arg(args["uri"]); + const char *uri = NULL; + if (uri_arg.size() > 0) { + uri = uri_arg.c_str(); + } + + uint32_t flags(args["flags"]); + uint32_t bandwidth(args["bandwidth"]); + + printf ("calling migrate, new_dom_name: %s, uri: %s, flags: %d (live is %d)\n", + new_dom_name ? new_dom_name : "NULL", + uri ? uri : "NULL", + flags, + VIR_MIGRATE_LIVE); + + rem_dom = virDomainMigrate(_domain_ptr, dest_conn, flags, + new_dom_name, + uri, + bandwidth); + + virConnectClose(dest_conn); + + if (!rem_dom) { + std::string err = FORMAT_ERR(_conn, "virDomainMigrate", &ret); + raiseException(session, event, err, STATUS_USER + ret); + return false; + } + virDomainFree(rem_dom); + + return true; +} diff --git a/src/DomainWrap.h b/src/DomainWrap.h index 27822bd..fc39a5f 100644 --- a/src/DomainWrap.h +++ b/src/DomainWrap.h @@ -1,10 +1,7 @@ -#include <qpid/management/Manageable.h> -#include <qpid/management/ManagementObject.h> -#include <qpid/agent/ManagementAgent.h> -#include <qpid/sys/Mutex.h> +#ifndef DOMAIN_WRAP_H +#define DOMAIN_WRAP_H -#include "Package.h" -#include "Domain.h" +#include "NodeWrap.h" #include <unistd.h> #include <cstdlib> @@ -15,38 +12,30 @@ #include <libvirt/libvirt.h> #include <libvirt/virterror.h> -using namespace qpid::management; -using namespace qpid::sys; -using namespace std; -using qpid::management::ManagementObject; -using qpid::management::Manageable; -using qpid::management::Args; -using qpid::sys::Mutex; -class DomainWrap : public Manageable +class DomainWrap: + PackageOwner<NodeWrap::PackageDefinition>, + public ManagedObject { - ManagementAgent *agent; - qmf::com::redhat::libvirt::Domain *domain; + virDomainPtr _domain_ptr; + virConnectPtr _conn; - virDomainPtr domain_ptr; - virConnectPtr conn; + std::string _domain_name; + std::string _domain_uuid; public: - - std::string domain_name; - std::string domain_uuid; - - DomainWrap(ManagementAgent *agent, NodeWrap *parent, virDomainPtr domain_ptr, - virConnectPtr connect); + DomainWrap(NodeWrap *parent, + virDomainPtr domain_ptr, virConnectPtr conn); ~DomainWrap(); - ManagementObject* GetManagementObject(void) const - { - return domain; - } - + std::string& name(void) { return _domain_name; } + std::string& uuid(void) { return _domain_uuid; } void update(); - status_t ManagementMethod (uint32_t methodId, Args& args, std::string &errstr); + bool handleMethod(qmf::AgentSession& session, qmf::AgentEvent& event); + +private: + bool migrate(qmf::AgentSession& session, qmf::AgentEvent& event); }; +#endif diff --git a/src/Error.h b/src/Error.h index 388b41a..5bd7397 100644 --- a/src/Error.h +++ b/src/Error.h @@ -1,11 +1,19 @@ +#ifndef ERROR_H +#define ERROR_H + +#include <libvirt/libvirt.h> +#include <libvirt/virterror.h> + #define REPORT_ERR(conn,msg) reportError(conn, msg, __func__, __LINE__, __FILE__) #define FORMAT_ERR(conn,msg,err_ret) formatError(conn, msg, err_ret, __func__, __LINE__, __FILE__) + void reportError(virConnectPtr conn, const char *msg, const char *function, int line, const char *file); std::string formatError(virConnectPtr conn, const char *msg, int *err_ret, const char *function, int line, const char *file); +#endif diff --git a/src/Exception.cpp b/src/Exception.cpp new file mode 100644 index 0000000..105df27 --- /dev/null +++ b/src/Exception.cpp @@ -0,0 +1,38 @@ +#include "Exception.h" +#include "Error.h" +#include <qmf/Schema.h> +#include <qmf/SchemaProperty.h> +#include <qmf/Data.h> + + +#define ERROR_TEXT "error_text" +#define ERROR_CODE "error_code" + + +static qmf::Schema errorSchema(qmf::SCHEMA_TYPE_DATA, + "com.redhat.libvirt", "error"); + + +void +initErrorSchema(qmf::AgentSession& session) +{ + qmf::SchemaProperty status_text(ERROR_TEXT, qmf::SCHEMA_DATA_STRING); + qmf::SchemaProperty status_code(ERROR_CODE, qmf::SCHEMA_DATA_INT); + + errorSchema.addProperty(status_text); + errorSchema.addProperty(status_code); + + session.registerSchema(errorSchema); +} + +void +raiseException(qmf::AgentSession& session, + qmf::AgentEvent& event, + const std::string& error_text, + unsigned int error_code) +{ + qmf::Data response(errorSchema); + response.setProperty(ERROR_TEXT, error_text); + response.setProperty(ERROR_CODE, error_code); + session.raiseException(event, response); +} diff --git a/src/Exception.h b/src/Exception.h new file mode 100644 index 0000000..efbd327 --- /dev/null +++ b/src/Exception.h @@ -0,0 +1,30 @@ +#ifndef EXCEPTION_H +#define EXCEPTION_H + +#include <qmf/AgentSession.h> +#include <qmf/AgentEvent.h> +#include <string> + + +#define STATUS_OK 0 +#define STATUS_UNKNOWN_OBJECT 1 +#define STATUS_UNKNOWN_METHOD 2 +#define STATUS_NOT_IMPLEMENTED 3 +#define STATUS_EXCEPTION 7 +#define STATUS_USER 0x10000 + +#define ERROR_UNKNOWN_OBJECT "Unknown Object" +#define ERROR_UNKNOWN_METHOD "Unknown Method" +#define ERROR_NOT_IMPLEMENTED "Not implemented" + + +void +initErrorSchema(qmf::AgentSession& session); + +void +raiseException(qmf::AgentSession& session, + qmf::AgentEvent& event, + const std::string& error_text, + unsigned int error_code); + +#endif diff --git a/src/Makefile.am b/src/Makefile.am index 1b4ef23..4bb27b2 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -5,37 +5,22 @@ INCLUDES = -I$(top_srcdir)/src/qmf/com/redhat/libvirt $(XML_CFLAGS) sbin_PROGRAMS = libvirt-qpid generated_file_list = \ - qmf/com/redhat/libvirt/Domain.cpp\ - qmf/com/redhat/libvirt/Node.cpp\ - qmf/com/redhat/libvirt/Package.cpp\ - qmf/com/redhat/libvirt/Pool.cpp\ - qmf/com/redhat/libvirt/Volume.cpp\ - qmf/com/redhat/libvirt/ArgsDomainGetXMLDesc.h\ - qmf/com/redhat/libvirt/ArgsDomainMigrate.h\ - qmf/com/redhat/libvirt/ArgsDomainRestore.h\ - qmf/com/redhat/libvirt/ArgsDomainSave.h\ - qmf/com/redhat/libvirt/ArgsNodeDomainDefineXML.h\ - qmf/com/redhat/libvirt/ArgsNodeStoragePoolCreateXML.h\ - qmf/com/redhat/libvirt/ArgsNodeStoragePoolDefineXML.h\ - qmf/com/redhat/libvirt/ArgsPoolCreateVolumeXML.h\ - qmf/com/redhat/libvirt/ArgsPoolGetXMLDesc.h\ - qmf/com/redhat/libvirt/ArgsVolumeGetXMLDesc.h\ - qmf/com/redhat/libvirt/Domain.h\ - qmf/com/redhat/libvirt/Node.h\ - qmf/com/redhat/libvirt/Package.h\ - qmf/com/redhat/libvirt/Pool.h\ - qmf/com/redhat/libvirt/Volume.h + qmf/com/redhat/libvirt/QmfPackage.cpp\ + qmf/com/redhat/libvirt/QmfPackage.h nodist_libvirt_qpid_SOURCES = $(generated_file_list) libvirt_qpid_SOURCES = \ DomainWrap.cpp \ Error.cpp \ + Exception.cpp \ NodeWrap.cpp \ PoolWrap.cpp \ VolumeWrap.cpp \ + ManagedObject.h \ DomainWrap.h \ Error.h \ + Exception.h \ NodeWrap.h \ PoolWrap.h \ VolumeWrap.h @@ -43,12 +28,12 @@ libvirt_qpid_SOURCES = \ $(generated_file_list): .libvirt-schema.xml.tstamp .libvirt-schema.xml.tstamp: libvirt-schema.xml - qmf-gen -o ./qmf $< && touch $@ || rm -f $@ + qmf-gen -2 -o ./qmf $< && touch $@ || rm -f $@ BUILT_SOURCES = $(generated_file_list) CLEANFILES = $(generated_file_list) .libvirt-schema.xml.tstamp -libvirt_qpid_LDADD = -lqmf -lqpidtypes -lqpidcommon -lqpidclient -lvirt $(XML_LIBS) +libvirt_qpid_LDADD = -lqmf2 -lqpidtypes -lqpidcommon -lqpidmessaging -lvirt $(XML_LIBS) dist_pkgdata_DATA = libvirt-schema.xml diff --git a/src/ManagedObject.h b/src/ManagedObject.h new file mode 100644 index 0000000..b372f24 --- /dev/null +++ b/src/ManagedObject.h @@ -0,0 +1,78 @@ +#ifndef MANAGED_OBJECT_H +#define MANAGED_OBJECT_H + +#include <qmf/AgentEvent.h> +#include <qmf/AgentSession.h> +#include <qmf/Data.h> +#include <qmf/DataAddr.h> + +#include <assert.h> + + +template <class T> +class PackageOwner +{ + PackageOwner<T> *_parent; + +public: + typedef T PackageDefinition; + +protected: + PackageOwner<T>(void): _parent(NULL) { } + PackageOwner<T>(PackageOwner<T> *parent): _parent(parent) { } + virtual ~PackageOwner<T>() { } + + virtual PackageDefinition& package(void) { + assert(_parent != NULL); + return _parent->package(); + } + + virtual void addData(qmf::Data& data) { + assert(_parent != NULL); + _parent->addData(data); + } + + virtual void delData(qmf::Data& data) { + assert(_parent != NULL); + _parent->delData(data); + } +}; + + +class ManagedObject +{ +public: + qpid::types::Variant objectID(void) { + assert(_data.hasAddr()); + return _data.getAddr().asMap(); + } + + virtual bool handleMethod(qmf::AgentSession& session, + qmf::AgentEvent& event) = 0; + +protected: + ManagedObject(qmf::Schema& schema): _data(schema) { } + virtual ~ManagedObject() { } + + bool operator==(const qmf::DataAddr& addr) { + if (!_data.hasAddr()) { + return false; + } + + // Due to a bug (#QPID3344) in current versions of libqmf2, the first + // operand to qmf::DataAddr::operator==() must not be const, otherwise + // a bogus value is returned. + qmf::DataAddr& mutableAddr = const_cast<qmf::DataAddr&>(_data.getAddr()); + + return mutableAddr == addr; + } + + bool operator!=(const qmf::DataAddr& addr) { + return !(*this == addr); + } + + qmf::Data _data; +}; + +#endif + diff --git a/src/NodeWrap.cpp b/src/NodeWrap.cpp index b663235..d65db11 100644 --- a/src/NodeWrap.cpp +++ b/src/NodeWrap.cpp @@ -6,20 +6,19 @@ #include <getopt.h> #include <syslog.h> #include <signal.h> +#include <cstdio> #include "NodeWrap.h" #include "DomainWrap.h" #include "PoolWrap.h" #include "Error.h" +#include "Exception.h" -#include "ArgsNodeDomainDefineXML.h" -#include "ArgsNodeStoragePoolCreateXML.h" -#include "ArgsNodeStoragePoolDefineXML.h" -#include "ArgsNodeFindStoragePoolSources.h" -namespace _qmf = qmf::com::redhat::libvirt; - -NodeWrap::NodeWrap(ManagementAgent* _agent, string _name) : name(_name), agent(_agent) +NodeWrap::NodeWrap(qmf::AgentSession& agent_session, PackageDefinition& package): + ManagedObject(package.data_Node), + _session(agent_session), + _package(package) { virNodeInfo info; char *hostname; @@ -36,33 +35,33 @@ NodeWrap::NodeWrap(ManagementAgent* _agent, string _name) : name(_name), agent(_ unsigned int minor; unsigned int rel; - conn = virConnectOpen(NULL); - if (!conn) { - REPORT_ERR(conn, "virConnectOpen"); + _conn = virConnectOpen(NULL); + if (!_conn) { + REPORT_ERR(_conn, "virConnectOpen"); throw -1; } - hostname = virConnectGetHostname(conn); + hostname = virConnectGetHostname(_conn); if (hostname == NULL) { - REPORT_ERR(conn, "virConnectGetHostname"); + REPORT_ERR(_conn, "virConnectGetHostname"); throw -1; } - hv_type = virConnectGetType(conn); + hv_type = virConnectGetType(_conn); if (hv_type == NULL) { - REPORT_ERR(conn, "virConnectGetType"); + REPORT_ERR(_conn, "virConnectGetType"); throw -1; } - uri = virConnectGetURI(conn); + uri = virConnectGetURI(_conn); if (uri == NULL) { - REPORT_ERR(conn, "virConnectGetURI"); + REPORT_ERR(_conn, "virConnectGetURI"); throw -1; } ret = virGetVersion(&libvirt_v, hv_type, &api_v); if (ret < 0) { - REPORT_ERR(conn, "virGetVersion"); + REPORT_ERR(_conn, "virGetVersion"); } else { major = libvirt_v / 1000000; libvirt_v %= 1000000; @@ -77,9 +76,9 @@ NodeWrap::NodeWrap(ManagementAgent* _agent, string _name) : name(_name), agent(_ snprintf(api_version, sizeof(api_version), "%d.%d.%d", major, minor, rel); } - ret = virConnectGetVersion(conn, &hv_v); + ret = virConnectGetVersion(_conn, &hv_v); if (ret < 0) { - REPORT_ERR(conn, "virConnectGetVersion"); + REPORT_ERR(_conn, "virConnectGetVersion"); } else { major = hv_v / 1000000; hv_v %= 1000000; @@ -88,48 +87,64 @@ NodeWrap::NodeWrap(ManagementAgent* _agent, string _name) : name(_name), agent(_ snprintf(hv_version, sizeof(hv_version), "%d.%d.%d", major, minor, rel); } - ret = virNodeGetInfo(conn, &info); + ret = virNodeGetInfo(_conn, &info); if (ret < 0) { - REPORT_ERR(conn, "virNodeGetInfo"); + REPORT_ERR(_conn, "virNodeGetInfo"); memset((void *) &info, sizeof(info), 1); } - mgmtObject = new _qmf::Node(agent, this, hostname, uri, libvirt_version, api_version, hv_version, hv_type, - info.model, info.memory, info.cpus, info.mhz, info.nodes, info.sockets, - info.cores, info.threads); - agent->addObject(mgmtObject); + + _data.setProperty("hostname", hostname); + _data.setProperty("uri", uri); + _data.setProperty("libvirtVersion", libvirt_version); + _data.setProperty("apiVersion", api_version); + _data.setProperty("hypervisorVersion", hv_version); + _data.setProperty("hypervisorType", hv_type); + + _data.setProperty("model", info.model); + _data.setProperty("memory", info.memory); + _data.setProperty("cpus", info.cpus); + _data.setProperty("mhz", info.mhz); + _data.setProperty("nodes", info.nodes); + _data.setProperty("sockets", info.sockets); + _data.setProperty("cores", info.cores); + _data.setProperty("threads", info.threads); + + addData(_data); } NodeWrap::~NodeWrap() { /* Go through our list of pools and destroy them all! MOOOHAHAHA */ - for (std::vector<PoolWrap*>::iterator iter = pools.begin(); iter != pools.end();) { - delete(*iter); - iter = pools.erase(iter); + PoolList::iterator iter_p = _pools.begin(); + while (iter_p != _pools.end()) { + delete(*iter_p); + iter_p = _pools.erase(iter_p); } /* Same for domains.. */ - for (std::vector<DomainWrap*>::iterator iter = domains.begin(); iter != domains.end();) { - delete (*iter); - iter = domains.erase(iter); + DomainList::iterator iter_d = _domains.begin(); + while (iter_d != _domains.end()) { + delete (*iter_d); + iter_d = _domains.erase(iter_d); } - mgmtObject->resourceDestroy(); + delData(_data); } -void NodeWrap::syncDomains() +void NodeWrap::syncDomains(void) { /* Sync up with domains that are defined but not active. */ - int maxname = virConnectNumOfDefinedDomains(conn); + int maxname = virConnectNumOfDefinedDomains(_conn); if (maxname < 0) { - REPORT_ERR(conn, "virConnectNumOfDefinedDomains"); + REPORT_ERR(_conn, "virConnectNumOfDefinedDomains"); return; } else { char **dnames; dnames = (char **) malloc(sizeof(char *) * maxname); - if ((maxname = virConnectListDefinedDomains(conn, dnames, maxname)) < 0) { - REPORT_ERR(conn, "virConnectListDefinedDomains"); + if ((maxname = virConnectListDefinedDomains(_conn, dnames, maxname)) < 0) { + REPORT_ERR(_conn, "virConnectListDefinedDomains"); free(dnames); return; } @@ -139,9 +154,9 @@ void NodeWrap::syncDomains() virDomainPtr domain_ptr; bool found = false; - for (std::vector<DomainWrap*>::iterator iter = domains.begin(); - iter != domains.end(); iter++) { - if ((*iter)->domain_name == dnames[i]) { + for (DomainList::iterator iter = _domains.begin(); + iter != _domains.end(); iter++) { + if ((*iter)->name() == dnames[i]) { found = true; break; } @@ -151,18 +166,18 @@ void NodeWrap::syncDomains() continue; } - domain_ptr = virDomainLookupByName(conn, dnames[i]); + domain_ptr = virDomainLookupByName(_conn, dnames[i]); if (!domain_ptr) { - REPORT_ERR(conn, "virDomainLookupByName"); + REPORT_ERR(_conn, "virDomainLookupByName"); } else { DomainWrap *domain; try { - domain = new DomainWrap(agent, this, domain_ptr, conn); + domain = new DomainWrap(this, domain_ptr, _conn); printf("Created new domain: %s, ptr is %p\n", dnames[i], domain_ptr); - domains.push_back(domain); + _domains.push_back(domain); } catch (int i) { - printf ("Error constructing domain\n"); - REPORT_ERR(conn, "constructing domain."); + printf("Error constructing domain\n"); + REPORT_ERR(_conn, "constructing domain."); delete domain; } } @@ -175,15 +190,15 @@ void NodeWrap::syncDomains() } /* Go through all the active domains */ - int maxids = virConnectNumOfDomains(conn); + int maxids = virConnectNumOfDomains(_conn); if (maxids < 0) { - REPORT_ERR(conn, "virConnectNumOfDomains"); + REPORT_ERR(_conn, "virConnectNumOfDomains"); return; } else { int *ids; ids = (int *) malloc(sizeof(int *) * maxids); - if ((maxids = virConnectListDomains(conn, ids, maxids)) < 0) { + if ((maxids = virConnectListDomains(_conn, ids, maxids)) < 0) { printf("Error getting list of defined domains\n"); return; } @@ -192,21 +207,21 @@ void NodeWrap::syncDomains() virDomainPtr domain_ptr; char dom_uuid[VIR_UUID_STRING_BUFLEN]; - domain_ptr = virDomainLookupByID(conn, ids[i]); + domain_ptr = virDomainLookupByID(_conn, ids[i]); if (!domain_ptr) { - REPORT_ERR(conn, "virDomainLookupByID"); + REPORT_ERR(_conn, "virDomainLookupByID"); continue; } if (virDomainGetUUIDString(domain_ptr, dom_uuid) < 0) { - REPORT_ERR(conn, "virDomainGetUUIDString"); + REPORT_ERR(_conn, "virDomainGetUUIDString"); continue; } bool found = false; - for (std::vector<DomainWrap*>::iterator iter = domains.begin(); - iter != domains.end(); iter++) { - if (strcmp((*iter)->domain_uuid.c_str(), dom_uuid) == 0) { + for (DomainList::iterator iter = _domains.begin(); + iter != _domains.end(); iter++) { + if (strcmp((*iter)->uuid().c_str(), dom_uuid) == 0) { found = true; break; } @@ -219,12 +234,12 @@ void NodeWrap::syncDomains() DomainWrap *domain; try { - domain = new DomainWrap(agent, this, domain_ptr, conn); + domain = new DomainWrap(this, domain_ptr, _conn); printf("Created new domain: %d, ptr is %p\n", ids[i], domain_ptr); - domains.push_back(domain); + _domains.push_back(domain); } catch (int i) { - printf ("Error constructing domain\n"); - REPORT_ERR(conn, "constructing domain."); + printf("Error constructing domain\n"); + REPORT_ERR(_conn, "constructing domain."); delete domain; } } @@ -233,14 +248,14 @@ void NodeWrap::syncDomains() } /* Go through our list of domains and ensure that they still exist. */ - for (std::vector<DomainWrap*>::iterator iter = domains.begin(); iter != domains.end();) { - - printf("verifying domain %s\n", (*iter)->domain_name.c_str()); - virDomainPtr ptr = virDomainLookupByUUIDString(conn, (*iter)->domain_uuid.c_str()); + DomainList::iterator iter = _domains.begin(); + while (iter != _domains.end()) { + printf("verifying domain %s\n", (*iter)->name().c_str()); + virDomainPtr ptr = virDomainLookupByUUIDString(_conn, (*iter)->uuid().c_str()); if (ptr == NULL) { - REPORT_ERR(conn, "virDomainLookupByUUIDString"); + REPORT_ERR(_conn, "virDomainLookupByUUIDString"); delete (*iter); - iter = domains.erase(iter); + iter = _domains.erase(iter); } else { virDomainFree(ptr); iter++; @@ -253,9 +268,9 @@ void NodeWrap::checkPool(char *pool_name) virStoragePoolPtr pool_ptr; bool found = false; - for (std::vector<PoolWrap*>::iterator iter = pools.begin(); - iter != pools.end(); iter++) { - if ((*iter)->pool_name == pool_name) { + for (PoolList::iterator iter = _pools.begin(); + iter != _pools.end(); iter++) { + if ((*iter)->name() == pool_name) { found = true; break; } @@ -265,38 +280,37 @@ void NodeWrap::checkPool(char *pool_name) return; } - pool_ptr = virStoragePoolLookupByName(conn, pool_name); + pool_ptr = virStoragePoolLookupByName(_conn, pool_name); if (!pool_ptr) { - REPORT_ERR(conn, "virStoragePoolLookupByName"); + REPORT_ERR(_conn, "virStoragePoolLookupByName"); } else { printf("Creating new pool: %s, ptr is %p\n", pool_name, pool_ptr); PoolWrap *pool; try { - pool = new PoolWrap(agent, this, pool_ptr, conn); + pool = new PoolWrap(this, pool_ptr, _conn); printf("Created new pool: %s, ptr is %p\n", pool_name, pool_ptr); - pools.push_back(pool); + _pools.push_back(pool); } catch (int i) { - printf ("Error constructing pool\n"); - REPORT_ERR(conn, "constructing pool."); + printf("Error constructing pool\n"); + REPORT_ERR(_conn, "constructing pool."); delete pool; } } } -void NodeWrap::syncPools() +void NodeWrap::syncPools(void) { int maxname; - maxname = virConnectNumOfStoragePools(conn); + maxname = virConnectNumOfStoragePools(_conn); if (maxname < 0) { - REPORT_ERR(conn, "virConnectNumOfStroagePools"); + REPORT_ERR(_conn, "virConnectNumOfStoragePools"); return; } else { - char **names; - names = (char **) malloc(sizeof(char *) * maxname); + char *names[maxname]; - if ((maxname = virConnectListStoragePools(conn, names, maxname)) < 0) { - REPORT_ERR(conn, "virConnectListStoragePools"); + if ((maxname = virConnectListStoragePools(_conn, names, maxname)) < 0) { + REPORT_ERR(_conn, "virConnectListStoragePools"); return; } @@ -304,19 +318,17 @@ void NodeWrap::syncPools() checkPool(names[i]); free(names[i]); } - free(names); } - maxname = virConnectNumOfDefinedStoragePools(conn); + maxname = virConnectNumOfDefinedStoragePools(_conn); if (maxname < 0) { - REPORT_ERR(conn, "virConnectNumOfDefinedStoragePools"); + REPORT_ERR(_conn, "virConnectNumOfDefinedStoragePools"); return; } else { - char **names; - names = (char **) malloc(sizeof(char *) * maxname); + char *names[maxname]; - if ((maxname = virConnectListDefinedStoragePools(conn, names, maxname)) < 0) { - REPORT_ERR(conn, "virConnectListDefinedStoragePools"); + if ((maxname = virConnectListDefinedStoragePools(_conn, names, maxname)) < 0) { + REPORT_ERR(_conn, "virConnectListDefinedStoragePools"); return; } @@ -324,19 +336,17 @@ void NodeWrap::syncPools() checkPool(names[i]); free(names[i]); } - - free(names); } /* Go through our list of pools and ensure that they still exist. */ - for (std::vector<PoolWrap*>::iterator iter = pools.begin(); iter != pools.end();) { - - printf ("Verifying pool %s\n", (*iter)->pool_name.c_str()); - virStoragePoolPtr ptr = virStoragePoolLookupByUUIDString(conn, (*iter)->pool_uuid.c_str()); + PoolList::iterator iter = _pools.begin(); + while (iter != _pools.end()) { + printf("Verifying pool %s\n", (*iter)->name().c_str()); + virStoragePoolPtr ptr = virStoragePoolLookupByUUIDString(_conn, (*iter)->uuid().c_str()); if (ptr == NULL) { - printf("Destroying pool %s\n", (*iter)->pool_name.c_str()); + printf("Destroying pool %s\n", (*iter)->name().c_str()); delete(*iter); - iter = pools.erase(iter); + iter = _pools.erase(iter); } else { virStoragePoolFree(ptr); iter++; @@ -344,7 +354,6 @@ void NodeWrap::syncPools() } } - void NodeWrap::doLoop() { fd_set fds; @@ -354,12 +363,10 @@ void NodeWrap::doLoop() /* Go through all domains and call update() for each, letting them update * information and statistics. */ while (1) { - int read_fd = agent->getSignalFd(); - // We're using this to check to see if our connection is still good. // I don't see any reason this call should fail unless there is some // connection problem.. - int maxname = virConnectNumOfDefinedDomains(conn); + int maxname = virConnectNumOfDefinedDomains(_conn); if (maxname < 0) { return; } @@ -367,151 +374,200 @@ void NodeWrap::doLoop() syncDomains(); syncPools(); - for (std::vector<DomainWrap*>::iterator iter = domains.begin(); - iter != domains.end(); iter++) { + for (DomainList::iterator iter = _domains.begin(); + iter != _domains.end(); iter++) { (*iter)->update(); } - for (std::vector<PoolWrap*>::iterator iter = pools.begin(); - iter != pools.end(); iter++) { + for (PoolList::iterator iter = _pools.begin(); + iter != _pools.end(); iter++) { (*iter)->update(); } - /* Poll agent fd. If any methods are being made this FD will be ready for reading. */ - FD_ZERO(&fds); - FD_SET(read_fd, &fds); + qmf::AgentEvent event; + if (_session.nextEvent(event, qpid::messaging::Duration(3000))) { + if (event.getType() == qmf::AGENT_METHOD) { + bool handled = handleMethod(_session, event); + if (!handled) { + raiseException(_session, event, + ERROR_UNKNOWN_OBJECT, STATUS_UNKNOWN_OBJECT); + } + } + } - /* Wait up to three seconds. */ - tv.tv_sec = 3; - tv.tv_usec = 0; + } +} - retval = select(read_fd + 1, &fds, NULL, NULL, &tv); - if (retval < 0) { - fprintf(stderr, "Error in select loop: %s\n", strerror(errno)); - continue; - } +bool +NodeWrap::domainDefineXML(qmf::AgentSession& session, + qmf::AgentEvent& event) +{ + const std::string xmlDesc(event.getArguments()["xmlDesc"].asString()); + int ret; + + virDomainPtr domain_ptr = virDomainDefineXML(_conn, xmlDesc.c_str()); + if (!domain_ptr) { + std::string err = FORMAT_ERR(_conn, "Error creating domain using xml description (virDomainDefineXML).", &ret); + raiseException(session, event, err, STATUS_USER + ret); + return false; + } - if (retval > 0) { - /* This implements any pending methods. */ - agent->pollCallbacks(); + // Now we have to check to see if this domain is actually new or not, + // because it's possible that one already exists with this name/description + // and we just replaced it... *ugh* + DomainList::iterator iter = _domains.begin(); + while (iter != _domains.end()) { + if ((*iter)->name() == virDomainGetName(domain_ptr)) { + // We're just replacing an existing domain, however I'm pretty sure + // the old domain pointer becomes invalid at this point, so we + // should destroy the old domain reference. The other option would + // be to replace it and keep the object valid... not sure which is + // better. + printf("Old domain already exists, removing it in favor of new object."); + delete(*iter); + iter = _domains.erase(iter); + } else { + iter++; } + } + DomainWrap *domain; + try { + domain = new DomainWrap(this, domain_ptr, _conn); + _domains.push_back(domain); + event.addReturnArgument("domain", domain->objectID()); + } catch (int i) { + delete domain; + std::string err = FORMAT_ERR(_conn, "Error constructing domain object in virDomainDefineXML.", &ret); + raiseException(session, event, err, STATUS_USER + ret); + return false; } + + return true; } -Manageable::status_t -NodeWrap::ManagementMethod(uint32_t methodId, Args& args, std::string &errstr) +bool +NodeWrap::storagePoolDefineXML(qmf::AgentSession& session, + qmf::AgentEvent& event) { - virDomainPtr domain_ptr; - cout << "Method Received: " << methodId << endl; + const std::string xmlDesc(event.getArguments()["xmlDesc"].asString()); + virStoragePoolPtr pool_ptr; int ret; - switch (methodId) { - case _qmf::Node::METHOD_DOMAINDEFINEXML: - { - _qmf::ArgsNodeDomainDefineXML *io_args = (_qmf::ArgsNodeDomainDefineXML *) &args; - domain_ptr = virDomainDefineXML(conn, io_args->i_xmlDesc.c_str()); - if (!domain_ptr) { - errstr = FORMAT_ERR(conn, "Error creating domain using xml description (virDomainDefineXML).", &ret); - return STATUS_USER + ret; - } else { - // Now we have to check to see if this domain is actually new or not, because it's possible that - // one already exists with this name/description and we just replaced it.. *ugh* - for (std::vector<DomainWrap*>::iterator iter = domains.begin(); iter != domains.end();) { - if (strcmp((*iter)->domain_name.c_str(), virDomainGetName(domain_ptr)) == 0) { - // We're just replacing an existing domain, however I'm pretty sure the - // old domain pointer becomes invalid at this point, so we should destroy - // the old domain reference. The other option would be to replace it and - // keep the object valid.. not sure which is better. - printf("Old domain already exists, removing it in favor of new object."); - delete(*iter); - iter = domains.erase(iter); - } else { - iter++; - } - } + pool_ptr = virStoragePoolDefineXML(_conn, xmlDesc.c_str(), 0); + if (pool_ptr == NULL) { + std::string err = FORMAT_ERR(_conn, "Error defining storage pool using xml description (virStoragePoolDefineXML).", &ret); + raiseException(session, event, err, STATUS_USER + ret); + return false; + } - DomainWrap *domain; - try { - domain = new DomainWrap(agent, this, domain_ptr, conn); - domains.push_back(domain); - io_args->o_domain = domain->GetManagementObject()->getObjectId(); - } catch (int i) { - delete domain; - errstr = FORMAT_ERR(conn, "Error constructing domain object in virDomainDefineXML.", &ret); - return STATUS_USER + i; - } + PoolWrap *pool; + try { + pool = new PoolWrap(this, pool_ptr, _conn); + _pools.push_back(pool); + event.addReturnArgument("pool", pool->objectID()); + } catch (int i) { + delete pool; + std::string err = FORMAT_ERR(_conn, "Error constructing pool object in virStoragePoolDefineXML.", &ret); + raiseException(session, event, err, STATUS_USER + ret); + return false; + } - return STATUS_OK; - } - } + return true; +} + +bool +NodeWrap::storagePoolCreateXML(qmf::AgentSession& session, + qmf::AgentEvent& event) +{ + const std::string xmlDesc(event.getArguments()["xmlDesc"].asString()); + virStoragePoolPtr pool_ptr; + int ret; - case _qmf::Node::METHOD_STORAGEPOOLDEFINEXML: - { - _qmf::ArgsNodeStoragePoolDefineXML *io_args = (_qmf::ArgsNodeStoragePoolDefineXML *) &args; - virStoragePoolPtr pool_ptr; + pool_ptr = virStoragePoolCreateXML (_conn, xmlDesc.c_str(), 0); + if (pool_ptr == NULL) { + std::string err = FORMAT_ERR(_conn, "Error creating storage pool using xml description (virStoragePoolCreateXML).", &ret); + raiseException(session, event, err, STATUS_USER + ret); + return false; + } - pool_ptr = virStoragePoolDefineXML (conn, io_args->i_xmlDesc.c_str(), 0); - if (pool_ptr == NULL) { - errstr = FORMAT_ERR(conn, "Error defining storage pool using xml description (virStoragePoolDefineXML).", &ret); - return STATUS_USER + ret; - } + PoolWrap *pool; + try { + pool = new PoolWrap(this, pool_ptr, _conn); + _pools.push_back(pool); + event.addReturnArgument("pool", pool->objectID()); + } catch (int i) { + delete pool; + std::string err = FORMAT_ERR(_conn, "Error constructing pool object in virStoragePoolCreateXML.", &ret); + raiseException(session, event, err, STATUS_USER + ret); + return false; + } - PoolWrap *pool; - try { - pool = new PoolWrap(agent, this, pool_ptr, conn); - pools.push_back(pool); - io_args->o_pool = pool->GetManagementObject()->getObjectId(); - } catch (int i) { - delete pool; - errstr = FORMAT_ERR(conn, "Error constructing pool object in virStoragePoolDefineXML.", &ret); - return STATUS_USER + i; - } - return STATUS_OK; + return true; +} - } - case _qmf::Node::METHOD_STORAGEPOOLCREATEXML: - { - _qmf::ArgsNodeStoragePoolCreateXML *io_args = (_qmf::ArgsNodeStoragePoolCreateXML *) &args; - virStoragePoolPtr pool_ptr; - - pool_ptr = virStoragePoolCreateXML (conn, io_args->i_xmlDesc.c_str(), 0); - if (pool_ptr == NULL) { - errstr = FORMAT_ERR(conn, "Error creating storage pool using xml description (virStoragePoolCreateXML).", &ret); - return STATUS_USER + ret; - } +bool +NodeWrap::findStoragePoolSources(qmf::AgentSession& session, + qmf::AgentEvent& event) +{ + qpid::types::Variant::Map args(event.getArguments()); + const std::string type(args["type"].asString()); + const std::string srcSpec(args["srcSpec"].asString()); + char *xml_result; + int ret; - PoolWrap *pool; - try { - pool = new PoolWrap(agent, this, pool_ptr, conn); - pools.push_back(pool); - io_args->o_pool = pool->GetManagementObject()->getObjectId(); - } catch (int i) { - delete pool; - errstr = FORMAT_ERR(conn, "Error constructing pool object in virStoragePoolCreateXML.", &ret); - return STATUS_USER + i; - } + xml_result = virConnectFindStoragePoolSources(_conn, type.c_str(), srcSpec.c_str(), 0); + if (xml_result == NULL) { + std::string err = FORMAT_ERR(_conn, "Error creating storage pool using xml description (virStoragePoolCreateXML).", &ret); + raiseException(session, event, err, STATUS_USER + ret); + return false; + } - return STATUS_OK; + event.addReturnArgument("xmlDesc", xml_result); + free(xml_result); + + return true; +} + +bool +NodeWrap::handleMethod(qmf::AgentSession& session, qmf::AgentEvent& event) +{ + if (!event.hasDataAddr() || *this == event.getDataAddr()) { + const std::string& methodName(event.getMethodName()); + bool success; + + if (methodName == "domainDefineXML") { + success = domainDefineXML(session, event); + } else if (methodName == "storagePoolDefineXML") { + success = storagePoolDefineXML(session, event); + } else if (methodName == "storagePoolCreateXML") { + success = storagePoolCreateXML(session, event); + } else if (methodName == "findStoragePoolSources") { + success = findStoragePoolSources(session, event); + } else { + raiseException(session, event, + ERROR_UNKNOWN_METHOD, STATUS_UNKNOWN_METHOD); + return true; } - case _qmf::Node::METHOD_FINDSTORAGEPOOLSOURCES: - { - _qmf::ArgsNodeFindStoragePoolSources *io_args = (_qmf::ArgsNodeFindStoragePoolSources *) &args; - char *xml_result; - - xml_result = virConnectFindStoragePoolSources(conn, io_args->i_type.c_str(), io_args->i_srcSpec.c_str(), 0); - if (xml_result == NULL) { - errstr = FORMAT_ERR(conn, "Error creating storage pool using xml description (virStoragePoolCreateXML).", &ret); - return STATUS_USER + ret; - } - io_args->o_xmlDesc = xml_result; - free(xml_result); + if (success) { + session.methodSuccess(event); + } + return true; + } else { + bool handled = false; - return STATUS_OK; + for (DomainList::iterator iter = _domains.begin(); + !handled && iter != _domains.end(); iter++) { + handled = (*iter)->handleMethod(session, event); + } + + for (PoolList::iterator iter = _pools.begin(); + !handled && iter != _pools.end(); iter++) { + handled = (*iter)->handleMethod(session, event); } - } - return STATUS_NOT_IMPLEMENTED; + return handled; + } } static void @@ -521,7 +577,6 @@ print_usage() printf("\t-d | --daemon run as a daemon.\n"); printf("\t-h | --help print this help message.\n"); printf("\t-b | --broker specify broker host name..\n"); - printf("\t-g | --gssapi force GSSAPI authentication.\n"); printf("\t-u | --username username to use for authentication purproses.\n"); printf("\t-s | --service service name to use for authentication purproses.\n"); printf("\t-p | --port specify broker port.\n"); @@ -536,8 +591,7 @@ int main(int argc, char** argv) { int idx = 0; bool verbose = false; bool daemonize = false; - bool gssapi = false; - char *host = NULL; + const char *host = NULL; char *username = NULL; char *service = NULL; int port = 5672; @@ -546,14 +600,13 @@ int main(int argc, char** argv) { {"help", 0, 0, 'h'}, {"daemon", 0, 0, 'd'}, {"broker", 1, 0, 'b'}, - {"gssapi", 0, 0, 'g'}, {"username", 1, 0, 'u'}, {"service", 1, 0, 's'}, {"port", 1, 0, 'p'}, {0, 0, 0, 0} }; - while ((arg = getopt_long(argc, argv, "hdb:gu:s:p:", opt, &idx)) != -1) { + while ((arg = getopt_long(argc, argv, "hdb:u:s:p:", opt, &idx)) != -1) { switch (arg) { case 'h': case '?': @@ -582,9 +635,6 @@ int main(int argc, char** argv) { exit(1); } break; - case 'g': - gssapi = true; - break; case 'p': if (optarg) { port = atoi(optarg); @@ -620,44 +670,42 @@ int main(int argc, char** argv) { // This prevents us from dying if libvirt disconnects. signal(SIGPIPE, SIG_IGN); - // Create the management agent - ManagementAgent::Singleton singleton; - ManagementAgent* agent = singleton.getInstance(); - - // Register the schema with the agent - _qmf::Package packageInit(agent); - - // Start the agent. It will attempt to make a connection to the - // management broker. The third argument is the interval for sending - // updates to stats/properties to the broker. The last is set to 'true' - // to keep this all single threaded. Otherwise a second thread would be - // used to implement methods. - - qpid::management::ConnectionSettings settings; - settings.host = host ? host : "127.0.0.1"; - settings.port = port; + qpid::types::Variant::Map options; if (username != NULL) { - settings.username = username; + options["username"] = username; } if (service != NULL) { - settings.service = service; + options["service"] = service; } - if (gssapi == true) { - settings.mechanism = "GSSAPI"; + + if (host == NULL) { + host = "127.0.0.1"; } - agent->setName("Red Hat", "libvirt-qpid"); - agent->init(settings, 3, true); + std::stringstream url; + + url << "amqp:" << "tcp" << ":" << host << ":" << port; + + qpid::messaging::Connection amqp_connection(url.str(), options); + amqp_connection.open(); + + qmf::AgentSession session(amqp_connection); + session.setVendor("redhat.com"); + session.setProduct("libvirt-qpid"); + session.setAttribute("hostname", host); - while(true) { + session.open(); + NodeWrap::PackageDefinition package; + package.configure(session); + + initErrorSchema(session); + + while (true) { try { - NodeWrap node(agent, "Libvirt Node"); + NodeWrap node(session, package); node.doLoop(); - } catch (int err) { - } + } catch (int err) { } sleep(10); } } - - diff --git a/src/NodeWrap.h b/src/NodeWrap.h index 920608b..6f55061 100644 --- a/src/NodeWrap.h +++ b/src/NodeWrap.h @@ -1,60 +1,70 @@ +#ifndef NODE_WRAP_H +#define NODE_WRAP_H -#include <qpid/management/Manageable.h> -#include <qpid/management/ManagementObject.h> -#include <qpid/agent/ManagementAgent.h> -#include <qpid/client/ConnectionSettings.h> -#include <qpid/sys/Mutex.h> - -#include "Package.h" -#include "Node.h" -#include "Domain.h" -#include "Pool.h" +#include "ManagedObject.h" +#include "QmfPackage.h" #include <unistd.h> #include <cstdlib> #include <iostream> +#include <vector> #include <sstream> #include <libvirt/libvirt.h> #include <libvirt/virterror.h> -using namespace qpid::management; -using namespace qpid::sys; -using namespace std; -using qpid::management::ManagementObject; -using qpid::management::Manageable; -using qpid::management::Args; -using qpid::sys::Mutex; -// Forward decl of DomainWrap to get around cyclic reference. class DomainWrap; class PoolWrap; -class NodeWrap : public Manageable +class NodeWrap: + public PackageOwner<qmf::com::redhat::libvirt::PackageDefinition>, + public ManagedObject { - string name; - ManagementAgent *agent; - qmf::com::redhat::libvirt::Node *mgmtObject; - std::vector<DomainWrap*> domains; - std::vector<PoolWrap*> pools; + typedef std::vector<DomainWrap*> DomainList; + typedef std::vector<PoolWrap*> PoolList; - virConnectPtr conn; + DomainList _domains; + PoolList _pools; -public: + virConnectPtr _conn; - NodeWrap(ManagementAgent* agent, string _name); - ~NodeWrap(); + qmf::AgentSession& _session; + PackageDefinition& _package; - ManagementObject* GetManagementObject(void) const - { return mgmtObject; } +public: + NodeWrap(qmf::AgentSession& agent_session, PackageDefinition& package); + ~NodeWrap(); void doLoop(); - void syncDomains(); + + bool handleMethod(qmf::AgentSession& session, qmf::AgentEvent& event); + + virtual PackageDefinition& package(void) { return _package; } + + virtual void addData(qmf::Data& data) { + _session.addData(data); + } + + virtual void delData(qmf::Data& data) { + _session.delData(data.getAddr()); + } + +protected: + void syncDomains(void); + void syncPools(void); void checkPool(char *pool_name); - void syncPools(); - status_t ManagementMethod (uint32_t methodId, Args& args, std::string &errstr); + bool domainDefineXML(qmf::AgentSession& session, + qmf::AgentEvent& event); + bool storagePoolDefineXML(qmf::AgentSession& session, + qmf::AgentEvent& event); + bool storagePoolCreateXML(qmf::AgentSession& session, + qmf::AgentEvent& event); + bool findStoragePoolSources(qmf::AgentSession& session, + qmf::AgentEvent& event); }; +#endif diff --git a/src/PoolWrap.cpp b/src/PoolWrap.cpp index 0274e63..a5992f2 100644 --- a/src/PoolWrap.cpp +++ b/src/PoolWrap.cpp @@ -1,4 +1,3 @@ - #include <libxml/xmlmemory.h> #include <libxml/parser.h> #include <string.h> @@ -7,42 +6,40 @@ #include "PoolWrap.h" #include "VolumeWrap.h" #include "Error.h" +#include "Exception.h" -#include "ArgsPoolCreateVolumeXML.h" -#include "ArgsPoolGetXMLDesc.h" - -namespace _qmf = qmf::com::redhat::libvirt; -PoolWrap::PoolWrap(ManagementAgent *_agent, NodeWrap *parent, - virStoragePoolPtr _pool_pointer, virConnectPtr _connect) : - agent(_agent), pool_ptr(_pool_pointer), conn(_connect) +PoolWrap::PoolWrap(NodeWrap *parent, + virStoragePoolPtr pool_ptr, + virConnectPtr connect): + PackageOwner<NodeWrap::PackageDefinition>(parent), + ManagedObject(package().data_Pool), + _pool_ptr(pool_ptr), _conn(connect) { int ret; char pool_uuid_str[VIR_UUID_STRING_BUFLEN]; const char *pool_name_str; char *parent_volume = NULL; - pool = NULL; - - ret = virStoragePoolGetUUIDString(pool_ptr, pool_uuid_str); + ret = virStoragePoolGetUUIDString(_pool_ptr, pool_uuid_str); if (ret < 0) { - REPORT_ERR(conn, "PoolWrap: Unable to get UUID\n"); + REPORT_ERR(_conn, "PoolWrap: Unable to get UUID\n"); throw 1; } - pool_name_str = virStoragePoolGetName(pool_ptr); + pool_name_str = virStoragePoolGetName(_pool_ptr); if (pool_name_str == NULL) { - REPORT_ERR(conn, "PoolWrap: error getting pool name\n"); + REPORT_ERR(_conn, "PoolWrap: error getting pool name\n"); throw 1; } - pool_sources_xml = virConnectFindStoragePoolSources(conn, "logical", NULL, 0); + _pool_sources_xml = virConnectFindStoragePoolSources(_conn, "logical", NULL, 0); - if (pool_sources_xml) { + if (_pool_sources_xml) { xmlDocPtr doc; xmlNodePtr cur; - doc = xmlParseMemory(pool_sources_xml, strlen(pool_sources_xml)); + doc = xmlParseMemory(_pool_sources_xml, strlen(_pool_sources_xml)); if (doc == NULL ) { goto done; @@ -77,7 +74,7 @@ PoolWrap::PoolWrap(ManagementAgent *_agent, NodeWrap *parent, if (name && path) { if (strcmp(pool_name_str, (char *) name) == 0) { virStorageVolPtr vol; - vol = virStorageVolLookupByPath(conn, (char *) path); + vol = virStorageVolLookupByPath(_conn, (char *) path); if (vol != NULL) { printf ("found storage volume associated with pool!\n"); parent_volume = virStorageVolGetPath(vol); @@ -96,22 +93,28 @@ PoolWrap::PoolWrap(ManagementAgent *_agent, NodeWrap *parent, } done: + _pool_name = pool_name_str; + _pool_uuid = pool_uuid_str; - pool_name_str = virStoragePoolGetName(pool_ptr); - - pool_name = pool_name_str; - pool_uuid = pool_uuid_str; - - pool = new _qmf::Pool(agent, this, parent, pool_uuid, pool_name, parent_volume ? parent_volume : ""); - agent->addObject(pool); + _data.setProperty("uuid", _pool_uuid); + _data.setProperty("name", _pool_name); + _data.setProperty("parentVolume", parent_volume ? parent_volume : ""); + _data.setProperty("node", parent->objectID()); // Call refresh storage volumes in case anything changed in libvirt. // I don't think we're too concerned if it fails? - virStoragePoolRefresh(pool_ptr, 0); + virStoragePoolRefresh(_pool_ptr, 0); // Set storage pool state to an inactive. Should the state be different, the // subsequent call to update will pick it up and fix it. - storagePoolState = VIR_STORAGE_POOL_INACTIVE; + _storagePoolState = VIR_STORAGE_POOL_INACTIVE; + + // Set the state before adding the new Data object + updateProperties(); + + // This must be done before update(), which will check for and create any + // volumes. (VolumeWrap objects will need a valid parent DataAddr.) + addData(_data); // Call update() here so we set the state and see if there are any volumes // before returning the new object. @@ -121,21 +124,21 @@ done: PoolWrap::~PoolWrap() { // Destroy volumes.. - for (std::vector<VolumeWrap*>::iterator iter = volumes.begin(); iter != volumes.end();) { + VolumeList::iterator iter = _volumes.begin(); + while (iter != _volumes.end()) { delete (*iter); - iter = volumes.erase(iter); + iter = _volumes.erase(iter); } - if (pool) { - pool->resourceDestroy(); - } - virStoragePoolFree(pool_ptr); + virStoragePoolFree(_pool_ptr); + + delData(_data); } -char * +const char * PoolWrap::getPoolSourcesXml() { - return pool_sources_xml; + return _pool_sources_xml; } void @@ -146,30 +149,30 @@ PoolWrap::syncVolumes() int i; virStoragePoolInfo info; - cout << "Syncing volumes.\n"; + std::cout << "Syncing volumes.\n"; - ret = virStoragePoolGetInfo(pool_ptr, &info); + ret = virStoragePoolGetInfo(_pool_ptr, &info); if (ret < 0) { - REPORT_ERR(conn, "PoolWrap: Unable to get info of storage pool"); + REPORT_ERR(_conn, "PoolWrap: Unable to get info of storage pool"); return; } // Only try to list volumes if the storage pool is active. if (info.state != VIR_STORAGE_POOL_INACTIVE) { - maxactive = virStoragePoolNumOfVolumes(pool_ptr); + maxactive = virStoragePoolNumOfVolumes(_pool_ptr); if (maxactive < 0) { //vshError(ctl, FALSE, "%s", _("Failed to list active vols")); - REPORT_ERR(conn, "error getting number of volumes in pool\n"); + REPORT_ERR(_conn, "error getting number of volumes in pool\n"); return; } char **names; names = (char **) malloc(sizeof(char *) * maxactive); - ret = virStoragePoolListVolumes(pool_ptr, names, maxactive); + ret = virStoragePoolListVolumes(_pool_ptr, names, maxactive); if (ret < 0) { - REPORT_ERR(conn, "error getting list of volumes\n"); + REPORT_ERR(_conn, "error getting list of volumes\n"); return; } @@ -178,28 +181,28 @@ PoolWrap::syncVolumes() bool found = false; char *volume_name = names[i]; - for (std::vector<VolumeWrap*>::iterator iter = volumes.begin(); - iter != volumes.end(); iter++) { - if ((*iter)->volume_name == volume_name) { + for (VolumeList::iterator iter = _volumes.begin(); + iter != _volumes.end(); iter++) { + if (strcmp((*iter)->name(), volume_name) == 0) { found = true; break; } } if (!found) { - vol_ptr = virStorageVolLookupByName(pool_ptr, volume_name); + vol_ptr = virStorageVolLookupByName(_pool_ptr, volume_name); if (vol_ptr == NULL) { - REPORT_ERR(conn, "error looking up storage volume by name\n"); + REPORT_ERR(_conn, "error looking up storage volume by name\n"); continue; } VolumeWrap *volume = NULL; try { - VolumeWrap *volume = new VolumeWrap(agent, this, vol_ptr, conn); + VolumeWrap *volume = new VolumeWrap(this, vol_ptr, _conn); printf("Created new volume: %s, ptr is %p\n", volume_name, vol_ptr); - volumes.push_back(volume); + _volumes.push_back(volume); } catch (int i) { printf ("Error constructing volume\n"); - REPORT_ERR(conn, "constructing volume."); + REPORT_ERR(_conn, "constructing volume."); if (volume) { delete volume; } @@ -214,14 +217,14 @@ PoolWrap::syncVolumes() } /* Go through our list of volumes and ensure that they still exist. */ - for (std::vector<VolumeWrap*>::iterator iter = volumes.begin(); iter != volumes.end();) { - - printf ("Verifying volume %s\n", (*iter)->volume_name.c_str()); - virStorageVolPtr ptr = virStorageVolLookupByName(pool_ptr, (*iter)->volume_name.c_str()); + VolumeList::iterator iter = _volumes.begin(); + while (iter != _volumes.end()) { + printf ("Verifying volume %s\n", (*iter)->name()); + virStorageVolPtr ptr = virStorageVolLookupByName(_pool_ptr, (*iter)->name()); if (ptr == NULL) { - printf("Destroying volume %s\n", (*iter)->volume_name.c_str()); + printf("Destroying volume %s\n", (*iter)->name()); delete(*iter); - iter = volumes.erase(iter); + iter = _volumes.erase(iter); } else { virStorageVolFree(ptr); iter++; @@ -229,170 +232,206 @@ PoolWrap::syncVolumes() } /* And finally *phew*, call update() on all volumes. */ - for (std::vector<VolumeWrap*>::iterator iter = volumes.begin(); iter != volumes.end(); iter++) { + for (iter = _volumes.begin(); iter != _volumes.end(); iter++) { (*iter)->update(); } } void -PoolWrap::update() +PoolWrap::updateProperties(void) { virStoragePoolInfo info; int ret; - ret = virStoragePoolGetInfo(pool_ptr, &info); + ret = virStoragePoolGetInfo(_pool_ptr, &info); if (ret < 0) { - REPORT_ERR(conn, "PoolWrap: Unable to get info of storage pool"); + REPORT_ERR(_conn, "PoolWrap: Unable to get info of storage pool"); return; } + const char *state = NULL; switch (info.state) { case VIR_STORAGE_POOL_INACTIVE: - pool->set_state("inactive"); + state = "inactive"; break; case VIR_STORAGE_POOL_BUILDING: - pool->set_state("building"); + state = "building"; break; case VIR_STORAGE_POOL_RUNNING: - pool->set_state("running"); + state = "running"; break; case VIR_STORAGE_POOL_DEGRADED: - pool->set_state("degraded"); + state = "degraded"; break; } + _data.setProperty("state", state); - pool->set_capacity(info.capacity); - pool->set_allocation(info.allocation); - pool->set_available(info.available); + _data.setProperty("capacity", (uint64_t)info.capacity); + _data.setProperty("allocation", (uint64_t)info.allocation); + _data.setProperty("available", (uint64_t)info.available); // Check if state has changed compared to stored state. If so, rescan // storage pool sources (eg. logical pools on a lun might now be visible) - if (storagePoolState != info.state) - pool_sources_xml = virConnectFindStoragePoolSources(conn, "logical", NULL, 0); + if (_storagePoolState != info.state) { + _pool_sources_xml = virConnectFindStoragePoolSources(_conn, "logical", NULL, 0); + } + + _storagePoolState = info.state; +} - storagePoolState = info.state; +void +PoolWrap::update(void) +{ + updateProperties(); - // Sync volumes after (potentially) rescanning for logical storage pool sources - // so we pick up any new pools if the state of this pool changed. + // Sync volumes after (potentially) rescanning for logical storage pool + // sources so we pick up any new pools if the state of this pool changed. syncVolumes(); } -Manageable::status_t -PoolWrap::ManagementMethod(uint32_t methodId, Args& args, std::string &errstr) +bool +PoolWrap::handleMethod(qmf::AgentSession& session, qmf::AgentEvent& event) { - cout << "Method Received: " << methodId << endl; int ret; - switch (methodId) { - case _qmf::Pool::METHOD_GETXMLDESC: - { - _qmf::ArgsPoolGetXMLDesc *ioArgs = (_qmf::ArgsPoolGetXMLDesc *) &args; - char *desc; - - desc = virStoragePoolGetXMLDesc(pool_ptr, 0); - if (desc) { - ioArgs->o_description = desc; - } else { - errstr = FORMAT_ERR(conn, "Error getting XML description of storage pool (virStoragePoolGetXMLDesc).", &ret); - return STATUS_USER + ret; - } - return STATUS_OK; + if (*this != event.getDataAddr()) { + bool handled = false; + VolumeList::iterator iter = _volumes.begin(); + + while (!handled && iter != _volumes.end()) { + handled = (*iter)->handleMethod(session, event); } + return handled; + } - case _qmf::Pool::METHOD_CREATE: - { - ret = virStoragePoolCreate(pool_ptr, 0); - if (ret < 0) { - errstr = FORMAT_ERR(conn, "Error creating new storage pool (virStoragePoolCreate).", &ret); - return STATUS_USER + ret; - } - update(); - return STATUS_OK; + const std::string& methodName(event.getMethodName()); + qpid::types::Variant::Map args(event.getArguments()); + + if (methodName == "getXMLDesc") { + const char *desc = virStoragePoolGetXMLDesc(_pool_ptr, 0); + if (!desc) { + std::string err = FORMAT_ERR(_conn, "Error getting XML description of storage pool (virStoragePoolGetXMLDesc).", &ret); + raiseException(session, event, err, STATUS_USER + ret); + } else { + event.addReturnArgument("description", desc); + session.methodSuccess(event); } + return true; + } - case _qmf::Pool::METHOD_BUILD: - { - ret = virStoragePoolBuild(pool_ptr, 0); - if (ret < 0) { - errstr = FORMAT_ERR(conn, "Error building storage pool (virStoragePoolBuild).", &ret); - return STATUS_USER + ret; - } + if (methodName == "create") { + ret = virStoragePoolCreate(_pool_ptr, 0); + if (ret < 0) { + std::string err = FORMAT_ERR(_conn, "Error creating new storage pool (virStoragePoolCreate).", &ret); + raiseException(session, event, err, STATUS_USER + ret); + } else { update(); - return STATUS_OK; + session.methodSuccess(event); } + return true; + } - case _qmf::Pool::METHOD_DESTROY: - { - ret = virStoragePoolDestroy(pool_ptr); - if (ret < 0) { - errstr = FORMAT_ERR(conn, "Error destroying storage pool (virStoragePoolDestroy).", &ret); - return STATUS_USER + ret; - } + if (methodName == "build") { + ret = virStoragePoolBuild(_pool_ptr, 0); + if (ret < 0) { + std::string err = FORMAT_ERR(_conn, "Error building storage pool (virStoragePoolBuild).", &ret); + raiseException(session, event, err, STATUS_USER + ret); + } else { update(); - return STATUS_OK; + session.methodSuccess(event); } + return true; + } - case _qmf::Pool::METHOD_DELETE: - { - ret = virStoragePoolDelete(pool_ptr, 0); - if (ret < 0) { - errstr = FORMAT_ERR(conn, "Error deleting storage pool (virStoragePoolDelete).", &ret); - return STATUS_USER + ret; - } + if (methodName == "destroy") { + ret = virStoragePoolDestroy(_pool_ptr); + if (ret < 0) { + std::string err = FORMAT_ERR(_conn, "Error destroying storage pool (virStoragePoolDestroy).", &ret); + raiseException(session, event, err, STATUS_USER + ret); + } else { update(); - return STATUS_OK; + session.methodSuccess(event); } + return true; + } - case _qmf::Pool::METHOD_UNDEFINE: - { - ret = virStoragePoolUndefine(pool_ptr); - if (ret < 0) { - errstr = FORMAT_ERR(conn, "Error undefining storage pool (virStoragePoolUndefine).", &ret); - return STATUS_USER + ret; - } + if (methodName == "delete") { + ret = virStoragePoolDelete(_pool_ptr, 0); + if (ret < 0) { + std::string err = FORMAT_ERR(_conn, "Error deleting storage pool (virStoragePoolDelete).", &ret); + raiseException(session, event, err, STATUS_USER + ret); + } else { update(); - return STATUS_OK; + session.methodSuccess(event); } + return true; + } - case _qmf::Pool::METHOD_CREATEVOLUMEXML: - { - _qmf::ArgsPoolCreateVolumeXML *io_args = (_qmf::ArgsPoolCreateVolumeXML *) &args; - virStorageVolPtr volume_ptr; - - volume_ptr = virStorageVolCreateXML(pool_ptr, io_args->i_xmlDesc.c_str(), 0); - if (volume_ptr == NULL) { - errstr = FORMAT_ERR(conn, "Error creating new storage volume from XML description (virStorageVolCreateXML).", &ret); - return STATUS_USER + ret; - } - - VolumeWrap *volume; - try { - volume = new VolumeWrap(agent, this, volume_ptr, conn); - volumes.push_back(volume); - io_args->o_volume = volume->GetManagementObject()->getObjectId(); - } catch (int i) { - delete volume; - errstr = FORMAT_ERR(conn, "Error constructing pool object in virStorageVolCreateXML.", &ret); - return STATUS_USER + i; - } - + if (methodName == "undefine") { + ret = virStoragePoolUndefine(_pool_ptr); + if (ret < 0) { + std::string err = FORMAT_ERR(_conn, "Error undefining storage pool (virStoragePoolUndefine).", &ret); + raiseException(session, event, err, STATUS_USER + ret); + } else { update(); - return STATUS_OK; + session.methodSuccess(event); } - - case _qmf::Pool::METHOD_REFRESH: - { - ret = virStoragePoolRefresh(pool_ptr, 0); - if (ret < 0) { - errstr = FORMAT_ERR(conn, "Error refreshing storage pool (virStoragePoolRefresh).", &ret); - return STATUS_USER + ret; - } + return true; + } + + if (methodName == "createVolumeXML") { + if (createVolumeXML(session, event)) { + session.methodSuccess(event); + } + return true; + } + + if (methodName == "refresh") { + ret = virStoragePoolRefresh(_pool_ptr, 0); + if (ret < 0) { + std::string err = FORMAT_ERR(_conn, "Error refreshing storage pool (virStoragePoolRefresh).", &ret); + raiseException(session, event, err, STATUS_USER + ret); + } else { update(); - return STATUS_OK; + session.methodSuccess(event); } + return true; } - return STATUS_NOT_IMPLEMENTED; + raiseException(session, event, + ERROR_UNKNOWN_METHOD, STATUS_UNKNOWN_METHOD); + return true; } +bool +PoolWrap::createVolumeXML(qmf::AgentSession& session, qmf::AgentEvent& event) +{ + int ret; + virStorageVolPtr volume_ptr; + + qpid::types::Variant::Map args(event.getArguments()); + + std::string xmlDesc(args["xmlDesc"]); + volume_ptr = virStorageVolCreateXML(_pool_ptr, xmlDesc.c_str(), 0); + if (volume_ptr == NULL) { + std::string err = FORMAT_ERR(_conn, "Error creating new storage volume from XML description (virStorageVolCreateXML).", &ret); + raiseException(session, event, err, STATUS_USER + ret); + return false; + } + + VolumeWrap *volume; + try { + volume = new VolumeWrap(this, volume_ptr, _conn); + _volumes.push_back(volume); + event.addReturnArgument("volume", volume->objectID()); + } catch (int i) { + delete volume; + std::string err = FORMAT_ERR(_conn, "Error constructing pool object in virStorageVolCreateXML.", &ret); + raiseException(session, event, err, STATUS_USER + ret); + return false; + } + + update(); + return true; +} diff --git a/src/PoolWrap.h b/src/PoolWrap.h index 654ce44..1e6dfeb 100644 --- a/src/PoolWrap.h +++ b/src/PoolWrap.h @@ -1,10 +1,7 @@ -#include <qpid/management/Manageable.h> -#include <qpid/management/ManagementObject.h> -#include <qpid/agent/ManagementAgent.h> -#include <qpid/sys/Mutex.h> +#ifndef POOL_WRAP_H +#define POOL_WRAP_H -#include "Package.h" -#include "Pool.h" +#include "NodeWrap.h" #include <unistd.h> #include <cstdlib> @@ -15,49 +12,46 @@ #include <libvirt/libvirt.h> #include <libvirt/virterror.h> -using namespace qpid::management; -using namespace qpid::sys; -using namespace std; -using qpid::management::ManagementObject; -using qpid::management::Manageable; -using qpid::management::Args; -using qpid::sys::Mutex; -// Forward decl. class VolumeWrap; -class PoolWrap : public Manageable +class PoolWrap: + public PackageOwner<NodeWrap::PackageDefinition>, + public ManagedObject { - ManagementAgent *agent; - qmf::com::redhat::libvirt::Pool *pool; + typedef std::vector<VolumeWrap*> VolumeList; - virStoragePoolPtr pool_ptr; - virConnectPtr conn; + virStoragePoolPtr _pool_ptr; + virConnectPtr _conn; - std::vector<VolumeWrap*> volumes; + VolumeList _volumes; - char *pool_sources_xml; - int storagePoolState; + char *_pool_sources_xml; + int _storagePoolState; -public: + std::string _pool_name; + std::string _pool_uuid; - PoolWrap(ManagementAgent *agent, NodeWrap *parent, virStoragePoolPtr pool_ptr, virConnectPtr connect); +public: + PoolWrap(NodeWrap *parent, + virStoragePoolPtr pool_ptr, + virConnectPtr connect); ~PoolWrap(); - std::string pool_name; - std::string pool_uuid; + std::string name(void) { return _pool_name; } + std::string uuid(void) { return _pool_uuid; } - ManagementObject* GetManagementObject(void) const - { - return pool; - } + bool handleMethod(qmf::AgentSession& session, qmf::AgentEvent& event); - char * getPoolSourcesXml(); + const char *getPoolSourcesXml(); void update(); void syncVolumes(); - status_t ManagementMethod (uint32_t methodId, Args& args, std::string &errstr); -}; +private: + void updateProperties(); + bool createVolumeXML(qmf::AgentSession& session, qmf::AgentEvent& event); +}; +#endif diff --git a/src/VolumeWrap.cpp b/src/VolumeWrap.cpp index d4ce536..fcda96e 100644 --- a/src/VolumeWrap.cpp +++ b/src/VolumeWrap.cpp @@ -9,52 +9,54 @@ #include "PoolWrap.h" #include "VolumeWrap.h" #include "Error.h" +#include "Exception.h" -#include "ArgsVolumeGetXMLDesc.h" -namespace _qmf = qmf::com::redhat::libvirt; - -VolumeWrap::VolumeWrap(ManagementAgent *agent, PoolWrap *parent, - virStorageVolPtr _volume_ptr, virConnectPtr _conn) : - conn(_conn), volume_ptr(_volume_ptr) +VolumeWrap::VolumeWrap(PoolWrap *parent, + virStorageVolPtr volume_ptr, + virConnectPtr connect): + PackageOwner<PoolWrap::PackageDefinition>(parent), + ManagedObject(package().data_Volume), + _volume_ptr(volume_ptr), _conn(connect), + _lvm_name(""), _has_lvm_child(false), + _wrap_parent(parent) { - const char *volume_key_s; - char *volume_path_s; - const char *volume_name_s; - - volume = NULL; + const char *volume_key; + char *volume_path; + const char *volume_name; - wrap_parent = parent; - - volume_key_s = virStorageVolGetKey(volume_ptr); - if (volume_key_s == NULL) { - REPORT_ERR(conn, "Error getting storage volume key\n"); + volume_key = virStorageVolGetKey(_volume_ptr); + if (volume_key == NULL) { + REPORT_ERR(_conn, "Error getting storage volume key\n"); throw 1; } - volume_key = volume_key_s; + _volume_key = volume_key; - volume_path_s = virStorageVolGetPath(volume_ptr); - if (volume_path_s == NULL) { - REPORT_ERR(conn, "Error getting volume path\n"); + volume_path = virStorageVolGetPath(_volume_ptr); + if (volume_path == NULL) { + REPORT_ERR(_conn, "Error getting volume path\n"); throw 1; } - volume_path = volume_path_s; + _volume_path = volume_path; - volume_name_s = virStorageVolGetName(volume_ptr); - if (volume_name_s == NULL) { - REPORT_ERR(conn, "Error getting volume name\n"); + volume_name = virStorageVolGetName(_volume_ptr); + if (volume_name == NULL) { + REPORT_ERR(_conn, "Error getting volume name\n"); throw 1; } - volume_name = volume_name_s; + _volume_name = volume_name; - lvm_name = ""; - has_lvm_child = false; + _data.setProperty("key", _volume_key); + _data.setProperty("path", _volume_path); + _data.setProperty("name", _volume_name); + _data.setProperty("childLVMName", _lvm_name); + _data.setProperty("storagePool", parent->objectID()); - checkForLVMPool(); + // Set defaults + _data.setProperty("capacity", (uint64_t)0); + _data.setProperty("allocation", (uint64_t)0); - volume = new _qmf::Volume(agent, this, parent, volume_key, volume_path, volume_name, lvm_name); - printf("adding volume to agent - volume %p\n", volume); - agent->addObject(volume); + addData(_data); printf("done\n"); } @@ -63,9 +65,9 @@ void VolumeWrap::checkForLVMPool() { char *real_path = NULL; - char *pool_sources_xml; + const char *pool_sources_xml; - pool_sources_xml = wrap_parent->getPoolSourcesXml(); + pool_sources_xml = _wrap_parent->getPoolSourcesXml(); if (pool_sources_xml) { xmlDocPtr doc; @@ -106,14 +108,14 @@ VolumeWrap::checkForLVMPool() if (name && path) { virStorageVolPtr vol; - printf ("xml returned device name %s, path %s; volume path is %s\n", name, path, volume_path.c_str()); - vol = virStorageVolLookupByPath(conn, (char *) path); + printf ("xml returned device name %s, path %s; volume path is %s\n", name, path, _volume_path.c_str()); + vol = virStorageVolLookupByPath(_conn, (char *) path); if (vol != NULL) { real_path = virStorageVolGetPath(vol); - if (real_path && strcmp(real_path, volume_path.c_str()) == 0) { + if (real_path && strcmp(real_path, _volume_path.c_str()) == 0) { printf ("found matching storage volume associated with pool!\n"); - lvm_name.assign((char *) name); - has_lvm_child = true; + _lvm_name.assign((char *) name); + _has_lvm_child = true; } } xmlFree(path); @@ -136,58 +138,59 @@ VolumeWrap::update() printf("Updating volume info\n"); - ret = virStorageVolGetInfo(volume_ptr, &info); + ret = virStorageVolGetInfo(_volume_ptr, &info); if (ret < 0) { - REPORT_ERR(conn, "VolumeWrap: Unable to get info of storage volume info\n"); + REPORT_ERR(_conn, "VolumeWrap: Unable to get info of storage volume info\n"); return; } - volume->set_capacity(info.capacity); - volume->set_allocation(info.allocation); + _data.setProperty("capacity", (uint64_t)info.capacity); + _data.setProperty("allocation", (uint64_t)info.allocation); } VolumeWrap::~VolumeWrap() { - if (volume) { - volume->resourceDestroy(); - } - virStorageVolFree(volume_ptr); + virStorageVolFree(_volume_ptr); + + delData(_data); } -Manageable::status_t -VolumeWrap::ManagementMethod(uint32_t methodId, Args& args, std::string &errstr) +bool +VolumeWrap::handleMethod(qmf::AgentSession& session, qmf::AgentEvent& event) { - cout << "Method Received: " << methodId << endl; int ret; - switch (methodId) { - case _qmf::Volume::METHOD_GETXMLDESC: - { - _qmf::ArgsVolumeGetXMLDesc *io_args = (_qmf::ArgsVolumeGetXMLDesc *) &args; - char *desc; - - desc = virStorageVolGetXMLDesc(volume_ptr, 0); - if (desc) { - io_args->o_description = desc; - } else { - errstr = FORMAT_ERR(conn, "Error getting xml description for volume (virStorageVolGetXMLDesc).", &ret); - return STATUS_USER + ret; - } - return STATUS_OK; + if (*this != event.getDataAddr()) { + return false; + } + + const std::string& methodName(event.getMethodName()); + qpid::types::Variant::Map args(event.getArguments()); + + if (methodName == "getXMLDesc") { + const char *desc = virStorageVolGetXMLDesc(_volume_ptr, 0); + if (!desc) { + std::string err = FORMAT_ERR(_conn, "Error getting xml description for volume (virStorageVolGetXMLDesc).", &ret); + raiseException(session, event, err, STATUS_USER + ret); + } else { + event.addReturnArgument("description", desc); + session.methodSuccess(event); } - case _qmf::Volume::METHOD_DELETE: - { - ret = virStorageVolDelete(volume_ptr, 0); - if (ret < 0) { - errstr = FORMAT_ERR(conn, "Error deleting storage volume (virStorageVolDelete).", &ret); - return STATUS_USER + ret; - } + return true; + } + + if (methodName == "delete") { + ret = virStorageVolDelete(_volume_ptr, 0); + if (ret < 0) { + std::string err = FORMAT_ERR(_conn, "Error deleting storage volume (virStorageVolDelete).", &ret); + raiseException(session, event, err, STATUS_USER + ret); + } else { update(); - return STATUS_OK; + session.methodSuccess(event); } + return true; } - - return STATUS_NOT_IMPLEMENTED; + raiseException(session, event, + ERROR_UNKNOWN_METHOD, STATUS_UNKNOWN_METHOD); + return true; } - - diff --git a/src/VolumeWrap.h b/src/VolumeWrap.h index 50d1347..7f3a63f 100644 --- a/src/VolumeWrap.h +++ b/src/VolumeWrap.h @@ -1,10 +1,7 @@ -#include <qpid/management/Manageable.h> -#include <qpid/management/ManagementObject.h> -#include <qpid/agent/ManagementAgent.h> -#include <qpid/sys/Mutex.h> +#ifndef VOLUME_WRAP_H +#define VOLUME_WRAP_H -#include "Package.h" -#include "Volume.h" +#include "PoolWrap.h" #include <unistd.h> #include <cstdlib> @@ -15,51 +12,36 @@ #include <libvirt/libvirt.h> #include <libvirt/virterror.h> -using namespace qpid::management; -using namespace qpid::sys; -using namespace std; -using qpid::management::ManagementObject; -using qpid::management::Manageable; -using qpid::management::Args; -using qpid::sys::Mutex; -// Forward decl. -class PoolWrap; - -class VolumeWrap : public Manageable +class VolumeWrap: + PackageOwner<PoolWrap::PackageDefinition>, + public ManagedObject { - ManagementAgent *agent; - qmf::com::redhat::libvirt::Volume *volume; - - std::string volume_key; - std::string volume_path; + virStorageVolPtr _volume_ptr; + virConnectPtr _conn; - std::string lvm_name; - bool has_lvm_child; + std::string _volume_name; + std::string _volume_key; + std::string _volume_path; - virConnectPtr conn; - virStorageVolPtr volume_ptr; + std::string _lvm_name; + bool _has_lvm_child; - PoolWrap *wrap_parent; + PoolWrap *_wrap_parent; void checkForLVMPool(); public: - - std::string volume_name; - - VolumeWrap(ManagementAgent *agent, PoolWrap *parent, virStorageVolPtr pool_ptr, virConnectPtr connect); + VolumeWrap(PoolWrap *parent, + virStorageVolPtr volume_ptr, + virConnectPtr connect); ~VolumeWrap(); - ManagementObject* GetManagementObject(void) const - { - return volume; - } + const char *name(void) { return _volume_name.c_str(); } void update(); - status_t ManagementMethod (uint32_t methodId, Args& args, std::string &errstr); + bool handleMethod(qmf::AgentSession& session, qmf::AgentEvent& event); }; - - +#endif
On Tue, Jul 12, 2011 at 06:28:46PM +0200, Zane Bitter wrote:
diff --git a/src/DomainWrap.cpp b/src/DomainWrap.cpp index 47876de..eab6bbc 100644 --- a/src/DomainWrap.cpp +++ b/src/DomainWrap.cpp @@ -2,266 +2,310 @@ #include "NodeWrap.h" #include "DomainWrap.h" #include "Error.h" +#include "Exception.h"
-#include "ArgsDomainMigrate.h" -#include "ArgsDomainRestore.h" -#include "ArgsDomainSave.h" -#include "ArgsDomainGetXMLDesc.h" +#include <cstdio>
-namespace _qmf = qmf::com::redhat::libvirt;
-DomainWrap::DomainWrap(ManagementAgent *agent, NodeWrap *parent, - virDomainPtr _domain_ptr, virConnectPtr _conn) : - domain_ptr(_domain_ptr), conn(_conn) +DomainWrap::DomainWrap( + NodeWrap *parent, + virDomainPtr domain_ptr, + virConnectPtr conn): + PackageOwner<NodeWrap::PackageDefinition>(parent), + ManagedObject(package().data_Domain), + _domain_ptr(domain_ptr), _conn(conn) { char dom_uuid[VIR_UUID_STRING_BUFLEN];
- domain = NULL; - - if (virDomainGetUUIDString(domain_ptr, dom_uuid) < 0) { - REPORT_ERR(conn, "DomainWrap: Unable to get UUID string of domain."); + if (virDomainGetUUIDString(_domain_ptr, dom_uuid) < 0) { + REPORT_ERR(_conn, "DomainWrap: Unable to get UUID string of domain."); throw 1;
I've noticed that most (all?) objects are being passed in a virConnectPtr instance too. This should be redundant for two reasons - If you have a virDomainPtr, you can call virDomainGetConnect() to get back the associated virConnectPtr (likewise for other libvirt objects) - We should kill the 'virConnectPtr' parameter from the REPORT_ERR macro/functions, and use 'virGetLastError' exclusively. This function is thread-safe, unlike the current virConnGetLastError being used. Since this problem was pre-existing, I'll leave it upto you whether you want to incorporate such a change into this patch or do it as a later followup.
void DomainWrap::update() { virDomainInfo info; int ret;
- ret = virDomainGetInfo(domain_ptr, &info); + ret = virDomainGetInfo(_domain_ptr, &info); if (ret < 0) { - REPORT_ERR(conn, "Domain get info failed."); + REPORT_ERR(_conn, "Domain get info failed."); /* Next domainSync() will take care of this in the node wrapper if the domain is * indeed dead. */ return; } else { + const char *state = NULL; switch (info.state) { - case VIR_DOMAIN_NOSTATE:
Add a 'default:' clause here
- domain->set_state("nostate"); + state = "nostate"; break; case VIR_DOMAIN_RUNNING: - domain->set_state("running"); + state = "running"; break; case VIR_DOMAIN_BLOCKED: - domain->set_state("blocked"); + state = "blocked"; break; case VIR_DOMAIN_PAUSED: - domain->set_state("paused"); + state = "paused"; break; case VIR_DOMAIN_SHUTDOWN: - domain->set_state("shutdown"); + state = "shutdown"; break; case VIR_DOMAIN_SHUTOFF: - domain->set_state("shutoff"); + state = "shutoff"; break; case VIR_DOMAIN_CRASHED: - domain->set_state("crashed"); + state = "crashed"; break; }
- domain->set_numVcpus(info.nrVirtCpu); - domain->set_maximumMemory(info.maxMem); - domain->set_memory(info.memory); - domain->set_cpuTime(info.cpuTime); + if (state) { + _data.setProperty("state", state); + }
We shouldn't skip out 'NOSTATE' here, juust make this unconditional
+ _data.setProperty("numVcpus", info.nrVirtCpu); + _data.setProperty("maximumMemory", info.maxMem); + _data.setProperty("memory", (uint64_t)info.memory); + _data.setProperty("cpuTime", (uint64_t)info.cpuTime); }
+bool +DomainWrap::migrate(qmf::AgentSession& session, qmf::AgentEvent& event) +{ + virConnectPtr dest_conn; + virDomainPtr rem_dom; + qpid::types::Variant::Map args(event.getArguments()); + int ret; + + // This is actually quite broken. Most setups won't allow unauthorized + // connection from one node to another directly like this. + dest_conn = virConnectOpen(args["destinationUri"].asString().c_str());
Since the agent was originally written, we now have a new migration API which avoids the need for a destination virConnectPtr object. Instead you can pass the 'destinationUri' directly to virDomainMigrateToURI, and set the PEER2PEER flag. NB, this still presumes that the source libvirtd can auth with the target libvirtd directly, but it avoids the need for matahari itself to authenticate. Typically we'd expect the libvirtd's to be setup with TLS+x590 or Kerberos so they can talk to each other
+ if (!dest_conn) { + std::string err = FORMAT_ERR(dest_conn, "Unable to connect to remote system for migration: virConnectOpen", &ret); + raiseException(session, event, err, STATUS_USER + ret); + return false; + } + + const std::string newDomainName_arg(args["newDomainName"]); + const char *new_dom_name = NULL; + if (newDomainName_arg.size() > 0) { + new_dom_name = newDomainName_arg.c_str(); + } + + const std::string uri_arg(args["uri"]); + const char *uri = NULL; + if (uri_arg.size() > 0) { + uri = uri_arg.c_str(); + } + + uint32_t flags(args["flags"]); + uint32_t bandwidth(args["bandwidth"]); + + printf ("calling migrate, new_dom_name: %s, uri: %s, flags: %d (live is %d)\n", + new_dom_name ? new_dom_name : "NULL", + uri ? uri : "NULL", + flags, + VIR_MIGRATE_LIVE); + + rem_dom = virDomainMigrate(_domain_ptr, dest_conn, flags, + new_dom_name, + uri, + bandwidth);
Ideally you want virDomainMigrateToURI, or even better virDomainMigrateToURI2 here.
diff --git a/src/Error.h b/src/Error.h index 388b41a..5bd7397 100644 --- a/src/Error.h +++ b/src/Error.h @@ -1,11 +1,19 @@ +#ifndef ERROR_H +#define ERROR_H + +#include <libvirt/libvirt.h> +#include <libvirt/virterror.h> +
#define REPORT_ERR(conn,msg) reportError(conn, msg, __func__, __LINE__, __FILE__)
#define FORMAT_ERR(conn,msg,err_ret) formatError(conn, msg, err_ret, __func__, __LINE__, __FILE__)
+ void reportError(virConnectPtr conn, const char *msg, const char *function, int line, const char *file);
std::string formatError(virConnectPtr conn, const char *msg, int *err_ret, const char *function, int line, const char *file);
+#endif
We want to remove virConnectPtr from both these methods & macros
diff --git a/src/Exception.cpp b/src/Exception.cpp new file mode 100644 index 0000000..105df27 --- /dev/null +++ b/src/Exception.cpp @@ -0,0 +1,38 @@ +#include "Exception.h" +#include "Error.h" +#include <qmf/Schema.h> +#include <qmf/SchemaProperty.h> +#include <qmf/Data.h> + + +#define ERROR_TEXT "error_text" +#define ERROR_CODE "error_code" + + +static qmf::Schema errorSchema(qmf::SCHEMA_TYPE_DATA, + "com.redhat.libvirt", "error");
This should be 'org.libvirt' really.
generated_file_list = \ - qmf/com/redhat/libvirt/Domain.cpp\ - qmf/com/redhat/libvirt/Node.cpp\ - qmf/com/redhat/libvirt/Package.cpp\ - qmf/com/redhat/libvirt/Pool.cpp\ - qmf/com/redhat/libvirt/Volume.cpp\ - qmf/com/redhat/libvirt/ArgsDomainGetXMLDesc.h\ - qmf/com/redhat/libvirt/ArgsDomainMigrate.h\ - qmf/com/redhat/libvirt/ArgsDomainRestore.h\ - qmf/com/redhat/libvirt/ArgsDomainSave.h\ - qmf/com/redhat/libvirt/ArgsNodeDomainDefineXML.h\ - qmf/com/redhat/libvirt/ArgsNodeStoragePoolCreateXML.h\ - qmf/com/redhat/libvirt/ArgsNodeStoragePoolDefineXML.h\ - qmf/com/redhat/libvirt/ArgsPoolCreateVolumeXML.h\ - qmf/com/redhat/libvirt/ArgsPoolGetXMLDesc.h\ - qmf/com/redhat/libvirt/ArgsVolumeGetXMLDesc.h\ - qmf/com/redhat/libvirt/Domain.h\ - qmf/com/redhat/libvirt/Node.h\ - qmf/com/redhat/libvirt/Package.h\ - qmf/com/redhat/libvirt/Pool.h\ - qmf/com/redhat/libvirt/Volume.h + qmf/com/redhat/libvirt/QmfPackage.cpp\ + qmf/com/redhat/libvirt/QmfPackage.h
Can we change these to org/libvirt too, or will that be an incompatible schema change ? Incidentally, is this fully backwards compatible from the POV of existing libvirt-qpid client apps, or are we starting a v2 libvirt schema here ?
static void @@ -521,7 +577,6 @@ print_usage() printf("\t-d | --daemon run as a daemon.\n"); printf("\t-h | --help print this help message.\n"); printf("\t-b | --broker specify broker host name..\n"); - printf("\t-g | --gssapi force GSSAPI authentication.\n"); printf("\t-u | --username username to use for authentication purproses.\n"); printf("\t-s | --service service name to use for authentication purproses.\n"); printf("\t-p | --port specify broker port.\n"); @@ -536,8 +591,7 @@ int main(int argc, char** argv) { int idx = 0; bool verbose = false; bool daemonize = false; - bool gssapi = false; - char *host = NULL; + const char *host = NULL; char *username = NULL; char *service = NULL; int port = 5672; @@ -546,14 +600,13 @@ int main(int argc, char** argv) { {"help", 0, 0, 'h'}, {"daemon", 0, 0, 'd'}, {"broker", 1, 0, 'b'}, - {"gssapi", 0, 0, 'g'}, {"username", 1, 0, 'u'}, {"service", 1, 0, 's'}, {"port", 1, 0, 'p'}, {0, 0, 0, 0} };
- while ((arg = getopt_long(argc, argv, "hdb:gu:s:p:", opt, &idx)) != -1) { + while ((arg = getopt_long(argc, argv, "hdb:u:s:p:", opt, &idx)) != -1) { switch (arg) { case 'h': case '?': @@ -582,9 +635,6 @@ int main(int argc, char** argv) { exit(1); } break; - case 'g': - gssapi = true; - break; case 'p': if (optarg) { port = atoi(optarg); @@ -620,44 +670,42 @@ int main(int argc, char** argv) { // This prevents us from dying if libvirt disconnects. signal(SIGPIPE, SIG_IGN);
- // Create the management agent - ManagementAgent::Singleton singleton; - ManagementAgent* agent = singleton.getInstance(); - - // Register the schema with the agent - _qmf::Package packageInit(agent); - - // Start the agent. It will attempt to make a connection to the - // management broker. The third argument is the interval for sending - // updates to stats/properties to the broker. The last is set to 'true' - // to keep this all single threaded. Otherwise a second thread would be - // used to implement methods. - - qpid::management::ConnectionSettings settings; - settings.host = host ? host : "127.0.0.1"; - settings.port = port; + qpid::types::Variant::Map options;
if (username != NULL) { - settings.username = username; + options["username"] = username; } if (service != NULL) { - settings.service = service; + options["service"] = service; } - if (gssapi == true) { - settings.mechanism = "GSSAPI"; + + if (host == NULL) { + host = "127.0.0.1"; }
- agent->setName("Red Hat", "libvirt-qpid"); - agent->init(settings, 3, true); + std::stringstream url; + + url << "amqp:" << "tcp" << ":" << host << ":" << port; + + qpid::messaging::Connection amqp_connection(url.str(), options); + amqp_connection.open(); + + qmf::AgentSession session(amqp_connection); + session.setVendor("redhat.com");
IMHO, this should really be 'org.libvirt' too
+ session.setProduct("libvirt-qpid"); + session.setAttribute("hostname", host);
Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Wed, Jul 13, 2011 at 2:46 AM, Daniel P. Berrange <berrange@redhat.com> wrote:
On Tue, Jul 12, 2011 at 06:28:46PM +0200, Zane Bitter wrote:
diff --git a/src/DomainWrap.cpp b/src/DomainWrap.cpp index 47876de..eab6bbc 100644 --- a/src/DomainWrap.cpp +++ b/src/DomainWrap.cpp @@ -2,266 +2,310 @@ #include "NodeWrap.h" #include "DomainWrap.h" #include "Error.h" +#include "Exception.h"
-#include "ArgsDomainMigrate.h" -#include "ArgsDomainRestore.h" -#include "ArgsDomainSave.h" -#include "ArgsDomainGetXMLDesc.h" +#include <cstdio>
-namespace _qmf = qmf::com::redhat::libvirt;
-DomainWrap::DomainWrap(ManagementAgent *agent, NodeWrap *parent, - virDomainPtr _domain_ptr, virConnectPtr _conn) : - domain_ptr(_domain_ptr), conn(_conn) +DomainWrap::DomainWrap( + NodeWrap *parent, + virDomainPtr domain_ptr, + virConnectPtr conn): + PackageOwner<NodeWrap::PackageDefinition>(parent), + ManagedObject(package().data_Domain), + _domain_ptr(domain_ptr), _conn(conn) { char dom_uuid[VIR_UUID_STRING_BUFLEN];
- domain = NULL; - - if (virDomainGetUUIDString(domain_ptr, dom_uuid) < 0) { - REPORT_ERR(conn, "DomainWrap: Unable to get UUID string of domain."); + if (virDomainGetUUIDString(_domain_ptr, dom_uuid) < 0) { + REPORT_ERR(_conn, "DomainWrap: Unable to get UUID string of domain."); throw 1;
I've noticed that most (all?) objects are being passed in a virConnectPtr instance too. This should be redundant for two reasons
- If you have a virDomainPtr, you can call virDomainGetConnect() to get back the associated virConnectPtr (likewise for other libvirt objects) - We should kill the 'virConnectPtr' parameter from the REPORT_ERR macro/functions, and use 'virGetLastError' exclusively. This function is thread-safe, unlike the current virConnGetLastError being used.
Since this problem was pre-existing, I'll leave it upto you whether you want to incorporate such a change into this patch or do it as a later followup.
Personally I'd prefer it be in a follow-up patch.
void DomainWrap::update() { virDomainInfo info; int ret;
- ret = virDomainGetInfo(domain_ptr, &info); + ret = virDomainGetInfo(_domain_ptr, &info); if (ret < 0) { - REPORT_ERR(conn, "Domain get info failed."); + REPORT_ERR(_conn, "Domain get info failed."); /* Next domainSync() will take care of this in the node wrapper if the domain is * indeed dead. */ return; } else { + const char *state = NULL; switch (info.state) { - case VIR_DOMAIN_NOSTATE:
Add a 'default:' clause here
- domain->set_state("nostate"); + state = "nostate"; break; case VIR_DOMAIN_RUNNING: - domain->set_state("running"); + state = "running"; break; case VIR_DOMAIN_BLOCKED: - domain->set_state("blocked"); + state = "blocked"; break; case VIR_DOMAIN_PAUSED: - domain->set_state("paused"); + state = "paused"; break; case VIR_DOMAIN_SHUTDOWN: - domain->set_state("shutdown"); + state = "shutdown"; break; case VIR_DOMAIN_SHUTOFF: - domain->set_state("shutoff"); + state = "shutoff"; break; case VIR_DOMAIN_CRASHED: - domain->set_state("crashed"); + state = "crashed"; break; }
- domain->set_numVcpus(info.nrVirtCpu); - domain->set_maximumMemory(info.maxMem); - domain->set_memory(info.memory); - domain->set_cpuTime(info.cpuTime); + if (state) { + _data.setProperty("state", state); + }
We shouldn't skip out 'NOSTATE' here, juust make this unconditional
+ _data.setProperty("numVcpus", info.nrVirtCpu); + _data.setProperty("maximumMemory", info.maxMem); + _data.setProperty("memory", (uint64_t)info.memory); + _data.setProperty("cpuTime", (uint64_t)info.cpuTime); }
+bool +DomainWrap::migrate(qmf::AgentSession& session, qmf::AgentEvent& event) +{ + virConnectPtr dest_conn; + virDomainPtr rem_dom; + qpid::types::Variant::Map args(event.getArguments()); + int ret; + + // This is actually quite broken. Most setups won't allow unauthorized + // connection from one node to another directly like this. + dest_conn = virConnectOpen(args["destinationUri"].asString().c_str());
Since the agent was originally written, we now have a new migration API which avoids the need for a destination virConnectPtr object. Instead you can pass the 'destinationUri' directly to virDomainMigrateToURI, and set the PEER2PEER flag.
NB, this still presumes that the source libvirtd can auth with the target libvirtd directly, but it avoids the need for matahari itself to authenticate. Typically we'd expect the libvirtd's to be setup with TLS+x590 or Kerberos so they can talk to each other
+ if (!dest_conn) { + std::string err = FORMAT_ERR(dest_conn, "Unable to connect to remote system for migration: virConnectOpen", &ret); + raiseException(session, event, err, STATUS_USER + ret); + return false; + } + + const std::string newDomainName_arg(args["newDomainName"]); + const char *new_dom_name = NULL; + if (newDomainName_arg.size() > 0) { + new_dom_name = newDomainName_arg.c_str(); + } + + const std::string uri_arg(args["uri"]); + const char *uri = NULL; + if (uri_arg.size() > 0) { + uri = uri_arg.c_str(); + } + + uint32_t flags(args["flags"]); + uint32_t bandwidth(args["bandwidth"]); + + printf ("calling migrate, new_dom_name: %s, uri: %s, flags: %d (live is %d)\n", + new_dom_name ? new_dom_name : "NULL", + uri ? uri : "NULL", + flags, + VIR_MIGRATE_LIVE); + + rem_dom = virDomainMigrate(_domain_ptr, dest_conn, flags, + new_dom_name, + uri, + bandwidth);
Ideally you want virDomainMigrateToURI, or even better virDomainMigrateToURI2 here.
No objection at all to making these sorts of changes, lets just do them as a separate work item.
diff --git a/src/Error.h b/src/Error.h index 388b41a..5bd7397 100644 --- a/src/Error.h +++ b/src/Error.h @@ -1,11 +1,19 @@ +#ifndef ERROR_H +#define ERROR_H + +#include <libvirt/libvirt.h> +#include <libvirt/virterror.h> +
#define REPORT_ERR(conn,msg) reportError(conn, msg, __func__, __LINE__, __FILE__)
#define FORMAT_ERR(conn,msg,err_ret) formatError(conn, msg, err_ret, __func__, __LINE__, __FILE__)
+ void reportError(virConnectPtr conn, const char *msg, const char *function, int line, const char *file);
std::string formatError(virConnectPtr conn, const char *msg, int *err_ret, const char *function, int line, const char *file);
+#endif
We want to remove virConnectPtr from both these methods & macros
diff --git a/src/Exception.cpp b/src/Exception.cpp new file mode 100644 index 0000000..105df27 --- /dev/null +++ b/src/Exception.cpp @@ -0,0 +1,38 @@ +#include "Exception.h" +#include "Error.h" +#include <qmf/Schema.h> +#include <qmf/SchemaProperty.h> +#include <qmf/Data.h> + + +#define ERROR_TEXT "error_text" +#define ERROR_CODE "error_code" + + +static qmf::Schema errorSchema(qmf::SCHEMA_TYPE_DATA, + "com.redhat.libvirt", "error");
This should be 'org.libvirt' really.
generated_file_list = \ - qmf/com/redhat/libvirt/Domain.cpp\ - qmf/com/redhat/libvirt/Node.cpp\ - qmf/com/redhat/libvirt/Package.cpp\ - qmf/com/redhat/libvirt/Pool.cpp\ - qmf/com/redhat/libvirt/Volume.cpp\ - qmf/com/redhat/libvirt/ArgsDomainGetXMLDesc.h\ - qmf/com/redhat/libvirt/ArgsDomainMigrate.h\ - qmf/com/redhat/libvirt/ArgsDomainRestore.h\ - qmf/com/redhat/libvirt/ArgsDomainSave.h\ - qmf/com/redhat/libvirt/ArgsNodeDomainDefineXML.h\ - qmf/com/redhat/libvirt/ArgsNodeStoragePoolCreateXML.h\ - qmf/com/redhat/libvirt/ArgsNodeStoragePoolDefineXML.h\ - qmf/com/redhat/libvirt/ArgsPoolCreateVolumeXML.h\ - qmf/com/redhat/libvirt/ArgsPoolGetXMLDesc.h\ - qmf/com/redhat/libvirt/ArgsVolumeGetXMLDesc.h\ - qmf/com/redhat/libvirt/Domain.h\ - qmf/com/redhat/libvirt/Node.h\ - qmf/com/redhat/libvirt/Package.h\ - qmf/com/redhat/libvirt/Pool.h\ - qmf/com/redhat/libvirt/Volume.h + qmf/com/redhat/libvirt/QmfPackage.cpp\ + qmf/com/redhat/libvirt/QmfPackage.h
Can we change these to org/libvirt too, or will that be an incompatible schema change ?
I believe it would be. Ted would know for sure.
Incidentally, is this fully backwards compatible from the POV of existing libvirt-qpid client apps, or are we starting a v2 libvirt schema here ?
I believe he has some tests which both versions pass. So I'm pretty sure the API is more than backwards compatible, it should be identical.
static void @@ -521,7 +577,6 @@ print_usage() printf("\t-d | --daemon run as a daemon.\n"); printf("\t-h | --help print this help message.\n"); printf("\t-b | --broker specify broker host name..\n"); - printf("\t-g | --gssapi force GSSAPI authentication.\n"); printf("\t-u | --username username to use for authentication purproses.\n"); printf("\t-s | --service service name to use for authentication purproses.\n"); printf("\t-p | --port specify broker port.\n"); @@ -536,8 +591,7 @@ int main(int argc, char** argv) { int idx = 0; bool verbose = false; bool daemonize = false; - bool gssapi = false; - char *host = NULL; + const char *host = NULL; char *username = NULL; char *service = NULL; int port = 5672; @@ -546,14 +600,13 @@ int main(int argc, char** argv) { {"help", 0, 0, 'h'}, {"daemon", 0, 0, 'd'}, {"broker", 1, 0, 'b'}, - {"gssapi", 0, 0, 'g'}, {"username", 1, 0, 'u'}, {"service", 1, 0, 's'}, {"port", 1, 0, 'p'}, {0, 0, 0, 0} };
- while ((arg = getopt_long(argc, argv, "hdb:gu:s:p:", opt, &idx)) != -1) { + while ((arg = getopt_long(argc, argv, "hdb:u:s:p:", opt, &idx)) != -1) { switch (arg) { case 'h': case '?': @@ -582,9 +635,6 @@ int main(int argc, char** argv) { exit(1); } break; - case 'g': - gssapi = true; - break; case 'p': if (optarg) { port = atoi(optarg); @@ -620,44 +670,42 @@ int main(int argc, char** argv) { // This prevents us from dying if libvirt disconnects. signal(SIGPIPE, SIG_IGN);
- // Create the management agent - ManagementAgent::Singleton singleton; - ManagementAgent* agent = singleton.getInstance(); - - // Register the schema with the agent - _qmf::Package packageInit(agent); - - // Start the agent. It will attempt to make a connection to the - // management broker. The third argument is the interval for sending - // updates to stats/properties to the broker. The last is set to 'true' - // to keep this all single threaded. Otherwise a second thread would be - // used to implement methods. - - qpid::management::ConnectionSettings settings; - settings.host = host ? host : "127.0.0.1"; - settings.port = port; + qpid::types::Variant::Map options;
if (username != NULL) { - settings.username = username; + options["username"] = username; } if (service != NULL) { - settings.service = service; + options["service"] = service; } - if (gssapi == true) { - settings.mechanism = "GSSAPI"; + + if (host == NULL) { + host = "127.0.0.1"; }
- agent->setName("Red Hat", "libvirt-qpid"); - agent->init(settings, 3, true); + std::stringstream url; + + url << "amqp:" << "tcp" << ":" << host << ":" << port; + + qpid::messaging::Connection amqp_connection(url.str(), options); + amqp_connection.open(); + + qmf::AgentSession session(amqp_connection); + session.setVendor("redhat.com");
IMHO, this should really be 'org.libvirt' too
+ session.setProduct("libvirt-qpid"); + session.setAttribute("hostname", host);
Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| _______________________________________________ Matahari mailing list Matahari@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/matahari
Hi Daniel, Thanks for the very helpful (and prompt!) feedback. Responses inline. cheers, Zane. On 13/07/11 08:49, Andrew Beekhof wrote:
On Wed, Jul 13, 2011 at 2:46 AM, Daniel P. Berrange<berrange@redhat.com> wrote:
On Tue, Jul 12, 2011 at 06:28:46PM +0200, Zane Bitter wrote:
diff --git a/src/DomainWrap.cpp b/src/DomainWrap.cpp index 47876de..eab6bbc 100644 --- a/src/DomainWrap.cpp +++ b/src/DomainWrap.cpp @@ -2,266 +2,310 @@ #include "NodeWrap.h" #include "DomainWrap.h" #include "Error.h" +#include "Exception.h"
-#include "ArgsDomainMigrate.h" -#include "ArgsDomainRestore.h" -#include "ArgsDomainSave.h" -#include "ArgsDomainGetXMLDesc.h" +#include<cstdio>
-namespace _qmf = qmf::com::redhat::libvirt;
-DomainWrap::DomainWrap(ManagementAgent *agent, NodeWrap *parent, - virDomainPtr _domain_ptr, virConnectPtr _conn) : - domain_ptr(_domain_ptr), conn(_conn) +DomainWrap::DomainWrap( + NodeWrap *parent, + virDomainPtr domain_ptr, + virConnectPtr conn): + PackageOwner<NodeWrap::PackageDefinition>(parent), + ManagedObject(package().data_Domain), + _domain_ptr(domain_ptr), _conn(conn) { char dom_uuid[VIR_UUID_STRING_BUFLEN];
- domain = NULL; - - if (virDomainGetUUIDString(domain_ptr, dom_uuid)< 0) { - REPORT_ERR(conn, "DomainWrap: Unable to get UUID string of domain."); + if (virDomainGetUUIDString(_domain_ptr, dom_uuid)< 0) { + REPORT_ERR(_conn, "DomainWrap: Unable to get UUID string of domain."); throw 1;
I've noticed that most (all?) objects are being passed in a virConnectPtr instance too. This should be redundant for two reasons
- If you have a virDomainPtr, you can call virDomainGetConnect() to get back the associated virConnectPtr (likewise for other libvirt objects) - We should kill the 'virConnectPtr' parameter from the REPORT_ERR macro/functions, and use 'virGetLastError' exclusively. This function is thread-safe, unlike the current virConnGetLastError being used.
Since this problem was pre-existing, I'll leave it upto you whether you want to incorporate such a change into this patch or do it as a later followup.
Personally I'd prefer it be in a follow-up patch.
Agree, this is a good candidate for a follow-up.
void DomainWrap::update() { virDomainInfo info; int ret;
- ret = virDomainGetInfo(domain_ptr,&info); + ret = virDomainGetInfo(_domain_ptr,&info); if (ret< 0) { - REPORT_ERR(conn, "Domain get info failed."); + REPORT_ERR(_conn, "Domain get info failed."); /* Next domainSync() will take care of this in the node wrapper if the domain is * indeed dead. */ return; } else { + const char *state = NULL; switch (info.state) { - case VIR_DOMAIN_NOSTATE:
Add a 'default:' clause here
- domain->set_state("nostate"); + state = "nostate"; break; case VIR_DOMAIN_RUNNING: - domain->set_state("running"); + state = "running"; break; case VIR_DOMAIN_BLOCKED: - domain->set_state("blocked"); + state = "blocked"; break; case VIR_DOMAIN_PAUSED: - domain->set_state("paused"); + state = "paused"; break; case VIR_DOMAIN_SHUTDOWN: - domain->set_state("shutdown"); + state = "shutdown"; break; case VIR_DOMAIN_SHUTOFF: - domain->set_state("shutoff"); + state = "shutoff"; break; case VIR_DOMAIN_CRASHED: - domain->set_state("crashed"); + state = "crashed"; break; }
- domain->set_numVcpus(info.nrVirtCpu); - domain->set_maximumMemory(info.maxMem); - domain->set_memory(info.memory); - domain->set_cpuTime(info.cpuTime); + if (state) { + _data.setProperty("state", state); + }
We shouldn't skip out 'NOSTATE' here, juust make this unconditional
+ _data.setProperty("numVcpus", info.nrVirtCpu); + _data.setProperty("maximumMemory", info.maxMem); + _data.setProperty("memory", (uint64_t)info.memory); + _data.setProperty("cpuTime", (uint64_t)info.cpuTime); }
Fixed. Thanks! There is a similar issue with the state of a Pool (as written it will segfault if a state other than inactive/building/running/degraded is returned). What would be an appropriate default there? Just set it to "unknown"?
+bool +DomainWrap::migrate(qmf::AgentSession& session, qmf::AgentEvent& event) +{ + virConnectPtr dest_conn; + virDomainPtr rem_dom; + qpid::types::Variant::Map args(event.getArguments()); + int ret; + + // This is actually quite broken. Most setups won't allow unauthorized + // connection from one node to another directly like this. + dest_conn = virConnectOpen(args["destinationUri"].asString().c_str());
Since the agent was originally written, we now have a new migration API which avoids the need for a destination virConnectPtr object. Instead you can pass the 'destinationUri' directly to virDomainMigrateToURI, and set the PEER2PEER flag.
NB, this still presumes that the source libvirtd can auth with the target libvirtd directly, but it avoids the need for matahari itself to authenticate. Typically we'd expect the libvirtd's to be setup with TLS+x590 or Kerberos so they can talk to each other
+ if (!dest_conn) { + std::string err = FORMAT_ERR(dest_conn, "Unable to connect to remote system for migration: virConnectOpen",&ret); + raiseException(session, event, err, STATUS_USER + ret); + return false; + } + + const std::string newDomainName_arg(args["newDomainName"]); + const char *new_dom_name = NULL; + if (newDomainName_arg.size()> 0) { + new_dom_name = newDomainName_arg.c_str(); + } + + const std::string uri_arg(args["uri"]); + const char *uri = NULL; + if (uri_arg.size()> 0) { + uri = uri_arg.c_str(); + } + + uint32_t flags(args["flags"]); + uint32_t bandwidth(args["bandwidth"]); + + printf ("calling migrate, new_dom_name: %s, uri: %s, flags: %d (live is %d)\n", + new_dom_name ? new_dom_name : "NULL", + uri ? uri : "NULL", + flags, + VIR_MIGRATE_LIVE); + + rem_dom = virDomainMigrate(_domain_ptr, dest_conn, flags, + new_dom_name, + uri, + bandwidth);
Ideally you want virDomainMigrateToURI, or even better virDomainMigrateToURI2 here.
No objection at all to making these sorts of changes, lets just do them as a separate work item.
Yep.
diff --git a/src/Error.h b/src/Error.h index 388b41a..5bd7397 100644 --- a/src/Error.h +++ b/src/Error.h @@ -1,11 +1,19 @@ +#ifndef ERROR_H +#define ERROR_H + +#include<libvirt/libvirt.h> +#include<libvirt/virterror.h> +
#define REPORT_ERR(conn,msg) reportError(conn, msg, __func__, __LINE__, __FILE__)
#define FORMAT_ERR(conn,msg,err_ret) formatError(conn, msg, err_ret, __func__, __LINE__, __FILE__)
+ void reportError(virConnectPtr conn, const char *msg, const char *function, int line, const char *file);
std::string formatError(virConnectPtr conn, const char *msg, int *err_ret, const char *function, int line, const char *file);
+#endif
We want to remove virConnectPtr from both these methods& macros
diff --git a/src/Exception.cpp b/src/Exception.cpp new file mode 100644 index 0000000..105df27 --- /dev/null +++ b/src/Exception.cpp @@ -0,0 +1,38 @@ +#include "Exception.h" +#include "Error.h" +#include<qmf/Schema.h> +#include<qmf/SchemaProperty.h> +#include<qmf/Data.h> + + +#define ERROR_TEXT "error_text" +#define ERROR_CODE "error_code" + + +static qmf::Schema errorSchema(qmf::SCHEMA_TYPE_DATA, + "com.redhat.libvirt", "error");
This should be 'org.libvirt' really.
Fixed.
generated_file_list = \ - qmf/com/redhat/libvirt/Domain.cpp\ - qmf/com/redhat/libvirt/Node.cpp\ - qmf/com/redhat/libvirt/Package.cpp\ - qmf/com/redhat/libvirt/Pool.cpp\ - qmf/com/redhat/libvirt/Volume.cpp\ - qmf/com/redhat/libvirt/ArgsDomainGetXMLDesc.h\ - qmf/com/redhat/libvirt/ArgsDomainMigrate.h\ - qmf/com/redhat/libvirt/ArgsDomainRestore.h\ - qmf/com/redhat/libvirt/ArgsDomainSave.h\ - qmf/com/redhat/libvirt/ArgsNodeDomainDefineXML.h\ - qmf/com/redhat/libvirt/ArgsNodeStoragePoolCreateXML.h\ - qmf/com/redhat/libvirt/ArgsNodeStoragePoolDefineXML.h\ - qmf/com/redhat/libvirt/ArgsPoolCreateVolumeXML.h\ - qmf/com/redhat/libvirt/ArgsPoolGetXMLDesc.h\ - qmf/com/redhat/libvirt/ArgsVolumeGetXMLDesc.h\ - qmf/com/redhat/libvirt/Domain.h\ - qmf/com/redhat/libvirt/Node.h\ - qmf/com/redhat/libvirt/Package.h\ - qmf/com/redhat/libvirt/Pool.h\ - qmf/com/redhat/libvirt/Volume.h + qmf/com/redhat/libvirt/QmfPackage.cpp\ + qmf/com/redhat/libvirt/QmfPackage.h
Can we change these to org/libvirt too, or will that be an incompatible schema change ?
I believe it would be. Ted would know for sure.
Incidentally, is this fully backwards compatible from the POV of existing libvirt-qpid client apps, or are we starting a v2 libvirt schema here ?
I believe he has some tests which both versions pass. So I'm pretty sure the API is more than backwards compatible, it should be identical.
That is correct (I will eventually post the test tool here too), and it goes some way to proving that it is, for now, possible to write a client that works with both the old and new versions. What it does not tell us is whether any client written against the previous version will work with the new matahari version. There are definite differences you can see just by playing around with qmf-tool. For instance, with v1 you don't need to specify the package (the com.redhat.libvirt part) when you do a query, but with v2 you do. Then there's that bizarre thing where v1 appears to convert the class names to lower case, but v2 doesn't (which patch #2 fixes). There may be others. The number of hoops to jump through just to return error codes was... surprising. From the other discussion in this thread it sounds like now is a good time to break backwards compatibility, change the package name to org.libvirt and drop that second patch (for consistency with other matahari agents).
static void @@ -521,7 +577,6 @@ print_usage() printf("\t-d | --daemon run as a daemon.\n"); printf("\t-h | --help print this help message.\n"); printf("\t-b | --broker specify broker host name..\n"); - printf("\t-g | --gssapi force GSSAPI authentication.\n"); printf("\t-u | --username username to use for authentication purproses.\n"); printf("\t-s | --service service name to use for authentication purproses.\n"); printf("\t-p | --port specify broker port.\n"); @@ -536,8 +591,7 @@ int main(int argc, char** argv) { int idx = 0; bool verbose = false; bool daemonize = false; - bool gssapi = false; - char *host = NULL; + const char *host = NULL; char *username = NULL; char *service = NULL; int port = 5672; @@ -546,14 +600,13 @@ int main(int argc, char** argv) { {"help", 0, 0, 'h'}, {"daemon", 0, 0, 'd'}, {"broker", 1, 0, 'b'}, - {"gssapi", 0, 0, 'g'}, {"username", 1, 0, 'u'}, {"service", 1, 0, 's'}, {"port", 1, 0, 'p'}, {0, 0, 0, 0} };
- while ((arg = getopt_long(argc, argv, "hdb:gu:s:p:", opt,&idx)) != -1) { + while ((arg = getopt_long(argc, argv, "hdb:u:s:p:", opt,&idx)) != -1) { switch (arg) { case 'h': case '?': @@ -582,9 +635,6 @@ int main(int argc, char** argv) { exit(1); } break; - case 'g': - gssapi = true; - break; case 'p': if (optarg) { port = atoi(optarg); @@ -620,44 +670,42 @@ int main(int argc, char** argv) { // This prevents us from dying if libvirt disconnects. signal(SIGPIPE, SIG_IGN);
- // Create the management agent - ManagementAgent::Singleton singleton; - ManagementAgent* agent = singleton.getInstance(); - - // Register the schema with the agent - _qmf::Package packageInit(agent); - - // Start the agent. It will attempt to make a connection to the - // management broker. The third argument is the interval for sending - // updates to stats/properties to the broker. The last is set to 'true' - // to keep this all single threaded. Otherwise a second thread would be - // used to implement methods. - - qpid::management::ConnectionSettings settings; - settings.host = host ? host : "127.0.0.1"; - settings.port = port; + qpid::types::Variant::Map options;
if (username != NULL) { - settings.username = username; + options["username"] = username; } if (service != NULL) { - settings.service = service; + options["service"] = service; } - if (gssapi == true) { - settings.mechanism = "GSSAPI"; + + if (host == NULL) { + host = "127.0.0.1"; }
- agent->setName("Red Hat", "libvirt-qpid"); - agent->init(settings, 3, true); + std::stringstream url; + + url<< "amqp:"<< "tcp"<< ":"<< host<< ":"<< port; + + qpid::messaging::Connection amqp_connection(url.str(), options); + amqp_connection.open(); + + qmf::AgentSession session(amqp_connection); + session.setVendor("redhat.com");
IMHO, this should really be 'org.libvirt' too
FWIW once this becomes a matahari agent this gets changed to "matahariproject.org" anyway whether we like it or not. (Perhaps matahari ought to make this configurable?) In fact, the previous vendor name was actually "Red Hat", and I had to change it because having a space in it borked something in v2 (I forget what). Since backward compatibility with anything relying on the agent vendor is going to be broken anyway, I've gone ahead and changed this patch to make it "libvirt.org".
+ session.setProduct("libvirt-qpid"); + session.setAttribute("hostname", host);
Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| _______________________________________________ Matahari mailing list Matahari@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/matahari
This schema change is necessary to ensure that classes keep the same names after changing from the QMF to the QMFv2 API. --- src/DomainWrap.cpp | 2 +- src/NodeWrap.cpp | 2 +- src/PoolWrap.cpp | 2 +- src/VolumeWrap.cpp | 2 +- src/libvirt-schema.xml | 8 ++++---- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/DomainWrap.cpp b/src/DomainWrap.cpp index eab6bbc..a72d970 100644 --- a/src/DomainWrap.cpp +++ b/src/DomainWrap.cpp @@ -12,7 +12,7 @@ DomainWrap::DomainWrap( virDomainPtr domain_ptr, virConnectPtr conn): PackageOwner<NodeWrap::PackageDefinition>(parent), - ManagedObject(package().data_Domain), + ManagedObject(package().data_domain), _domain_ptr(domain_ptr), _conn(conn) { char dom_uuid[VIR_UUID_STRING_BUFLEN]; diff --git a/src/NodeWrap.cpp b/src/NodeWrap.cpp index d65db11..40c4339 100644 --- a/src/NodeWrap.cpp +++ b/src/NodeWrap.cpp @@ -16,7 +16,7 @@ NodeWrap::NodeWrap(qmf::AgentSession& agent_session, PackageDefinition& package): - ManagedObject(package.data_Node), + ManagedObject(package.data_node), _session(agent_session), _package(package) { diff --git a/src/PoolWrap.cpp b/src/PoolWrap.cpp index a5992f2..dee2597 100644 --- a/src/PoolWrap.cpp +++ b/src/PoolWrap.cpp @@ -13,7 +13,7 @@ PoolWrap::PoolWrap(NodeWrap *parent, virStoragePoolPtr pool_ptr, virConnectPtr connect): PackageOwner<NodeWrap::PackageDefinition>(parent), - ManagedObject(package().data_Pool), + ManagedObject(package().data_pool), _pool_ptr(pool_ptr), _conn(connect) { int ret; diff --git a/src/VolumeWrap.cpp b/src/VolumeWrap.cpp index fcda96e..59b21d8 100644 --- a/src/VolumeWrap.cpp +++ b/src/VolumeWrap.cpp @@ -16,7 +16,7 @@ VolumeWrap::VolumeWrap(PoolWrap *parent, virStorageVolPtr volume_ptr, virConnectPtr connect): PackageOwner<PoolWrap::PackageDefinition>(parent), - ManagedObject(package().data_Volume), + ManagedObject(package().data_volume), _volume_ptr(volume_ptr), _conn(connect), _lvm_name(""), _has_lvm_child(false), _wrap_parent(parent) diff --git a/src/libvirt-schema.xml b/src/libvirt-schema.xml index a1b88e1..6fbca76 100644 --- a/src/libvirt-schema.xml +++ b/src/libvirt-schema.xml @@ -7,7 +7,7 @@ <!-- In libvirt this is really the 'Connect' class, I may end up changing it.. --> - <class name="Node"> + <class name="node"> <property name="hostname" type="sstr" access="RC" desc="Host name" index="y"/> <property name="uri" type="sstr" access="RC" desc="URI of libvirt"/> <property name="libvirtVersion" type="sstr" access="RC" desc="Version of libvirt on the managed node"/> @@ -54,7 +54,7 @@ </method> </class> - <class name="Domain"> + <class name="domain"> <property name="uuid" access="RC" type="sstr" desc="Domain UUID" index="y"/> <property name="name" access="RC" type="sstr" desc="Domain name" index="y"/> <property name="id" type="int64" desc="Hypervisor Domain id, -1 if not running."/> @@ -109,7 +109,7 @@ </method> </class> - <class name="Pool"> + <class name="pool"> <property name="uuid" access="RC" type="sstr" desc="Pool UUID" index="y"/> <property name="name" access="RC" type="sstr" desc="Pool name" index="y"/> <property name="parentVolume" access="RC" type="sstr" desc="If this pool is an LVM pool, this will contain the parent volume path."/> @@ -154,7 +154,7 @@ </class> - <class name="Volume"> + <class name="volume"> <property name="key" type="sstr" access="RC" desc="Storage volume key" index="y"/> <property name="path" type="sstr" access="RC" desc="Storage volume path" index="y"/> <property name="name" type="sstr" access="RC" desc="Storage volume name" index="y"/>
On Wed, Jul 13, 2011 at 2:29 AM, Zane Bitter <zbitter@redhat.com> wrote:
This schema change is necessary to ensure that classes keep the same names after changing from the QMF to the QMFv2 API.
Odd. But if you say so, ACK :-)
--- src/DomainWrap.cpp | 2 +- src/NodeWrap.cpp | 2 +- src/PoolWrap.cpp | 2 +- src/VolumeWrap.cpp | 2 +- src/libvirt-schema.xml | 8 ++++---- 5 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/src/DomainWrap.cpp b/src/DomainWrap.cpp index eab6bbc..a72d970 100644 --- a/src/DomainWrap.cpp +++ b/src/DomainWrap.cpp @@ -12,7 +12,7 @@ DomainWrap::DomainWrap( virDomainPtr domain_ptr, virConnectPtr conn): PackageOwner<NodeWrap::PackageDefinition>(parent), - ManagedObject(package().data_Domain), + ManagedObject(package().data_domain), _domain_ptr(domain_ptr), _conn(conn) { char dom_uuid[VIR_UUID_STRING_BUFLEN]; diff --git a/src/NodeWrap.cpp b/src/NodeWrap.cpp index d65db11..40c4339 100644 --- a/src/NodeWrap.cpp +++ b/src/NodeWrap.cpp @@ -16,7 +16,7 @@
NodeWrap::NodeWrap(qmf::AgentSession& agent_session, PackageDefinition& package): - ManagedObject(package.data_Node), + ManagedObject(package.data_node), _session(agent_session), _package(package) { diff --git a/src/PoolWrap.cpp b/src/PoolWrap.cpp index a5992f2..dee2597 100644 --- a/src/PoolWrap.cpp +++ b/src/PoolWrap.cpp @@ -13,7 +13,7 @@ PoolWrap::PoolWrap(NodeWrap *parent, virStoragePoolPtr pool_ptr, virConnectPtr connect): PackageOwner<NodeWrap::PackageDefinition>(parent), - ManagedObject(package().data_Pool), + ManagedObject(package().data_pool), _pool_ptr(pool_ptr), _conn(connect) { int ret; diff --git a/src/VolumeWrap.cpp b/src/VolumeWrap.cpp index fcda96e..59b21d8 100644 --- a/src/VolumeWrap.cpp +++ b/src/VolumeWrap.cpp @@ -16,7 +16,7 @@ VolumeWrap::VolumeWrap(PoolWrap *parent, virStorageVolPtr volume_ptr, virConnectPtr connect): PackageOwner<PoolWrap::PackageDefinition>(parent), - ManagedObject(package().data_Volume), + ManagedObject(package().data_volume), _volume_ptr(volume_ptr), _conn(connect), _lvm_name(""), _has_lvm_child(false), _wrap_parent(parent) diff --git a/src/libvirt-schema.xml b/src/libvirt-schema.xml index a1b88e1..6fbca76 100644 --- a/src/libvirt-schema.xml +++ b/src/libvirt-schema.xml @@ -7,7 +7,7 @@
<!-- In libvirt this is really the 'Connect' class, I may end up changing it.. -->
- <class name="Node"> + <class name="node"> <property name="hostname" type="sstr" access="RC" desc="Host name" index="y"/> <property name="uri" type="sstr" access="RC" desc="URI of libvirt"/> <property name="libvirtVersion" type="sstr" access="RC" desc="Version of libvirt on the managed node"/> @@ -54,7 +54,7 @@ </method> </class>
- <class name="Domain"> + <class name="domain"> <property name="uuid" access="RC" type="sstr" desc="Domain UUID" index="y"/> <property name="name" access="RC" type="sstr" desc="Domain name" index="y"/> <property name="id" type="int64" desc="Hypervisor Domain id, -1 if not running."/> @@ -109,7 +109,7 @@ </method> </class>
- <class name="Pool"> + <class name="pool"> <property name="uuid" access="RC" type="sstr" desc="Pool UUID" index="y"/> <property name="name" access="RC" type="sstr" desc="Pool name" index="y"/> <property name="parentVolume" access="RC" type="sstr" desc="If this pool is an LVM pool, this will contain the parent volume path."/> @@ -154,7 +154,7 @@
</class>
- <class name="Volume"> + <class name="volume"> <property name="key" type="sstr" access="RC" desc="Storage volume key" index="y"/> <property name="path" type="sstr" access="RC" desc="Storage volume path" index="y"/> <property name="name" type="sstr" access="RC" desc="Storage volume name" index="y"/>
_______________________________________________ Matahari mailing list Matahari@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/matahari
--- AUTHORS | 2 configure.ac | 1 libvirt-qpid.spec | 2 src/LibvirtAgent.cpp | 97 ++++++++++++++++++++++++ src/LibvirtAgent.h | 36 +++++++++ src/Makefile.am | 7 +- src/NodeWrap.cpp | 203 +++++--------------------------------------------- src/NodeWrap.h | 22 +---- 8 files changed, 169 insertions(+), 201 deletions(-) create mode 100644 src/LibvirtAgent.cpp create mode 100644 src/LibvirtAgent.h diff --git a/AUTHORS b/AUTHORS index 367ea49..8af5aba 100644 --- a/AUTHORS +++ b/AUTHORS @@ -9,6 +9,6 @@ Patches have also been contributed by: Ted Ross <tross@redhat.com> -Conversion to QMFv2 API by: +Conversion to matahari agent and QMFv2 API by: Zane Bitter <zbitter@redhat.com> diff --git a/configure.ac b/configure.ac index 3896d9d..03bbb80 100644 --- a/configure.ac +++ b/configure.ac @@ -21,6 +21,7 @@ dnl Required minimum versions of all libs we depend on LIBVIRT_REQUIRED="4.4.0" PKG_CHECK_MODULES(XML, libxml-2.0) +PKG_CHECK_MODULES([DEPS], [glib-2.0]) AC_OUTPUT(Makefile src/Makefile doc/Makefile) diff --git a/libvirt-qpid.spec b/libvirt-qpid.spec index d64552d..c8a88fd 100644 --- a/libvirt-qpid.spec +++ b/libvirt-qpid.spec @@ -10,6 +10,7 @@ Requires: libxml2 >= 2.7.1 Requires: qmf >= 0.5.790661 Requires: qpid-cpp-client >= 0.10 Requires: libvirt >= 0.4.4 +Requires: matahari-lib >= 0.4.1 Requires(post): /sbin/chkconfig Requires(preun): /sbin/chkconfig Requires(preun): initscripts @@ -17,6 +18,7 @@ BuildRequires: qpid-cpp-client-devel >= 0.10 BuildRequires: libxml2-devel >= 2.7.1 BuildRequires: libvirt-devel >= 0.5.0 BuildRequires: qmf-devel >= 0.5.790661 +BuildRequires: matahari-devel >= 0.4.1 Url: http://libvirt.org/qpid %description diff --git a/src/LibvirtAgent.cpp b/src/LibvirtAgent.cpp new file mode 100644 index 0000000..deb77ca --- /dev/null +++ b/src/LibvirtAgent.cpp @@ -0,0 +1,97 @@ +#include "LibvirtAgent.h" +#include "NodeWrap.h" +#include "Exception.h" + +#include <syslog.h> + + +#define POLL_TIME_s 3 + + +static gboolean handleTimer(gpointer user_data) +{ + LibvirtAgent *agent = (LibvirtAgent *)user_data; + if (agent) { + agent->updateData(); + return TRUE; + } + + return FALSE; +} + +int +LibvirtAgent::setup(qmf::AgentSession session) +{ + _package.configure(session); + initErrorSchema(session); + + _node = new NodeWrap(this); + + _timer = g_timeout_add_seconds(POLL_TIME_s, &handleTimer, this); + + return 0; +} + +LibvirtAgent::~LibvirtAgent() +{ + if (_timer) { + g_source_remove(_timer); + } + delete _node; +} + +void +LibvirtAgent::addData(qmf::Data& data) +{ + _agent_session.addData(data); +} + +void +LibvirtAgent::delData(qmf::Data& data) +{ + _agent_session.delData(data.getAddr()); +} + +gboolean +LibvirtAgent::invoke(qmf::AgentSession session, qmf::AgentEvent event, + gpointer user_data) +{ + if (event.getType() != qmf::AGENT_METHOD) { + return TRUE; + } + + bool handled = _node->handleMethod(session, event); + if (!handled) { + raiseException(session, event, + ERROR_UNKNOWN_OBJECT, STATUS_UNKNOWN_OBJECT); + } + + return TRUE; +} + +void +LibvirtAgent::updateData(void) +{ + if (_node) { + _node->poll(); + } +} + +int +main(int argc, char **argv) +{ + LibvirtAgent agent; + + int rc = agent.init(argc, argv, "libvirt"); + + openlog("libvirt-qpid", 0, LOG_DAEMON); + + // This prevents us from dying if libvirt disconnects. + signal(SIGPIPE, SIG_IGN); + + if (rc == 0) { + agent.run(); + } + return rc; +} + diff --git a/src/LibvirtAgent.h b/src/LibvirtAgent.h new file mode 100644 index 0000000..3b2a940 --- /dev/null +++ b/src/LibvirtAgent.h @@ -0,0 +1,36 @@ +#ifndef LIBVIRT_AGENT_H +#define LIBVIRT_AGENT_H + +#include <matahari/mh_agent.h> +#include "QmfPackage.h" +#include "ManagedObject.h" + + +class NodeWrap; + + +class LibvirtAgent: + public MatahariAgent, + public PackageOwner<qmf::com::redhat::libvirt::PackageDefinition> +{ +public: + ~LibvirtAgent(); + + int setup(qmf::AgentSession session); + gboolean invoke(qmf::AgentSession session, qmf::AgentEvent event, + gpointer user_data); + + PackageDefinition& package(void) { return _package; } + void addData(qmf::Data& data); + void delData(qmf::Data& data); + + void updateData(void); + +private: + PackageDefinition _package; + NodeWrap *_node; + int _timer; +}; + +#endif + diff --git a/src/Makefile.am b/src/Makefile.am index 4bb27b2..40d9371 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1,6 +1,6 @@ ## Process this file with automake to produce Makefile.in -INCLUDES = -I$(top_srcdir)/src/qmf/com/redhat/libvirt $(XML_CFLAGS) +INCLUDES = -I$(top_srcdir)/src/qmf/com/redhat/libvirt sbin_PROGRAMS = libvirt-qpid @@ -11,6 +11,7 @@ generated_file_list = \ nodist_libvirt_qpid_SOURCES = $(generated_file_list) libvirt_qpid_SOURCES = \ + LibvirtAgent.cpp \ DomainWrap.cpp \ Error.cpp \ Exception.cpp \ @@ -18,6 +19,7 @@ libvirt_qpid_SOURCES = \ PoolWrap.cpp \ VolumeWrap.cpp \ ManagedObject.h \ + LibvirtAgent.h \ DomainWrap.h \ Error.h \ Exception.h \ @@ -33,7 +35,8 @@ $(generated_file_list): .libvirt-schema.xml.tstamp BUILT_SOURCES = $(generated_file_list) CLEANFILES = $(generated_file_list) .libvirt-schema.xml.tstamp -libvirt_qpid_LDADD = -lqmf2 -lqpidtypes -lqpidcommon -lqpidmessaging -lvirt $(XML_LIBS) +libvirt_qpid_CXXFLAGS = $(XML_CFLAGS) $(DEPS_CFLAGS) +libvirt_qpid_LDADD = -lqmf2 -lqpidtypes -lqpidcommon -lqpidmessaging -lmcommon -lmcommon_qmf -lvirt $(XML_LIBS) $(DEPS_LIBS) dist_pkgdata_DATA = libvirt-schema.xml diff --git a/src/NodeWrap.cpp b/src/NodeWrap.cpp index 40c4339..f68e666 100644 --- a/src/NodeWrap.cpp +++ b/src/NodeWrap.cpp @@ -4,7 +4,6 @@ #include <errno.h> #include <unistd.h> #include <getopt.h> -#include <syslog.h> #include <signal.h> #include <cstdio> @@ -15,10 +14,10 @@ #include "Exception.h" -NodeWrap::NodeWrap(qmf::AgentSession& agent_session, PackageDefinition& package): - ManagedObject(package.data_node), - _session(agent_session), - _package(package) +NodeWrap::NodeWrap(LibvirtAgent *agent): + PackageOwner<LibvirtAgent::PackageDefinition>(agent), + ManagedObject(package().data_node), + _agent(agent) { virNodeInfo info; char *hostname; @@ -354,46 +353,27 @@ void NodeWrap::syncPools(void) } } -void NodeWrap::doLoop() +void +NodeWrap::poll(void) { - fd_set fds; - struct timeval tv; - int retval; - - /* Go through all domains and call update() for each, letting them update - * information and statistics. */ - while (1) { - // We're using this to check to see if our connection is still good. - // I don't see any reason this call should fail unless there is some - // connection problem.. - int maxname = virConnectNumOfDefinedDomains(_conn); - if (maxname < 0) { - return; - } + // We're using this to check to see if our connection is still good. + // I don't see any reason this call should fail unless there is some + // connection problem.. + int maxname = virConnectNumOfDefinedDomains(_conn); + if (maxname < 0) { + return; + } - syncDomains(); - syncPools(); - - for (DomainList::iterator iter = _domains.begin(); - iter != _domains.end(); iter++) { - (*iter)->update(); - } - for (PoolList::iterator iter = _pools.begin(); - iter != _pools.end(); iter++) { - (*iter)->update(); - } - - qmf::AgentEvent event; - if (_session.nextEvent(event, qpid::messaging::Duration(3000))) { - if (event.getType() == qmf::AGENT_METHOD) { - bool handled = handleMethod(_session, event); - if (!handled) { - raiseException(_session, event, - ERROR_UNKNOWN_OBJECT, STATUS_UNKNOWN_OBJECT); - } - } - } + syncDomains(); + syncPools(); + for (DomainList::iterator iter = _domains.begin(); + iter != _domains.end(); iter++) { + (*iter)->update(); + } + for (PoolList::iterator iter = _pools.begin(); + iter != _pools.end(); iter++) { + (*iter)->update(); } } @@ -570,142 +550,3 @@ NodeWrap::handleMethod(qmf::AgentSession& session, qmf::AgentEvent& event) } } -static void -print_usage() -{ - printf("Usage:\tlibvirt-qpid <options>\n"); - printf("\t-d | --daemon run as a daemon.\n"); - printf("\t-h | --help print this help message.\n"); - printf("\t-b | --broker specify broker host name..\n"); - printf("\t-u | --username username to use for authentication purproses.\n"); - printf("\t-s | --service service name to use for authentication purproses.\n"); - printf("\t-p | --port specify broker port.\n"); -} - - -//============================================================== -// Main program -//============================================================== -int main(int argc, char** argv) { - int arg; - int idx = 0; - bool verbose = false; - bool daemonize = false; - const char *host = NULL; - char *username = NULL; - char *service = NULL; - int port = 5672; - - struct option opt[] = { - {"help", 0, 0, 'h'}, - {"daemon", 0, 0, 'd'}, - {"broker", 1, 0, 'b'}, - {"username", 1, 0, 'u'}, - {"service", 1, 0, 's'}, - {"port", 1, 0, 'p'}, - {0, 0, 0, 0} - }; - - while ((arg = getopt_long(argc, argv, "hdb:u:s:p:", opt, &idx)) != -1) { - switch (arg) { - case 'h': - case '?': - print_usage(); - exit(0); - break; - case 'd': - daemonize = true; - break; - case 'v': - verbose = true; - break; - case 's': - if (optarg) { - service = strdup(optarg); - } else { - print_usage(); - exit(1); - } - break; - case 'u': - if (optarg) { - username = strdup(optarg); - } else { - print_usage(); - exit(1); - } - break; - case 'p': - if (optarg) { - port = atoi(optarg); - } else { - print_usage(); - exit(1); - } - break; - case 'b': - if (optarg) { - host = strdup(optarg); - } else { - print_usage(); - exit(1); - } - break; - default: - fprintf(stderr, "unsupported option '-%c'. See --help.\n", arg); - print_usage(); - exit(0); - break; - } - } - - if (daemonize == true) { - if (daemon(0, 0) < 0) { - fprintf(stderr, "Error daemonizing: %s\n", strerror(errno)); - exit(1); - } - } - openlog("libvirt-qpid", 0, LOG_DAEMON); - - // This prevents us from dying if libvirt disconnects. - signal(SIGPIPE, SIG_IGN); - - qpid::types::Variant::Map options; - - if (username != NULL) { - options["username"] = username; - } - if (service != NULL) { - options["service"] = service; - } - - if (host == NULL) { - host = "127.0.0.1"; - } - - std::stringstream url; - - url << "amqp:" << "tcp" << ":" << host << ":" << port; - - qpid::messaging::Connection amqp_connection(url.str(), options); - amqp_connection.open(); - - qmf::AgentSession session(amqp_connection); - session.setVendor("redhat.com"); - session.setProduct("libvirt-qpid"); - session.setAttribute("hostname", host); - - session.open(); - NodeWrap::PackageDefinition package; - package.configure(session); - - initErrorSchema(session); - - while (true) { - try { - NodeWrap node(session, package); - node.doLoop(); - } catch (int err) { } - sleep(10); - } -} diff --git a/src/NodeWrap.h b/src/NodeWrap.h index 6f55061..b1ee98a 100644 --- a/src/NodeWrap.h +++ b/src/NodeWrap.h @@ -1,8 +1,7 @@ #ifndef NODE_WRAP_H #define NODE_WRAP_H -#include "ManagedObject.h" -#include "QmfPackage.h" +#include "LibvirtAgent.h" #include <unistd.h> #include <cstdlib> @@ -19,7 +18,7 @@ class DomainWrap; class PoolWrap; class NodeWrap: - public PackageOwner<qmf::com::redhat::libvirt::PackageDefinition>, + public PackageOwner<LibvirtAgent::PackageDefinition>, public ManagedObject { typedef std::vector<DomainWrap*> DomainList; @@ -30,27 +29,16 @@ class NodeWrap: virConnectPtr _conn; - qmf::AgentSession& _session; - PackageDefinition& _package; + LibvirtAgent *_agent; public: - NodeWrap(qmf::AgentSession& agent_session, PackageDefinition& package); + NodeWrap(LibvirtAgent *agent); ~NodeWrap(); - void doLoop(); + void poll(void); bool handleMethod(qmf::AgentSession& session, qmf::AgentEvent& event); - virtual PackageDefinition& package(void) { return _package; } - - virtual void addData(qmf::Data& data) { - _session.addData(data); - } - - virtual void delData(qmf::Data& data) { - _session.delData(data.getAddr()); - } - protected: void syncDomains(void); void syncPools(void);
Looks good to me. Ack On Wed, Jul 13, 2011 at 2:29 AM, Zane Bitter <zbitter@redhat.com> wrote:
--- AUTHORS | 2 configure.ac | 1 libvirt-qpid.spec | 2 src/LibvirtAgent.cpp | 97 ++++++++++++++++++++++++ src/LibvirtAgent.h | 36 +++++++++ src/Makefile.am | 7 +- src/NodeWrap.cpp | 203 +++++--------------------------------------------- src/NodeWrap.h | 22 +---- 8 files changed, 169 insertions(+), 201 deletions(-) create mode 100644 src/LibvirtAgent.cpp create mode 100644 src/LibvirtAgent.h
diff --git a/AUTHORS b/AUTHORS index 367ea49..8af5aba 100644 --- a/AUTHORS +++ b/AUTHORS @@ -9,6 +9,6 @@ Patches have also been contributed by:
Ted Ross <tross@redhat.com>
-Conversion to QMFv2 API by: +Conversion to matahari agent and QMFv2 API by:
Zane Bitter <zbitter@redhat.com> diff --git a/configure.ac b/configure.ac index 3896d9d..03bbb80 100644 --- a/configure.ac +++ b/configure.ac @@ -21,6 +21,7 @@ dnl Required minimum versions of all libs we depend on LIBVIRT_REQUIRED="4.4.0"
PKG_CHECK_MODULES(XML, libxml-2.0) +PKG_CHECK_MODULES([DEPS], [glib-2.0])
AC_OUTPUT(Makefile src/Makefile doc/Makefile)
diff --git a/libvirt-qpid.spec b/libvirt-qpid.spec index d64552d..c8a88fd 100644 --- a/libvirt-qpid.spec +++ b/libvirt-qpid.spec @@ -10,6 +10,7 @@ Requires: libxml2 >= 2.7.1 Requires: qmf >= 0.5.790661 Requires: qpid-cpp-client >= 0.10 Requires: libvirt >= 0.4.4 +Requires: matahari-lib >= 0.4.1 Requires(post): /sbin/chkconfig Requires(preun): /sbin/chkconfig Requires(preun): initscripts @@ -17,6 +18,7 @@ BuildRequires: qpid-cpp-client-devel >= 0.10 BuildRequires: libxml2-devel >= 2.7.1 BuildRequires: libvirt-devel >= 0.5.0 BuildRequires: qmf-devel >= 0.5.790661 +BuildRequires: matahari-devel >= 0.4.1 Url: http://libvirt.org/qpid
%description diff --git a/src/LibvirtAgent.cpp b/src/LibvirtAgent.cpp new file mode 100644 index 0000000..deb77ca --- /dev/null +++ b/src/LibvirtAgent.cpp @@ -0,0 +1,97 @@ +#include "LibvirtAgent.h" +#include "NodeWrap.h" +#include "Exception.h" + +#include <syslog.h> + + +#define POLL_TIME_s 3 + + +static gboolean handleTimer(gpointer user_data) +{ + LibvirtAgent *agent = (LibvirtAgent *)user_data; + if (agent) { + agent->updateData(); + return TRUE; + } + + return FALSE; +} + +int +LibvirtAgent::setup(qmf::AgentSession session) +{ + _package.configure(session); + initErrorSchema(session); + + _node = new NodeWrap(this); + + _timer = g_timeout_add_seconds(POLL_TIME_s, &handleTimer, this); + + return 0; +} + +LibvirtAgent::~LibvirtAgent() +{ + if (_timer) { + g_source_remove(_timer); + } + delete _node; +} + +void +LibvirtAgent::addData(qmf::Data& data) +{ + _agent_session.addData(data); +} + +void +LibvirtAgent::delData(qmf::Data& data) +{ + _agent_session.delData(data.getAddr()); +} + +gboolean +LibvirtAgent::invoke(qmf::AgentSession session, qmf::AgentEvent event, + gpointer user_data) +{ + if (event.getType() != qmf::AGENT_METHOD) { + return TRUE; + } + + bool handled = _node->handleMethod(session, event); + if (!handled) { + raiseException(session, event, + ERROR_UNKNOWN_OBJECT, STATUS_UNKNOWN_OBJECT); + } + + return TRUE; +} + +void +LibvirtAgent::updateData(void) +{ + if (_node) { + _node->poll(); + } +} + +int +main(int argc, char **argv) +{ + LibvirtAgent agent; + + int rc = agent.init(argc, argv, "libvirt"); + + openlog("libvirt-qpid", 0, LOG_DAEMON); + + // This prevents us from dying if libvirt disconnects. + signal(SIGPIPE, SIG_IGN); + + if (rc == 0) { + agent.run(); + } + return rc; +} + diff --git a/src/LibvirtAgent.h b/src/LibvirtAgent.h new file mode 100644 index 0000000..3b2a940 --- /dev/null +++ b/src/LibvirtAgent.h @@ -0,0 +1,36 @@ +#ifndef LIBVIRT_AGENT_H +#define LIBVIRT_AGENT_H + +#include <matahari/mh_agent.h> +#include "QmfPackage.h" +#include "ManagedObject.h" + + +class NodeWrap; + + +class LibvirtAgent: + public MatahariAgent, + public PackageOwner<qmf::com::redhat::libvirt::PackageDefinition> +{ +public: + ~LibvirtAgent(); + + int setup(qmf::AgentSession session); + gboolean invoke(qmf::AgentSession session, qmf::AgentEvent event, + gpointer user_data); + + PackageDefinition& package(void) { return _package; } + void addData(qmf::Data& data); + void delData(qmf::Data& data); + + void updateData(void); + +private: + PackageDefinition _package; + NodeWrap *_node; + int _timer; +}; + +#endif + diff --git a/src/Makefile.am b/src/Makefile.am index 4bb27b2..40d9371 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1,6 +1,6 @@ ## Process this file with automake to produce Makefile.in
-INCLUDES = -I$(top_srcdir)/src/qmf/com/redhat/libvirt $(XML_CFLAGS) +INCLUDES = -I$(top_srcdir)/src/qmf/com/redhat/libvirt
sbin_PROGRAMS = libvirt-qpid
@@ -11,6 +11,7 @@ generated_file_list = \ nodist_libvirt_qpid_SOURCES = $(generated_file_list)
libvirt_qpid_SOURCES = \ + LibvirtAgent.cpp \ DomainWrap.cpp \ Error.cpp \ Exception.cpp \ @@ -18,6 +19,7 @@ libvirt_qpid_SOURCES = \ PoolWrap.cpp \ VolumeWrap.cpp \ ManagedObject.h \ + LibvirtAgent.h \ DomainWrap.h \ Error.h \ Exception.h \ @@ -33,7 +35,8 @@ $(generated_file_list): .libvirt-schema.xml.tstamp BUILT_SOURCES = $(generated_file_list) CLEANFILES = $(generated_file_list) .libvirt-schema.xml.tstamp
-libvirt_qpid_LDADD = -lqmf2 -lqpidtypes -lqpidcommon -lqpidmessaging -lvirt $(XML_LIBS) +libvirt_qpid_CXXFLAGS = $(XML_CFLAGS) $(DEPS_CFLAGS) +libvirt_qpid_LDADD = -lqmf2 -lqpidtypes -lqpidcommon -lqpidmessaging -lmcommon -lmcommon_qmf -lvirt $(XML_LIBS) $(DEPS_LIBS)
dist_pkgdata_DATA = libvirt-schema.xml
diff --git a/src/NodeWrap.cpp b/src/NodeWrap.cpp index 40c4339..f68e666 100644 --- a/src/NodeWrap.cpp +++ b/src/NodeWrap.cpp @@ -4,7 +4,6 @@ #include <errno.h> #include <unistd.h> #include <getopt.h> -#include <syslog.h> #include <signal.h> #include <cstdio>
@@ -15,10 +14,10 @@ #include "Exception.h"
-NodeWrap::NodeWrap(qmf::AgentSession& agent_session, PackageDefinition& package): - ManagedObject(package.data_node), - _session(agent_session), - _package(package) +NodeWrap::NodeWrap(LibvirtAgent *agent): + PackageOwner<LibvirtAgent::PackageDefinition>(agent), + ManagedObject(package().data_node), + _agent(agent) { virNodeInfo info; char *hostname; @@ -354,46 +353,27 @@ void NodeWrap::syncPools(void) } }
-void NodeWrap::doLoop() +void +NodeWrap::poll(void) { - fd_set fds; - struct timeval tv; - int retval; - - /* Go through all domains and call update() for each, letting them update - * information and statistics. */ - while (1) { - // We're using this to check to see if our connection is still good. - // I don't see any reason this call should fail unless there is some - // connection problem.. - int maxname = virConnectNumOfDefinedDomains(_conn); - if (maxname < 0) { - return; - } + // We're using this to check to see if our connection is still good. + // I don't see any reason this call should fail unless there is some + // connection problem.. + int maxname = virConnectNumOfDefinedDomains(_conn); + if (maxname < 0) { + return; + }
- syncDomains(); - syncPools(); - - for (DomainList::iterator iter = _domains.begin(); - iter != _domains.end(); iter++) { - (*iter)->update(); - } - for (PoolList::iterator iter = _pools.begin(); - iter != _pools.end(); iter++) { - (*iter)->update(); - } - - qmf::AgentEvent event; - if (_session.nextEvent(event, qpid::messaging::Duration(3000))) { - if (event.getType() == qmf::AGENT_METHOD) { - bool handled = handleMethod(_session, event); - if (!handled) { - raiseException(_session, event, - ERROR_UNKNOWN_OBJECT, STATUS_UNKNOWN_OBJECT); - } - } - } + syncDomains(); + syncPools();
+ for (DomainList::iterator iter = _domains.begin(); + iter != _domains.end(); iter++) { + (*iter)->update(); + } + for (PoolList::iterator iter = _pools.begin(); + iter != _pools.end(); iter++) { + (*iter)->update(); } }
@@ -570,142 +550,3 @@ NodeWrap::handleMethod(qmf::AgentSession& session, qmf::AgentEvent& event) } }
-static void -print_usage() -{ - printf("Usage:\tlibvirt-qpid <options>\n"); - printf("\t-d | --daemon run as a daemon.\n"); - printf("\t-h | --help print this help message.\n"); - printf("\t-b | --broker specify broker host name..\n"); - printf("\t-u | --username username to use for authentication purproses.\n"); - printf("\t-s | --service service name to use for authentication purproses.\n"); - printf("\t-p | --port specify broker port.\n"); -} - - -//============================================================== -// Main program -//============================================================== -int main(int argc, char** argv) { - int arg; - int idx = 0; - bool verbose = false; - bool daemonize = false; - const char *host = NULL; - char *username = NULL; - char *service = NULL; - int port = 5672; - - struct option opt[] = { - {"help", 0, 0, 'h'}, - {"daemon", 0, 0, 'd'}, - {"broker", 1, 0, 'b'}, - {"username", 1, 0, 'u'}, - {"service", 1, 0, 's'}, - {"port", 1, 0, 'p'}, - {0, 0, 0, 0} - }; - - while ((arg = getopt_long(argc, argv, "hdb:u:s:p:", opt, &idx)) != -1) { - switch (arg) { - case 'h': - case '?': - print_usage(); - exit(0); - break; - case 'd': - daemonize = true; - break; - case 'v': - verbose = true; - break; - case 's': - if (optarg) { - service = strdup(optarg); - } else { - print_usage(); - exit(1); - } - break; - case 'u': - if (optarg) { - username = strdup(optarg); - } else { - print_usage(); - exit(1); - } - break; - case 'p': - if (optarg) { - port = atoi(optarg); - } else { - print_usage(); - exit(1); - } - break; - case 'b': - if (optarg) { - host = strdup(optarg); - } else { - print_usage(); - exit(1); - } - break; - default: - fprintf(stderr, "unsupported option '-%c'. See --help.\n", arg); - print_usage(); - exit(0); - break; - } - } - - if (daemonize == true) { - if (daemon(0, 0) < 0) { - fprintf(stderr, "Error daemonizing: %s\n", strerror(errno)); - exit(1); - } - } - openlog("libvirt-qpid", 0, LOG_DAEMON); - - // This prevents us from dying if libvirt disconnects. - signal(SIGPIPE, SIG_IGN); - - qpid::types::Variant::Map options; - - if (username != NULL) { - options["username"] = username; - } - if (service != NULL) { - options["service"] = service; - } - - if (host == NULL) { - host = "127.0.0.1"; - } - - std::stringstream url; - - url << "amqp:" << "tcp" << ":" << host << ":" << port; - - qpid::messaging::Connection amqp_connection(url.str(), options); - amqp_connection.open(); - - qmf::AgentSession session(amqp_connection); - session.setVendor("redhat.com"); - session.setProduct("libvirt-qpid"); - session.setAttribute("hostname", host); - - session.open(); - NodeWrap::PackageDefinition package; - package.configure(session); - - initErrorSchema(session); - - while (true) { - try { - NodeWrap node(session, package); - node.doLoop(); - } catch (int err) { } - sleep(10); - } -} diff --git a/src/NodeWrap.h b/src/NodeWrap.h index 6f55061..b1ee98a 100644 --- a/src/NodeWrap.h +++ b/src/NodeWrap.h @@ -1,8 +1,7 @@ #ifndef NODE_WRAP_H #define NODE_WRAP_H
-#include "ManagedObject.h" -#include "QmfPackage.h" +#include "LibvirtAgent.h"
#include <unistd.h> #include <cstdlib> @@ -19,7 +18,7 @@ class DomainWrap; class PoolWrap;
class NodeWrap: - public PackageOwner<qmf::com::redhat::libvirt::PackageDefinition>, + public PackageOwner<LibvirtAgent::PackageDefinition>, public ManagedObject { typedef std::vector<DomainWrap*> DomainList; @@ -30,27 +29,16 @@ class NodeWrap:
virConnectPtr _conn;
- qmf::AgentSession& _session; - PackageDefinition& _package; + LibvirtAgent *_agent;
public: - NodeWrap(qmf::AgentSession& agent_session, PackageDefinition& package); + NodeWrap(LibvirtAgent *agent); ~NodeWrap();
- void doLoop(); + void poll(void);
bool handleMethod(qmf::AgentSession& session, qmf::AgentEvent& event);
- virtual PackageDefinition& package(void) { return _package; } - - virtual void addData(qmf::Data& data) { - _session.addData(data); - } - - virtual void delData(qmf::Data& data) { - _session.delData(data.getAddr()); - } - protected: void syncDomains(void); void syncPools(void);
_______________________________________________ Matahari mailing list Matahari@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/matahari
On Tue, Jul 12, 2011 at 06:28:23PM +0200, Zane Bitter wrote:
The following series converts libvirt-qpid into a matahari agent using the QMFv2 APIs.
Since the patches are rather large, I have also pushed them to GitHub for easier reviewing: https://github.com/zaneb/libvirt-qpid/commits/review
Any and all comments are welcome.
To my quick glance, this looks pretty reasonable. As a general question, are we maintaining compatibility with previous releases of libvirt-qpid at a schema level here, or are we starting a new v2 schema with the port to QMFv2 ? Regards,x Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 12/07/11 18:48, Daniel P. Berrange wrote:
On Tue, Jul 12, 2011 at 06:28:23PM +0200, Zane Bitter wrote:
The following series converts libvirt-qpid into a matahari agent using the QMFv2 APIs.
Since the patches are rather large, I have also pushed them to GitHub for easier reviewing: https://github.com/zaneb/libvirt-qpid/commits/review
Any and all comments are welcome.
To my quick glance, this looks pretty reasonable. As a general question, are we maintaining compatibility with previous releases of libvirt-qpid at a schema level here, or are we starting a new v2 schema with the port to QMFv2 ?
Thus far I've attempted to keep them compatible with existing clients (although I haven't tested any). That may or may not be necessary/desirable, especially if we are changing the name anyway. So I'm happy to have that debate. cheers, Zane.
On 07/12/2011 12:52 PM, Zane Bitter wrote:
On 12/07/11 18:48, Daniel P. Berrange wrote:
On Tue, Jul 12, 2011 at 06:28:23PM +0200, Zane Bitter wrote:
The following series converts libvirt-qpid into a matahari agent using the QMFv2 APIs.
Since the patches are rather large, I have also pushed them to GitHub for easier reviewing: https://github.com/zaneb/libvirt-qpid/commits/review
Any and all comments are welcome.
To my quick glance, this looks pretty reasonable. As a general question, are we maintaining compatibility with previous releases of libvirt-qpid at a schema level here, or are we starting a new v2 schema with the port to QMFv2 ?
Thus far I've attempted to keep them compatible with existing clients (although I haven't tested any). That may or may not be necessary/desirable, especially if we are changing the name anyway. So I'm happy to have that debate.
If we think we might need to break schema compat in the near future anyhow, perhaps now is the time to do it given all of the other changes taking place rather than waiting?
On 07/12/2011 06:55 PM, Perry Myers wrote:
If we think we might need to break schema compat in the near future anyhow, perhaps now is the time to do it given all of the other changes taking place rather than waiting?
ack, bump version and go ahead. The only project I know using libvirt-qpid (and old matahari btw) was ovirt-server and that one is currently out of shape anyway (due to rails changes). Anyone reviving it will need to adapt to both new matahari host agent and libvirt-qpid schemas. Alan
participants (5)
-
Alan Pevec -
Andrew Beekhof -
Daniel P. Berrange -
Perry Myers -
Zane Bitter