Am 25.04.2013 um 17:03 hat Jeff Cody geschrieben:
> On Thu, Apr 25, 2013 at 03:04:23PM +0200, Kevin Wolf wrote:
> > Am 23.04.2013 um 16:24 hat Jeff Cody geschrieben:
> > > + if (!vhdx_checksum_is_valid(buffer, VHDX_HEADER_BLOCK_SIZE, 4) ||
> > > + s->rt.signature != VHDX_RT_MAGIC) {
> > > + ret = -EINVAL;
> > > + goto fail;
> > > + }
> > > +
> > > + for (i = 0; i < s->rt.entry_count; i++) {
> > > + memcpy(&rt_entry, buffer+offset, sizeof(rt_entry));
> > > + offset += sizeof(rt_entry);
> > > +
> > > + leguid_to_cpus(&rt_entry.guid);
> > > + le64_to_cpus(&rt_entry.file_offset);
> > > + le32_to_cpus(&rt_entry.length);
> > > + le32_to_cpus(&rt_entry.data_bits);
> > > +
> > > + /* see if we recognize the entry */
> > > + if (guid_eq(rt_entry.guid, bat_guid)) {
> > > + s->bat_rt = rt_entry;
> > > + continue;
> > > + }
> > > +
> > > + if (guid_eq(rt_entry.guid, metadata_guid)) {
> > > + s->metadata_rt = rt_entry;
> > > + continue;
> > > + }
> >
> > If the same regions occurs multiple times, the latest wins. Such images
> > aren't valid, but what should we do with such cases? Is this good enough
> > or should we detect it?
>
> I think such images are more undefined rather than explicitly invalid.
> I don't think the spec touches on the idea of multiple regions of the
> same type.
It says "The Guid field specifies a 128-bit identifier for the object
and must be unique within the table.", so it's explicitly forbidden to
have two regions of the same type.
> That said, I don't what to make of an image file with
> multiple BAT regions, for instance - I think error is the only sane
> option.
>
> Maybe keep a list of all entries, and if there are duplicates error
> out with -EINVAL?
We could do that if we really care. Or just check for the ones that we
recognise that they aren't set yet when we assign them.
> > > + /* We now have the file parameters, so we can tell if this is a
> > > + * differencing file (i.e.. has_parent), is dynamic or fixed
> > > + * sized (leave_blocks_allocated), and the block size */
> > > +
> > > + /* The parent locator required iff the file parameters has_parent
> > > set */
> > > + if (s->params.data_bits & VHDX_PARAMS_HAS_PARENT) {
> > > + if (s->metadata_entries.present & ~META_PARENT_LOCATOR_PRESENT) {
> >
> > Not sure what you're trying to achieve here. We get -ENOTSUP if any
> > metadata entry except the parent locator is present, and -EINVAL if
> > there is none?
> >
>
> That is a mistake - that check should be sans tilde:
> if (s->metadata_entries.present & META_PARENT_LOCATOR_PRESENT)
Right, that makes more sense.
Kevin
