On Thu, Feb 27, 2025 at 12:41:10 +0100, Anthony Harivel wrote:
> Hi Peter,
> 
> >> @@ -7062,6 +7065,15 @@ qemuBuildAccelCommandLine(virCommand *cmd,
> >>              def->kvm_features->features[VIR_DOMAIN_KVM_DIRTY_RING] == 
> >> VIR_TRISTATE_SWITCH_ON) {
> >>              virBufferAsprintf(&buf, ",dirty-ring-size=%d", 
> >> def->kvm_features->dirty_ring_size);
> >>          }
> >> +
> >> +        if (def->features[VIR_DOMAIN_FEATURE_KVM] == 
> >> VIR_TRISTATE_SWITCH_ON &&
> >> +            def->kvm_features->features[VIR_DOMAIN_KVM_RAPL] == 
> >> VIR_TRISTATE_SWITCH_ON) {
> >> +            virBufferAddLit(&buf, ",rapl=true");
> >> +
> >> +            if (def->kvm_features->rapl_helper_socket != NULL) {
> >> +                virBufferAsprintf(&buf, ",rapl-helper-socket=%s", 
> >> def->kvm_features->rapl_helper_socket);
> >> +            }
> >
> > As noted above; qemu makes the socket mandatory so the check doesn't
> > make much sense.
> >
> 
> I'm not sure to follow the above remark. The socket is mandatory in QEMU 
> yes, so it should also be mandatory in libvirt so that we don't make the 
> QEMU process fails at start ? 

You should validate that the socket is present; either provided by user
or auto-generated once managed mode will be introduced.

Also you then don't need to conditionally format it any more once
you check that it's present.


> Or do we just let the user check what QEMU is returning so that the user 
> correct later the XML ? 

Normally we validate config beforehand; the errors from qemu can be iffy
sometimes.

> 
> Thanks
> Anthony
> 
> > Also we'll most likely need to use virQEMUBuildBufferEscapeComma here to

Also don't forget this                   ^^^

> > properly format paths with a comma since they are user-provided.
> >
> 
> 

Reply via email to