Re: [libvirt] [PATCH v4 5/8] qemu_monitor: implement query-cpu-model-comparison

>> +int >> +qemuMonitorJSONGetCPUModelComparison(qemuMonitorPtr mon, >> +                                     const char *model_a_name, >> +                                     int model_a_nprops, >> +                                     virCPUFeatureDefPtr model_a_props, >> +                                     const char *model_b_name, >> +                                     int model_b_nprops, >> +                                     virCPUFeatureDefPtr model_b_props, >> +                                     qemuMonitorCPUModelInfoPtr >> *model_result) >> +{ >> +    int ret = -1; >> +    virJSONValuePtr model_a; >> +    virJSONValuePtr model_b = NULL; >> +    virJSONValuePtr cmd = NULL; >> +    virJSONValuePtr reply = NULL; >> +    virJSONValuePtr data; >> +    const char *result_name; >> +    virJSONValuePtr props; >> +    qemuMonitorCPUModelInfoPtr compare = NULL; >> + >> +    if (!(model_a = qemuMonitorJSONMakeCPUModel(model_a_name, >> model_a_nprops, >> +                                                model_a_props, >> true)) || >> +        !(model_b = qemuMonitorJSONMakeCPUModel(model_b_name, >> model_b_nprops, >> +                                                model_b_props, true))) >> +        goto cleanup; >> + >> +    if (!(cmd = >> qemuMonitorJSONMakeCommand("query-cpu-model-comparison", >> +                                           "a:modela", &model_a, >> +                                           "a:modelb", &model_b, >> +                                           NULL))) >> +        goto cleanup; >> + >> +    if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) >> +        goto cleanup; >> + >> +    if (qemuMonitorJSONCheckError(cmd, reply) < 0) > > Did you test scenarios which contain unknown cpu model name or unknown > features? > This caused errors for me during testing that resulted in e.g. >   error : qemuMonitorJSONCheckError:415 : internal error: unable to > execute QEMU command 'query-cpu-model-comparison': Property '.boris' > not found >   warning : virQEMUCapsLogProbeFailure:4586 : Failed to probe > capabilities for /usr/bin/qemu-system-s390x: internal error: unable to > execute QEMU command 'query-cpu-model-comparison': Property '.boris' > not found > > If I think about a scenario like: I run "virsh domcapabilties" on a > new machine with new cpu model and new cpu features, new libvirt, new > qemu and use "virsh hypervisor-cpu-compare" with the output xml on my > old machine with old cpu &feature, older libvirt and older qemu > I would expect to get as an answer: Incompatible > > If my expectation to get incompatible is wrong please correct me but > an "internal error" is not what should occur. > What is the qemu specification for this? > > Two routes: 1) Fix up the logic in virQEMUCapsCPUModelComparison Please see my comments on patch 8. 2) Change the error class in the QEMU QMP code The QMP code in QEMU currently reports a "GenericError" whenever something goes wrong (such as an unknown CPU model or feature). A possible solution to this would be to alter that code to set a specific error class, have libvirt check for that specific error type in the JSON, then handle it appropriately to print a cleaner message. There is one drawback: we would technically be reporting this error twice -- once in virsh, and another in the libvirt daemon (the internal error) unless we wanted to suppress it. This is one solution without too much messy code.