On 08/09/2012 09:44 AM, Peter Krempa wrote: > On 08/09/12 15:38, Eric Blake wrote: >> On 08/09/2012 07:31 AM, Peter Krempa wrote: >>> This patch introduces a new error code VIR_ERR_OPERATION_UNSUPPORTED to >>> mark error messages regarding operations that failed due to lack of >>> support on the hypervisor or other than libvirt issues. >>> >>> The code is first used in reporting error if qemu does not support >>> block >>> IO tuning variables yielding error message: >>> error: Unable to get block I/O throttle parameters >>> error: Operation not supported: block_io_throttle field >>> 'total_bytes_sec' missing in qemu's output >>> >>> instead of: >>> error: Unable to get block I/O throttle parameters >>> error: internal error cannot read total_bytes_sec >>> --- >> >> In the past, we have used VIR_ERR_CONFIG_UNSUPPORTED for messages about >> a qemu binary that doesn't support something; would that be any better >> than inventing a new error here? Or are all of those errors worth >> switching over to this new code? > > Using VIR_ERR_CONFIG_UNSUPPORTED seems reasonable to me in cases where > the unsupported feature is requested by the user. Eg. when setting > something or requesting some weird configuration or even if the > hypervisor doesn't support it. > > On the other hand It doesn't make sense to me to use it on getter-type > APIs where the user isn't setting anything just wants some information > back. In this case I'd rather like to see a new message, as stating > that config isn't supported is a little bit strange. > >> >> As written, your patch seems fine, but only if we agree that a new error >> is the way to go. There was already a short discussion about this related to another patch: https://www.redhat.com/archives/libvir-list/2012-August/msg00589.html Peter wrote this patch in response to that discussion. My vote is in favor of the new code. <knows-a-bandwagon-when-he-sees-it/> -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list