Re: [libvirt] [PATCH] conf: Report sensible error for invalid disk name

On Tue, Nov 20, 2012 at 02:55:28PM +0100, Martin Kletzander wrote: > The error "... but the cause is unknown" appeared for XMLs similar to > this: > > <disk type='file' device='cdrom'> > <driver name='qemu' type='raw'/> > <source file='/dev/zero'/> > <target dev='sr0'/> > </disk> > > Notice unsupported disk type (for the driver), but also no address > specified. The first part is not a problem and we should not abort > immediately because of that, but the combination with the address > unknown was causing an unspecified error. > --- > src/util/util.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/src/util/util.c b/src/util/util.c > index 75b18c1..d5b2c97 100644 > --- a/src/util/util.c > +++ b/src/util/util.c > @@ -2189,8 +2189,12 @@ int virDiskNameToIndex(const char *name) { > } > } > > - if (!ptr) > + if (!ptr) { > + virReportError(VIR_ERR_XML_ERROR, > + _("Unknown disk name '%s' and no address specified"), > + name); > return -1; > + } > > for (i = 0; *ptr; i++) { > idx = (idx + (i < 1 ? 0 : 1)) * 26; IMHO, you should really be adding an ATTRIBUTE_NONNULL annotation and then fixing the callers not to invoke it if the disk name they have is Null.

Adding virReportError here is wrong, because a number of callers will already report errors.