[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] virsh: Use old API if remote libvirtd does not support new



On 09/13/2011 09:16 AM, Peter Krempa wrote:
Commit ffe28ab74b821c916ec4ba8efb5c992454e4bd24 introduced regression
while comunicating with older libvirtd command 'domblkstat' used the new
API and did not check for VIR_ERR_RPC error code signalling the remote
server does not support this API and did not fall back to older API.
Thereafter 'domblkstat' ended with "error: unknown procedure: 243".
---

ACK.

footnote: Will not apply along with:
[libvirt] [PATCH v5] virsh: Add more human-friendly output of domblkstat command
as that path does fix this bug.

Apply this first, then rebase that patch into a v6. Then the human-friendly patch is separate, rather than pushing two fixes in one patch (that is, upstream should have two patches, while for back-porting purposes, it might make sense to backport just this).


  tools/virsh.c |    3 ++-
  1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 3b060bf..430168c 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -1090,7 +1090,8 @@ cmdDomblkstat (vshControl *ctl, const vshCmd *cmd)
       * then.
       */
      if (rc<  0) {
-        if (last_error->code != VIR_ERR_NO_SUPPORT) {
+        if (last_error->code != VIR_ERR_NO_SUPPORT&&
+            last_error->code != VIR_ERR_RPC) {

Hmm, I wonder if any of my recent snapshot patches also need to check for VIR_ERR_RPC (for example, see cmdUndefine, which checks for VIR_ERR_NO_SUPPORT but not VIR_ERR_RPC). Yep, I'm starting to convince myself that they do - if you have a new client talking to a pre-0.8.0 server, then virDomainSnapshotNum would fail with VIR_ERR_RPC rather than VIR_ERR_NO_SUPPORT.

Or, maybe this means that we have a bug in our RPC code - when the local side gets VIR_ERR_RPC from the remote side due to unknown rpc number, should the local side be translating that into VIR_ERR_NO_SUPPORT instead of passing it back to the user as VIR_ERR_RPC? But only for unknown rpc numbers; for other rpc errors, VIR_ERR_RPC still makes sense.

--
Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]