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

Re: [libvirt] [PATCH v2 3/3] virsh: Implement domblkerror command



On Mon, Jan 30, 2012 at 16:20:07 -0700, Eric Blake wrote:
> On 01/30/2012 09:01 AM, Jiri Denemark wrote:
> > This command lists all disk devices with errors
> > ---
> >  tools/virsh.c   |   89 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  tools/virsh.pod |    7 ++++
> >  2 files changed, 96 insertions(+), 0 deletions(-)
...
> > +static const vshCmdOptDef opts_domblkerror[] = {
> > +    {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id, or uuid")},
> > +    {NULL, 0, 0, NULL}
> 
> Should we also allow this additional usage:
> 
> virsh domblkerror dom vda
> 
> which lists the status of just vda (no error, no space left, ...)?  That
> would mean adding:
> 
> {"disk", VSH_OT_DATA, VSH_OFLAG_OPT, N_("particular disk to check")}
> 
> then filtering through the libvirt API to match just that disk?  You can
> say "no, that's overkill" and not implement it, and I won't be hurt.

I think grep/sed/awk and similar friends can do similar job here if anyone
wants to get the error status of just one specific disk.

> > +    if ((count = virDomainGetDiskErrors(dom, disks, ndisks, 0)) == -1)
> > +        goto cleanup;
> > +
> > +    for (i = 0; i < count; i++) {
> > +        vshPrint(ctl, "%s: %s\n",
> > +                 disks[i].disk,
> > +                 vshDomainIOErrorToString(disks[i].error));
> > +    }
> 
> Are we okay that if there are no disk errors (count is 0), we have no
> output, not even mentioning that the command succeeded?

I guess we are not and I changed that. I also made this command consistent
with python API which reports no disk errors for domain without disks instead
of complaining that the domain has no disks.

Jirka


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