On 01/31/2012 07:03 AM, Daniel P. Berrange wrote:
On Tue, Jan 31, 2012 at 01:49:11PM +0900, Taku Izumi wrote:
@@ -3156,6 +3159,26 @@ virDomainDiskDefParseXML(virCapsPtr caps def->snapshot = VIR_DOMAIN_DISK_SNAPSHOT_NO; }
+ def->rawio = -1; /* unspecified */ + if (rawio) { + if (def->device == VIR_DOMAIN_DISK_DEVICE_LUN) { + if (STREQ(rawio, "yes")) { + def->rawio = 1; + } else if (STREQ(rawio, "no")) { + def->rawio = 0; + } else { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown disk rawio setting '%s'"), + rawio); + goto error; + } + } else { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("rawio can be used only with device='lun'")); s/INTERNAL_ERROR/XML_ERROR/
ACK with that change
I've pushed this patch with that, and some other, changes (see attached diff for all changes aside from what you pointed out - I managed to squash that in before I thought to save a diff). Just now I realize I had misinterpreted your comment as being only about the first of those two messages (my brain stopped looking at the email as soon as I hit the first one - you should have done "s/.../.../g" :-), but when I looked at the file I saw the 2nd, I decided it should be VIR_ERR_CONFIG_UNSUPPORTED. Should I push a change to XML_ERROR, or leave it as is? (I *still* get confused about which is more appropriate where, but this one is in kind of a gray area.) I also found that "make check wouldn't pass, which was mostly traced to the concept of making the default value of rawio "-1". The problem with this is that there are other functions that create and fill-in domain structures, and they hadn't been taught about this default value. I changed the object to have a "bool rawio_specified", and modified the parse and format functions accordingly. Also there was no test that exercised the rawio attribute - I added that into the virtio-lun test. (and just now, I realize that I forgot to add a blurb in the docs. I'll do that when I'm done with these emails)